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

[Warren Hunt]

================
Comment at: lib/Parse/ParsePragma.cpp:1313-1323
@@ +1312,13 @@
+                                  Token &Tok) {
+  Token AnnotTok;
+  AnnotTok.startToken();
+  AnnotTok.setKind(tok::annot_pragma_ms_pragma);
+  AnnotTok.setLocation(Tok.getLocation());
+  SmallVector<Token, 8> TokenVector;
+  TokenVector.push_back(AnnotTok);
+  for (; Tok.isNot(tok::eod); PP.Lex(Tok))
+    TokenVector.push_back(Tok);
+  Token *TokenArray = new Token[TokenVector.size()];
+  memcpy(TokenArray, TokenVector.data(), sizeof(Token)* TokenVector.size());
+  PP.EnterTokenStream(TokenArray, TokenVector.size(), true, true);
+}
----------------
Richard Smith wrote:
> Dropping the `eod` delimiter from the stream here looks questionable. You have no way to distinguish between
> 
>     #pragma code_seg(blah)
> 
> and
> 
>     #pragma code_seg
>     (blah)
> 
> Error recovery might also do strange things if the tokens from the pragma are in the normal token stream, since the rest of the parser isn't aware that `annot_pragma_ms_pragma` opens some kind of token scope. Maybe you could set the list of tokens as the annotation value on the `annot_pragma_ms_pragma` token instead?
Fixed.

================
Comment at: lib/Sema/SemaDecl.cpp:7498
@@ -7497,1 +7497,3 @@
 
+  if (/*NewFD->isFunctionDefinition() &&*/ CodeSegStack.Current)
+    NewFD->addAttr(MSSegmentAttr::CreateImplicit(
----------------
Richard Smith wrote:
> Commented-out code?
Moved to ActOnFunctionDeclarator to get the definitionness.

================
Comment at: lib/Parse/ParseDeclCXX.cpp:2618-2621
@@ -2617,1 +2617,6 @@
 
+      if (Tok.is(tok::annot_pragma_ms_pragma)) {
+        HandlePragmaMSPragma();
+        continue;
+      }
+
----------------
Richard Smith wrote:
> Do you really need to check for this in so many places? (Do none of them call the other ones?) ;(
Other patches that add pragmas touch each of these places.

================
Comment at: include/clang/Sema/Sema.h:305-306
@@ +304,4 @@
+      : Current(0) {}
+    typedef std::pair<llvm::StringRef,
+                      std::pair<StringLiteral *, SourceLocation>> Slot;
+    SmallVector<Slot, 2> Stack;
----------------
Richard Smith wrote:
> I can't guess what these three fields are. Maybe this should be a struct?
Done.

================
Comment at: include/clang/Sema/Sema.h:311-314
@@ +310,6 @@
+  };
+  MSSegmentStack DataSegStack;
+  MSSegmentStack BSSSegStack;
+  MSSegmentStack ConstSegStack;
+  MSSegmentStack CodeSegStack;
+
----------------
Richard Smith wrote:
> We should serialize/deserialize these if they occur in a PCH (but we shouldn't do so if they're in a module). I don't think anyone is particularly likely to care about this, but please at least add a FIXME.
FIXME added.


http://llvm-reviews.chandlerc.com/D3065