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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
