Consider changing shadowing rules for macro generated code about language

[jakemac53]

cc @munificent @leafpetersen thoughts?

[jakemac53]

This also makes macro augmentations libraries behave identically to, and have the same capabilities as, user written augmentation libraries.

[jakemac53]

There is an issue here related to annotation evaluation. We currently say this:

Because macros are not allowed to generate code that shadows an identifier
in the same library, we know that if an annotation class or any arguments to it
could be resolved, then we can assume that resolution is correct.

This allows us to provide an API for macro authors to attempt to evaluate an
annotation in _any phase_. The API may fail (if it requires more macro
expansion to be done), but that is not expected to be a common situation. In
the case where it does fail, users should typically be able to move some of
their code to a separate library (which they import). Then things from that
library can safely be used in annotations in the current library, and evaluated
by macros.

Which would no longer be valid.

We could expose annotations in a more similar way to how we expose other things, and not allow this premature expansion. In the types phase you would just get an identifier for the thing the annotation references (could be either a const constructor or variable reference), and possibly access to a code object for its arguments if it is a constructor call. Just like other identifiers all you could get from it in the first phase would be the String name of the identifier. We wouldn't be able to let you resolve the annotations identifier until the 3rd phase.

Note that even in the 3rd phase we might still not be able to fully evaluate the arguments to the annotation unless there are no references to other objects.

[scheglov]

Other possible approach is to prevent already observed identifiers to change their meaning. The meaning of "observed" can be either explicit, e.g. whether a macro generator actually asked for resolution of an annotation, or implicit, e.g. whether an identifier was used as a type annotation, that we resolved after the types phase, but then shadowed by declaring a top-level function (the naming convention does not like it, but technically possible). So, these observed identifiers should be re-checked, but not => x body that we initially talked about - nobody saw it, or even could see it, so we accept that the users intention was to use whatever it happens at the end. The key is consistency throughout phases.

[jakemac53]

Other possible approach is to prevent already observed identifiers to change their meaning. The meaning of "observed" can be either explicit, e.g. whether a macro generator actually asked for resolution of an annotation, or implicit, e.g. whether an identifier was used as a type annotation, that we resolved after the types phase, but then shadowed by declaring a top-level function (the naming convention does not like it, but technically possible). So, these observed identifiers should be re-checked, but not => x body that we initially talked about - nobody saw it, or even could see it, so we accept that the users intention was to use whatever it happens at the end. The key is consistency throughout phases.

Anything declared in the declarations phase would not be a valid type annotation, so while we could have resolved something incorrectly earlier, it would never result in valid code (unless we had special rules for the resolution just for macro generated code, which we don't want). It does imply some sort of check in the declarations phase that you didn't shadow a type that was used as a type annotation though.

I do definitely see the appeal of just not allowing "observed" identifiers to change their meaning, I just worry about the cost/complexity of actually enforcing that.

[rrousselGit]

I personally really dislike the way shadowing works at the moment.
Macros or not, I don't like that top-level symbols can take over inherited members. I've inadvertently broke apps before by exporting consts in my packages named widget, which clashes with State.widget.

I'd be in favour of changing it everywhere, not just for macros. But I assume that's not gonna happen due to being breaking.

So I'm a bit on the fence about having macros behave differently. While I would prefer a different behavior, I don't like the idea of having a difference just for macros. That increases the learning curve.