Skip to content

Commit

Permalink
use labels for AST selectors
Browse files Browse the repository at this point in the history
Uses labels to select AST nodes, which is more accurate, requires less code (instead of checking individual kinds), and also fixes a bug where additional `TerminalKind`s of `PrecedenceOperator` are not taken into account.

Closes NomicFoundation#872
Closes NomicFoundation#986
  • Loading branch information
OmarTawfik committed Jun 10, 2024
1 parent 33cf0ee commit 3fcde27
Show file tree
Hide file tree
Showing 18 changed files with 1,502 additions and 3,554 deletions.
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ use std::rc::Rc;
use crate::compiler::analysis::{Analysis, ItemMetadata};
use crate::compiler::version_set::VersionSet;
use crate::internals::Spanned;
use crate::model::{Identifier, SpannedItem, SpannedVersionSpecifier};
use crate::model::{
Identifier, SpannedField, SpannedItem, SpannedOperatorModel, SpannedPrecedenceOperator,
SpannedVersionSpecifier,
};

pub(crate) fn analyze_definitions(analysis: &mut Analysis) {
collect_top_level_items(analysis);
Expand Down Expand Up @@ -84,6 +87,15 @@ fn check_precedence_items(analysis: &mut Analysis) {
}

current_expressions.insert(name);

// Make sure all operators have the same structure, as they are represented using the same AST type:
let first_op = &precedence_expression.operators[0];
for second_op in precedence_expression.operators.iter().skip(1) {
if !compare_operators(analysis, first_op, second_op) {
analysis.errors.add(name, &Errors::OperatorMismatch);
continue;
}
}
}

for primary_expression in &item.primary_expressions {
Expand All @@ -98,6 +110,76 @@ fn check_precedence_items(analysis: &mut Analysis) {
}
}

fn compare_operators(
analysis: &mut Analysis,
first_op: &SpannedPrecedenceOperator,
second_op: &SpannedPrecedenceOperator,
) -> bool {
match (&*first_op.model, &*second_op.model) {
// Allow it if they are both prefixes:
(SpannedOperatorModel::Prefix, SpannedOperatorModel::Prefix)
// Allow it if they are both suffixes:
| (SpannedOperatorModel::Postfix, SpannedOperatorModel::Postfix)
// Allow it if they are both binary (regardless of associativity):
| (
SpannedOperatorModel::BinaryLeftAssociative
| SpannedOperatorModel::BinaryRightAssociative,
SpannedOperatorModel::BinaryLeftAssociative
| SpannedOperatorModel::BinaryRightAssociative,
) => {}
_ => return false,
};

// Must have the same number of fields:
if first_op.fields.len() != second_op.fields.len() {
return false;
}

for ((first_key, first_field), (second_key, second_field)) in
first_op.fields.iter().zip(second_op.fields.iter())
{
// Must have the same labels:
if first_key != second_key {
return false;
}

let (
// Allow it if both are required:
(
SpannedField::Required { reference: first_ref },
SpannedField::Required { reference: second_ref },
)
// Allow it if both are optional (regardless of enablement):
| (
SpannedField::Optional { reference: first_ref, enabled: _ },
SpannedField::Optional { reference: second_ref, enabled: _ },
)
) = (first_field, second_field)
else {
return false;
};

// Allow it if they both reference the same exact kind:
if first_ref == second_ref {
continue;
}

// Otherwise, allow it if both are terminals:
match (
&analysis.metadata[&**first_ref].item,
&analysis.metadata[&**second_ref].item,
) {
(
SpannedItem::Keyword { .. } | SpannedItem::Token { .. },
SpannedItem::Keyword { .. } | SpannedItem::Token { .. },
) => {}
_ => return false,
}
}

true
}

fn get_item_name(item: &SpannedItem) -> &Spanned<Identifier> {
match item {
SpannedItem::Struct { item } => &item.name,
Expand Down Expand Up @@ -169,4 +251,6 @@ enum Errors<'err> {
ExistingVariant(&'err Identifier),
#[error("An expression with the name '{0}' already exists.")]
ExistingExpression(&'err Identifier),
#[error("All operators under the same expression must have the same model and type.")]
OperatorMismatch,
}
4 changes: 3 additions & 1 deletion crates/codegen/language/definition/src/model/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,9 @@ pub struct Topic {
pub items: Vec<Item>,
}

#[derive(strum_macros::AsRefStr, strum_macros::EnumIter, strum_macros::VariantNames)]
#[derive(
Clone, Copy, Debug, strum_macros::AsRefStr, strum_macros::EnumIter, strum_macros::VariantNames,
)]
#[strum(serialize_all = "snake_case")]
pub enum BuiltInLabel {
Item,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,27 @@ codegen_language_macros::compile!(Language(
Precedence(
name = Bar,
precedence_expressions = [
PrecedenceExpression(name = Expression1, operators = []),
PrecedenceExpression(name = Expression2, operators = []),
PrecedenceExpression(name = Expression1, operators = [])
PrecedenceExpression(
name = Expression1,
operators = [PrecedenceOperator(
model = BinaryLeftAssociative,
fields = (operator = Required(Baz))
)]
),
PrecedenceExpression(
name = Expression2,
operators = [PrecedenceOperator(
model = BinaryLeftAssociative,
fields = (operator = Required(Baz))
)]
),
PrecedenceExpression(
name = Expression1,
operators = [PrecedenceOperator(
model = BinaryLeftAssociative,
fields = (operator = Required(Baz))
)]
)
],
primary_expressions = [PrimaryExpression(reference = Baz)]
),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: An expression with the name 'Expression1' already exists.
--> src/fail/definitions/duplicate_expression_name/test.rs:20:53
--> src/fail/definitions/duplicate_expression_name/test.rs:33:36
|
20 | PrecedenceExpression(name = Expression1, operators = [])
| ^^^^^^^^^^^
33 | ... name = Expression1,
| ^^^^^^^^^^^
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
#![allow(unused_crate_dependencies)]

codegen_language_macros::compile!(Language(
name = Foo,
documentation_dir = "foo/bar",
root_item = Bar,
leading_trivia = Sequence([]),
trailing_trivia = Sequence([]),
versions = ["1.0.0"],
sections = [Section(
title = "Section One",
topics = [Topic(
title = "Topic One",
items = [
Precedence(
name = Expression,
precedence_expressions = [PrecedenceExpression(
name = Foo,
operators = [
PrecedenceOperator(
model = BinaryLeftAssociative,
fields = (operator = Required(Baz1))
),
PrecedenceOperator(
model = Prefix,
fields = (operator = Required(Baz1))
)
]
)],
primary_expressions = [PrimaryExpression(reference = Baz1)]
),
Token(
name = Baz1,
definitions = [TokenDefinition(scanner = Atom("baz1"))]
)
]
)]
)]
));

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
error: All operators under the same expression must have the same model and type.
--> src/fail/definitions/operator_mismatch/test.rs:18:32
|
18 | name = Foo,
| ^^^
Loading

0 comments on commit 3fcde27

Please sign in to comment.