Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Syntax factory #1530

Merged
merged 26 commits into from
Mar 4, 2025
Merged

Syntax factory #1530

merged 26 commits into from
Mar 4, 2025

Conversation

7h3kk1d
Copy link
Contributor

@7h3kk1d 7h3kk1d commented Feb 20, 2025

Add factory functions for AST

Adds a bunch of helper functions for building up the AST with default annotations.

Replaces the constructors in tests with the new helper functions to remove fresh calls. Also moved builtins.re and transition.re to use the new functions.

…statics tests

- Uses temp module to remove fresh/invalid calls in statics test
- Uses Info error module to create expressions with default no errors
Copy link

codecov bot commented Feb 20, 2025

Codecov Report

Attention: Patch coverage is 78.43602% with 91 lines in your changes missing coverage. Please review.

Project coverage is 53.29%. Comparing base (4390190) to head (9d3f368).
Report is 27 commits behind head on dev.

Files with missing lines Patch % Lines
src/haz3lcore/dynamics/Transition.re 16.32% 41 Missing ⚠️
src/haz3lcore/dynamics/Builtins.re 74.39% 21 Missing ⚠️
src/haz3lcore/lang/term/Grammar.re 92.00% 14 Missing ⚠️
src/haz3lmenhir/Conversion.re 87.50% 14 Missing ⚠️
src/haz3lcore/lang/term/Typ.re 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #1530      +/-   ##
==========================================
+ Coverage   50.33%   53.29%   +2.95%     
==========================================
  Files         103      103              
  Lines       10442    10583     +141     
==========================================
+ Hits         5256     5640     +384     
+ Misses       5186     4943     -243     
Files with missing lines Coverage Δ
src/haz3lcore/lang/Operators.re 92.43% <ø> (+43.69%) ⬆️
src/haz3lcore/lang/term/Cls.re 7.14% <ø> (+7.14%) ⬆️
src/haz3lcore/lang/term/IdTagged.re 66.66% <100.00%> (+3.50%) ⬆️
src/haz3lcore/lang/term/TPat.re 88.23% <ø> (+64.70%) ⬆️
src/haz3lcore/lang/term/Term.re 51.41% <ø> (+18.80%) ⬆️
src/haz3lcore/tiles/Secondary.re 27.27% <ø> (ø)
src/haz3lcore/lang/term/Typ.re 55.08% <0.00%> (+10.87%) ⬆️
src/haz3lcore/lang/term/Grammar.re 78.67% <92.00%> (+11.33%) ⬆️
src/haz3lmenhir/Conversion.re 54.41% <87.50%> (-1.36%) ⬇️
src/haz3lcore/dynamics/Builtins.re 63.35% <74.39%> (-0.23%) ⬇️
... and 1 more

... and 3 files with indirect coverage changes

};

// pat
let pat_invalid = (~ann=?, s): pat_t(DefaultAnnotation.t) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can also nest modules inside here instead of prepending sort. This works with local open so you could do:

FTemp.(Exp.(list([int(1), int(2), cast(int(3), Typ.int(), Typ.unknown(Internal)))))

Comment on lines 433 to 445
ty_prod([
ty_tup_label(
ty_label("a"),
ty_prod([
ty_tup_label(
ty_label("b"),
ty_prod([
ty_tup_label(ty_label("c"), ty_unknown(Hole(EmptyHole))),
]),
),
]),
),
]),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example of the fresh calls being removed.

Comment on lines 795 to 884
Common(
TupleLabelError({
malformed_labels: [],
duplicate_labels: [],
invalid_labels: ["c"],
typ:
TupLabel(
Label("c") |> Typ.fresh,
Int |> Typ.fresh,
)
|> Typ.fresh,
}),
),
),
TupLabel(
error_exp(
Exp(Common(NoType(InvalidLabel("c")))),
Label("c"),
Inconsistent(
FTemp.(
Expectation({
ana:
ty_parens(
ty_prod([
ty_int(),
ty_tup_label(ty_label("a"), ty_string()),
]),
),
syn:
ty_prod([
ty_tup_label(ty_label("c"), ty_int()),
ty_tup_label(ty_label("a"), ty_string()),
]),
})
),
no_error_exp(Int(1)),
),
),
no_error_exp(
TupLabel(
no_error_exp(Label("a")),
no_error_exp(String("hello")),
),
),
tuple(
~ann=
Some(
Exp(
Common(
TupleLabelError({
malformed_labels: [],
duplicate_labels: [],
invalid_labels: ["c"],
typ:
FTemp.(
ty_prod([
ty_tup_label(ty_label("c"), ty_int()),
ty_tup_label(ty_label("a"), ty_string()),
])
),
}),
),
),
]),
),
),
[
{
tup_label(
~ann=
Some(
Exp(
Common(
TupleLabelError({
malformed_labels: [],
duplicate_labels: [],
invalid_labels: ["c"],
typ:
FTemp.(
ty_tup_label(ty_label("c"), ty_int())
),
}),
),
),
),
error_exp(
Exp(Common(NoType(InvalidLabel("c")))),
Label("c"),
),
int(1),
);
},
tup_label(label("a"), string("hello")),
],
),
),
no_error_exp(Bool(true)),
),
bool(true),
)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test is significantly smaller and only 30 out of the remaining ~90 lines are the AST. The rest are the annotations.

Base automatically changed from syntax_playground to dev February 25, 2025 17:20
let tests = (
"Grammar",
[QCheck_alcotest.to_alcotest(qcheck_map_annotation_test)],
[
test_case(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added these test cases to walk over all of the expressions, types, patterns, ... to make sure that we can build one for each cls and the class is in 1-1 correspondence with the term types. This turns out not to be true for types for Constructor.

The tests help ensure cls are in order and that we have the factory functions. I think that's good enough for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'll find a way to derive both in the future.

@7h3kk1d 7h3kk1d marked this pull request as ready for review February 26, 2025 22:08
Comment on lines +421 to +424
module type DefaultAnnotation = {
type t;
let default_value: unit => t;
};
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could have 2 types here t for the type of the annotation and another type s that could be used in the factory methods below and require a default_value : option(s) -> t as part of the builder. I haven't bothered but for optional annotations it allows the people calling the builder to use a different type like not provide Some constructors.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leave as a comment in the file

Comment on lines +421 to +424
module type DefaultAnnotation = {
type t;
let default_value: unit => t;
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leave as a comment in the file

@cyrus- cyrus- merged commit 6df1774 into dev Mar 4, 2025
4 checks passed
@cyrus- cyrus- deleted the syntax_factory branch March 4, 2025 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants