================
Comment at: lib/Parse/ParsePragma.cpp:438
@@ +437,3 @@
+ BalancedDelimiterTracker Parens(*this, tok::l_paren, tok::eof);
+ PP.Lex(Tok); // pragma kind
+ // Figure out which #pragma we're dealing with. The switch has no default
----------------
It's strange to use `PP.Lex` here -- in the parser you should generally be
using `ConsumeToken` instead. If you're trying to avoid updating
`PrevTokLocation` (which seems like a good idea, since pragma tokens aren't
tokens in the usual sense), you're not doing enough, because
`BalancedDelimiterTracker` and `ParseStringLiteralExpression` will update it.
Maybe save and restore it on entry to/exit from this function, and use
`ConsumeToken` here?
================
Comment at: lib/Parse/ParsePragma.cpp:490-493
@@ +489,6 @@
+ }
+ SegmentName = cast<StringLiteral>(SegmentNameExpr.get());
+ // Setting section "" has no effect
+ if (SegmentName->getLength())
+ Action = (Sema::PragmaMsStackAction)(Action | Sema::PSK_Set);
+ }
----------------
This would fit better in the `ActOn...` functions. Generally, we try to avoid
the parser inspecting AST nodes.
================
Comment at: lib/Parse/ParsePragma.cpp:487
@@ +486,3 @@
+ if (SegmentNameExpr.isInvalid()) {
+ PP.Diag(PragmaLoc, diag::warn_pragma_expected_string) << PragmaName;
+ goto ERROR_RETURN;
----------------
In this case, a diagnostic will already have been produced. (Generally
speaking, when a function returns you an invalid `*Result` or a function
returning `bool` on error returns `true`, that indicates that it has already
produced a diagnostic.)
================
Comment at: lib/Parse/ParsePragma.cpp:485
@@ +484,3 @@
+ if (Tok.isNot(tok::r_paren)) {
+ ExprResult SegmentNameExpr = ParseStringLiteralExpression();
+ if (SegmentNameExpr.isInvalid()) {
----------------
Don't call this unless `isTokenStringLiteral()`, or it'll assert.
http://llvm-reviews.chandlerc.com/D3065
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
