feat(compiler)!: Remove arbitrary per-file compiler flags, add acceptable options as module attributes #1804

alex-snezhko · 2023-04-06T02:48:14Z

@noPervasives
@runtimeMode
module Example

Closes #1625

spotandjake

Im loving how much cleaner this looks, looks like you forgot to run the formatter thats why the tests are failing the attribute looks a lot cleaner then the comment though.

phated

This is great ❤️ No shocker that I have opinions on the API. I'd also like to know if you can apply this to submodules in a file, since I think that would be :chefs-kiss:

cli/__test__/index.gr

compiler/src/compile.re

compiler/test/input/memoryBase/asserts.gr

compiler/test/suites/arrays.re

compiler/test/suites/print.re

peblair

One comment and one question, but otherwise this looks good!

compiler/src/utils/config.rei

compiler/test/suites/parsing.re

compiler/src/utils/config.re

compiler/src/parsing/parsetree.re

spotandjake

Looks good to me :), one small question.

stdlib/runtime/wasi.gr

compiler/graindoc/docblock.re

phated

@alex-snezhko this needs to be updated for the new formatter. We'll probably need locations on the module attributes to handle comments between them too.

phated

Had a few questions, but overall looks great

compiler/src/formatting/fmt.re

compiler/src/parsing/parser.messages

compiler/src/parsing/parser_header.re

compiler/src/typed/env.rei

compiler/src/typed/typed_well_formedness.re

compiler/src/utils/config.re

phated · 2024-02-25T23:54:41Z

compiler/test/grainfmt/comments.expected.gr

+// Comment after first attr
+@runtimeMode
+// Comment after second attr
+


Should this newline be removed so attributes are always "hugging" the module?

This is the behavior for attributes on other items as well, maybe can address in a followup?

Please open an issue

Wait, you've added the formatter changes. You can just change the line ending. Right? We still need a follow up to fix the other ones though.

Looked into this and I think it would require making some invasive changes to the print_comment_range function to disallow double-breaks. I think we can just defer to #2055

compiler/test/grainfmt/comments.expected.gr

compiler/test/input/memoryBase/sameAsDefault.gr

phated · 2024-02-29T13:36:10Z

@alex-snezhko since the location fix was landed in @ospencer's PR. Can you back out your changes/resolve conflicts here?

compiler/src/formatting/fmt.re

compiler/src/parsing/parsetree_iter.re

compiler/test/suites/basic_functionality.re

compiler/test/suites/functions.re

phated

Few more things

…dness check

phated · 2024-03-03T22:12:57Z

compiler/src/parsing/parsetree_iter.re

-  enter_attribute: (attribute, attribute_context) => unit,
-  leave_attribute: (attribute, attribute_context) => unit,


I don't like that you removed these. We might want to iter attributes at some point

phated · 2024-03-03T22:16:04Z

compiler/src/formatting/fmt.re

+          ~none=hardline,
+          ~lead=space,
+          ~trail=hardline,


I'm pretty sure that the ~none and/or ~trail being hardline here are causing your extra newline in the attributes.

Doesn't look like that fixed it; I think it's the fact that print_comment_range inserts phatom_hardline ++ phantom_hardline since line_delta > 1 between the comment and module header

phated · 2024-03-03T22:16:33Z

compiler/src/parsing/parsetree_iter.re

-  enter_attribute: attribute => unit,
-  leave_attribute: attribute => unit,


Can you put these back please? I don't see a reason to remove them

phated · 2024-03-03T22:17:18Z

compiler/src/parsing/parsetree_iter.re

@@ -87,6 +83,7 @@ let iter_attributes = (hooks, attrs) => {

 let rec iter_parsed_program = (hooks, {statements} as program) => {
  hooks.enter_parsed_program(program);
+  iter_attributes(hooks, program.attributes);


What's going on here?

What do you mean? It's iterating over the attributes in the same way done with expressions and toplevels

phated · 2024-03-03T22:18:17Z

compiler/test/grainfmt/comments.expected.gr

@@ -1,3 +1,9 @@
+// Comment before first attr
+/* Block comment before attr */@noPervasives // Line comment on attr


This is wrong. It needs to have a space between */ and @

Looks like in other places it actually puts the block comment on its own line, which I think I prefer here

alex-snezhko requested review from ospencer, peblair, jozanza, phated and marcusroberts as code owners April 6, 2023 02:48

spotandjake reviewed Apr 6, 2023

View reviewed changes

phated requested changes Apr 7, 2023

View reviewed changes

alex-snezhko changed the title ~~feat(compiler)!: Per-file compiler flags as module attribute~~ feat(compiler)!: Remove arbitrary per-file compiler flags, add acceptable options as module attributes Apr 11, 2023

alex-snezhko requested a review from phated April 11, 2023 22:26

peblair reviewed Apr 12, 2023

View reviewed changes

compiler/src/utils/config.rei Outdated Show resolved Hide resolved

compiler/test/suites/parsing.re Outdated Show resolved Hide resolved

phated reviewed Apr 15, 2023

View reviewed changes

compiler/src/utils/config.re Outdated Show resolved Hide resolved

phated assigned alex-snezhko Apr 16, 2023

alex-snezhko force-pushed the grainc-flags-attr branch from c035ccc to 5f06640 Compare April 25, 2023 19:10

ospencer reviewed Apr 26, 2023

View reviewed changes

compiler/src/parsing/parsetree.re Outdated Show resolved Hide resolved

alex-snezhko force-pushed the grainc-flags-attr branch from 3667416 to 685cb5f Compare April 28, 2023 21:28

peblair approved these changes Jun 12, 2023

View reviewed changes

alex-snezhko mentioned this pull request Jun 26, 2023

feat(compiler)!: foreign module syntax #1873

Open

alex-snezhko force-pushed the grainc-flags-attr branch from 74083da to 667e25a Compare June 28, 2023 21:51

spotandjake added the compiler label Jul 21, 2023

alex-snezhko force-pushed the grainc-flags-attr branch from 667e25a to 85ca98e Compare January 27, 2024 03:57

alex-snezhko requested a review from phated January 27, 2024 19:38

spotandjake reviewed Feb 2, 2024

View reviewed changes

stdlib/runtime/wasi.gr Show resolved Hide resolved

alex-snezhko force-pushed the grainc-flags-attr branch from 85ca98e to a6664bf Compare February 4, 2024 01:14

ospencer approved these changes Feb 21, 2024

View reviewed changes

phated reviewed Feb 22, 2024

View reviewed changes

compiler/graindoc/docblock.re Outdated Show resolved Hide resolved

phated requested changes Feb 22, 2024

View reviewed changes

alex-snezhko force-pushed the grainc-flags-attr branch 2 times, most recently from 86edd36 to c3e4c0a Compare February 25, 2024 23:40

phated requested changes Feb 26, 2024

View reviewed changes

alex-snezhko force-pushed the grainc-flags-attr branch from 6b8a360 to 1d7bad3 Compare March 2, 2024 22:38

phated reviewed Mar 2, 2024

View reviewed changes

compiler/src/formatting/fmt.re Outdated Show resolved Hide resolved

phated reviewed Mar 2, 2024

View reviewed changes

compiler/src/parsing/parsetree_iter.re Outdated Show resolved Hide resolved

phated reviewed Mar 2, 2024

View reviewed changes

compiler/test/suites/basic_functionality.re Show resolved Hide resolved

phated reviewed Mar 2, 2024

View reviewed changes

compiler/test/suites/functions.re Outdated Show resolved Hide resolved

phated requested changes Mar 2, 2024

View reviewed changes

alex-snezhko force-pushed the grainc-flags-attr branch 3 times, most recently from dc87d08 to c20f159 Compare March 3, 2024 20:00

alex-snezhko added 3 commits March 3, 2024 21:17

feat(compiler)!: Per-file compiler flags as module attribute

2918d8c

Make attributes like other places, include module attrs in well-forme…

03b45e7

…dness check

Remove specialized enter_attribute function

39aa39d

alex-snezhko force-pushed the grainc-flags-attr branch from c20f159 to 39aa39d Compare March 3, 2024 21:18

phated reviewed Mar 3, 2024

View reviewed changes

alex-snezhko and others added 2 commits March 3, 2024 22:45

Adjust module attribute formatting

003f1d7

update comment test

49d44f7

phated approved these changes Mar 3, 2024

View reviewed changes

restore printing of comments with no module attributes

8069c43

ospencer enabled auto-merge March 3, 2024 23:23

ospencer added this pull request to the merge queue Mar 4, 2024

Merged via the queue into grain-lang:main with commit 72b2139 Mar 4, 2024
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(compiler)!: Remove arbitrary per-file compiler flags, add acceptable options as module attributes #1804

feat(compiler)!: Remove arbitrary per-file compiler flags, add acceptable options as module attributes #1804

alex-snezhko commented Apr 6, 2023 •

edited

spotandjake left a comment

phated left a comment

peblair left a comment

spotandjake left a comment

phated left a comment

phated left a comment

phated Feb 25, 2024

alex-snezhko Feb 26, 2024

phated Mar 2, 2024

phated Mar 2, 2024

alex-snezhko Mar 3, 2024

phated commented Feb 29, 2024

phated left a comment

phated Mar 3, 2024

phated Mar 3, 2024

alex-snezhko Mar 3, 2024

phated Mar 3, 2024

phated Mar 3, 2024

alex-snezhko Mar 3, 2024

phated Mar 3, 2024

alex-snezhko Mar 3, 2024

		enter_attribute: (attribute, attribute_context) => unit,
		leave_attribute: (attribute, attribute_context) => unit,

		enter_attribute: attribute => unit,
		leave_attribute: attribute => unit,

		@@ -1,3 +1,9 @@
		// Comment before first attr
		/* Block comment before attr */@noPervasives // Line comment on attr

feat(compiler)!: Remove arbitrary per-file compiler flags, add acceptable options as module attributes #1804

feat(compiler)!: Remove arbitrary per-file compiler flags, add acceptable options as module attributes #1804

Conversation

alex-snezhko commented Apr 6, 2023 • edited

spotandjake left a comment

Choose a reason for hiding this comment

phated left a comment

Choose a reason for hiding this comment

peblair left a comment

Choose a reason for hiding this comment

spotandjake left a comment

Choose a reason for hiding this comment

phated left a comment

Choose a reason for hiding this comment

phated left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phated commented Feb 29, 2024

phated left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alex-snezhko commented Apr 6, 2023 •

edited