[PATCH] D114439: [Annotation] Allow parameter pack expansions in annotate attribute

[Steffen Larsen via Phabricator via cfe-commits]

steffenlarsen marked an inline comment as done.
steffenlarsen added inline comments.


================
Comment at: clang/lib/Parse/ParseDecl.cpp:468
+
+      // General case. Parse all available expressions.
+      CommaLocsTy CommaLocs;
----------------
aaron.ballman wrote:
> I think it'd be good to move this comment up closer to the `else` on line 461 so that it's clear what the else clause covers (and more clear that this is the common case).
I completely agree. It has been moved to right after the `else`.


================
Comment at: clang/lib/Parse/ParseExpr.cpp:3374-3375
 
+    if (EarlyTypoCorrection)
+      Expr = Actions.CorrectDelayedTyposInExpr(Expr);
+
----------------
aaron.ballman wrote:
> steffenlarsen wrote:
> > aaron.ballman wrote:
> > > Not that I think this is a bad change, but it seems unrelated to the patch. Can you explain why you made the change?
> > Admittedly I am not sure why it is needed, but in the previous version of the parsing for attribute parameters it does the typo-correction immediately after parsing the expression and if this isn't added a handful of tests fail.
> Ah, thanks for the info! So this is basically doing the same work as what's done on line 3410 in the late failure case -- I wonder if this should actually be tied to the `FailImmediatelyOnInvalidExpr` parameter instead of having its own parameter?
They could be unified in the current version, but I don't think there is an actual relation between them.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114439/new/

https://reviews.llvm.org/D114439