Don't issue warning 53 if the compiler is stopping early

[ccasin]

While reviewing #13170 , I noticed yet another issue with the new warning 53 implementation. If the compiler is stopped before lambda (for example, because -i or -stop-after typing is passed), we can't issue warning 53 accurately - the compiler hasn't had a chance to consume all the attributes yet. As a result, when those flags are passed, we end up issuing warning 53 for attributes that are in fact valid.

This PR is a fix for that unfortunate behavior. I've gone with a somewhat heavy-handed approach, just checking in warn_unused whether one of those flags is passed. (Perhaps there are other similar flags I've missed.)

I'm not sure this is the best approach, and am happy to hear alternative suggestions. We could instead try to move the call to warn_unused to a place where it will only be reached if the relevant flags are not passed. I couldn't immediately spot a good place. (Plus, there are actually two calls to warn_unused, and I'm likely to add a third in response to #13155, so perhaps it's nice to keep the logic inside of it).

[Octachron]

Did you have a look at how much efforts it would take to classify the builtin attributes by compiler passes and only emits the warning for misplaced attributes if the compiler has reached the corresponding pass?
With this design I would expect that switching the type of warn_unused to stopped_at:Compiler_pass.t -> last:Compiler_pass.t -> unit could work?

[ccasin]

Did you have a look at how much efforts it would take to classify the builtin attributes by compiler passes and only emits the warning for misplaced attributes if the compiler has reached the corresponding pass? With this design I would expect that switching the type of warn_unused to stopped_at:Compiler_pass.t -> last:Compiler_pass.t -> unit could work?

I did consider this, but my instinct was that it wasn't worth the complexity/maintanence burden of keeping the classification up to date, considering the limited utility of these flags - I would be surprised if anyone is too sad they don't get warning 53 when using them. But maybe I'm underestimating this?

[Octachron]

We don't add new builtin attributes often, so I am not worried about the maintenance burden of the classification.

Nevertheless, I agree that I don't expect people to complain much if the misplaced-attribute warning doesn't work with -i. Even if there are few scenarii where it could be informative, for instance in order to detect a misplaced [@@immediate] when using -i to build a signature.

[Octachron]

I would propose to update the documentation of warn_unused to mention that the warnings are only emitted if the compilation is not stopped early.

[ccasin]

Good point! Added.

[Octachron]

I agree that avoiding false positives with the misplaced-attribute warning when the compiler stops early is a good compromise.

Thanks for the fix !