You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Expressions.new([
Annotation.new(Path.new(["Bar"])),
Def.new("foo", [] of Arg),
] of ASTNode)
@straight-shoota suggested changing this in #9322 (comment) such that the annotation in this example would be stored within the Def node itself. Similar to how #12044 handled it for method/macro parameters. Which makes a lot more sense to me as well.
From an implementation perspective it's a bit tricky since unlike annotated parameters we do not necessarily know what specific node we're parsing until after the annotations have already been parsed. The best way I could think of is keeping track of the Annotation nodes within the parser itself, then apply them to an applicable node once we determine what it is, then clear the annotations array to prepare it for the next method/type/what have you.
Backwards compatibility also came up, which seems to suggest we would want to keep both APIs, meaning like we'd still represent the annotations as an Expressions, but also store a copy of them within the Def?
The text was updated successfully, but these errors were encountered:
$ ccrystal run --prelude=empty test.cr
name # => LibPCRE2
annotations # => [@[Link("pcre2-8", pkg_config: "libpcre2-8")]]
...
It seems we can't actually do this purely in the parser due to macro code like this not yet being evaluated to allow adding the second Link annotation to the LibDef node directly. The current implementation works I guess because by the time the semantic visitor runs, the macro has expanded which results in def visit(node : Annotation) being called twice properly populating @annotations with both. Whereas the ones I parsed directly and added to the LibDef only has the first one. And AFAIK we wouldn't really have a way to know what node the macro generated nodes relate to?
Maybe the current implementation is actually best then. It all happens in TopLevelVisitor: macro expressions are expanded and annotations are collected and attached to the following node.
From #9322 (comment):
Is currently parsed as:
@straight-shoota suggested changing this in #9322 (comment) such that the annotation in this example would be stored within the
Def
node itself. Similar to how #12044 handled it for method/macro parameters. Which makes a lot more sense to me as well.From an implementation perspective it's a bit tricky since unlike annotated parameters we do not necessarily know what specific node we're parsing until after the annotations have already been parsed. The best way I could think of is keeping track of the
Annotation
nodes within the parser itself, then apply them to an applicable node once we determine what it is, then clear the annotations array to prepare it for the next method/type/what have you.Backwards compatibility also came up, which seems to suggest we would want to keep both APIs, meaning like we'd still represent the annotations as an
Expressions
, but also store a copy of them within theDef
?The text was updated successfully, but these errors were encountered: