Method/macro parameter annotation support

[Blacksmoke16]

Resolves #12039

• Includes support for annotating macro/method parameters
  • Parsing, reading, and rendering them on to_s
  • There isn't actually a way to access annotations on macros, or anything about them for that matter, so I didn't write tests for that. However it does parse/render correctly.

Based on #9322 (comment) i took a bit of a diff approach and stored the parsed annotation instances within the Arg node itself. Ended up making the implementation much easier and figured we wouldn't have to worry about breaking compatibility since this is a new feature.

[Blacksmoke16]

Related: #5334

[Blacksmoke16]

Seems something is broken with to_s.cr when expanding macros. The Def in record_spec.cr is being generated as like def initialize(, __id id : Int32), even tho it shows up correctly in the test output?

I'll see if I can figure out what's wrong tomorrow.

EDIT: Seems a single * arg doesnt ever call the visit(node : Arg) overload?
EDIT2: Figured it out I think. semantic/to_s.cr redefines that method 🙃, so the one in syntax/to_s.cr doesn't actually do anything, which broke things because it was no longer adding the * in the Def visitor method

[Blacksmoke16]

Bump. Another review on this would be much appreciated 🙂.

[asterite]

How's the memory consumption after this PR for, say, compiling the compiler? You can check this with "-s"

[Blacksmoke16]

@asterite

On this branch:

make clean && make stats=1 progress=1

...

Parse:                             00:00:00.000038766 (   1.08MB)
Semantic (top level):              00:00:00.267053231 ( 129.96MB)
Semantic (new):                    00:00:00.001583005 ( 129.96MB)
Semantic (type declarations):      00:00:00.030205549 ( 137.96MB)
Semantic (abstract def check):     00:00:00.024382963 ( 137.96MB)
Semantic (ivars initializers):     00:00:04.060857175 (1037.90MB)
Semantic (cvars initializers):     00:00:00.008242924 (1045.90MB)
Semantic (main):                   00:00:00.850217616 (1213.90MB)
Semantic (cleanup):                00:00:00.000485844 (1213.90MB)
Semantic (recursive struct check): 00:00:00.000968438 (1213.90MB)
Codegen (crystal):                 00:00:04.155784348 (1497.90MB)
Codegen (bc+obj):                  00:00:00.616157028 (1505.90MB)
Codegen (linking):                 00:00:02.735330050 (1505.90MB)

On the commit I branched off of:

make clean && make stats=1 progress=1

...

Parse:                             00:00:00.000033139 (   1.08MB)
Semantic (top level):              00:00:00.270554537 ( 129.96MB)
Semantic (new):                    00:00:00.001891485 ( 129.96MB)
Semantic (type declarations):      00:00:00.029909324 ( 137.96MB)
Semantic (abstract def check):     00:00:00.024808537 ( 137.96MB)
Semantic (ivars initializers):     00:00:04.090095406 (1037.90MB)
Semantic (cvars initializers):     00:00:00.008378645 (1045.90MB)
Semantic (main):                   00:00:00.846730859 (1213.90MB)
Semantic (cleanup):                00:00:00.000431342 (1213.90MB)
Semantic (recursive struct check): 00:00:00.001096690 (1213.90MB)
Codegen (crystal):                 00:00:04.213905891 (1497.90MB)
Codegen (bc+obj):                  00:00:06.765188742 (1497.90MB)
Codegen (linking):                 00:00:02.735254769 (1497.90MB)

So seems pretty much the same?

[asterite]

Interesting! Since we are adding something to every Arg I thought the memory was going to be way up... but no 🤷 😄

[asterite]

Instead of keeping the annotations on each arg, I think we can move this to the Def and Macro objects, similar to how the splat index is kept there.

But we could do this refactor later on if we see memory issues. I just don't think that annotations on args will be that common, and so adding this overhead to every arg instead of adding just a nilable attribute to Def and Macro might be better.

[asterite]

Looks good!