[PATCH] Implementation of #pragma (data|code|const|bss)_seg

[Richard Smith]

================
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