[PATCH] MS ABI: Add support for #pragma pointers_to_members

[Richard Smith]

================
Comment at: include/clang/Basic/DiagnosticParseKinds.td:807-808
@@ -804,1 +806,4 @@
+// - #pragma pointers_to_members
+def err_pragma_pointers_to_members_unknown_kind : Error<
+  "unknown kind of inheritance model">;
 
----------------
Maybe add "expected single_inheritance, multiple_inheritance, or virtual_inheritance"?

================
Comment at: lib/Parse/ParsePragma.cpp:820
@@ +819,3 @@
+  }
+  PP.Lex(Tok);
+  const IdentifierInfo *Arg = Tok.getIdentifierInfo();
----------------
Have you checked whether macro expansion should be performed on this token?

================
Comment at: lib/Parse/ParsePragma.cpp:837-840
@@ +836,6 @@
+    RepresentationMethod = Sema::PPTMK_FullGeneralityVirtualInheritance;
+  }
+
+  if (HaveRepresentationMethod &&
+      RepresentationMethod != Sema::PPTMK_BestCase) {
+    if (Tok.is(tok::comma)) {
----------------
Delete these four lines? =)

================
Comment at: lib/Parse/ParsePragma.cpp:857-858
@@ +856,4 @@
+
+  if (!HaveRepresentationMethod ||
+      RepresentationMethod != Sema::PPTMK_BestCase) {
+    if (Arg->isStr("single_inheritance")) {
----------------
Maybe fold this into the `if` above too?

================
Comment at: lib/Parse/ParsePragma.cpp:865
@@ +864,3 @@
+      RepresentationMethod = Sema::PPTMK_FullGeneralityVirtualInheritance;
+    } else if (Tok.isNot(tok::r_paren)) {
+      PP.Diag(Tok.getLocation(),
----------------
`Tok` can't be `tok::r_paren` here, because we already know it's got `IdentifierInfo`.

================
Comment at: lib/Parse/ParsePragma.cpp:888-890
@@ +887,5 @@
+
+  Token *Toks = (Token *)PP.getPreprocessorAllocator().Allocate(
+      sizeof(Token) * 1, llvm::alignOf<Token>());
+  new (Toks) Token();
+  Toks[0].startToken();
----------------
`Token *Tok = new (PP.getPreprocessorAllocator()) Token;`

================
Comment at: lib/Parse/ParsePragma.cpp:896-897
@@ +895,4 @@
+      reinterpret_cast<void *>(static_cast<uintptr_t>(RepresentationMethod)));
+  PP.EnterTokenStream(Toks, 1, /*DisableMacroExpansion=*/true,
+                      /*OwnsTokens=*/false);
+}
----------------
Can you use `EnterToken` instead of `EnterTokenStream` here?

================
Comment at: lib/Parse/Parser.cpp:112
@@ -111,1 +111,3 @@
   }
+  if (getLangOpts().MSVCCompat) {
+    MSPointersToMembers.reset(new PragmaMSPointersToMembers());
----------------
Why `MSVCCompat` rather than `MicrosoftExt`, or something based on whether we're using the MS ABI?

================
Comment at: lib/Sema/Sema.cpp:214-215
@@ -212,3 +213,4 @@
   if (VisContext) FreeVisContext();
   MSStructPragmaOn = false;
+  MSPointerToMemberRepresentationMethod = PPTMK_BestCase;
   // Kill all the active scopes.
----------------
Why are we messing with either of these in the `Sema` destructor?

================
Comment at: lib/Sema/SemaType.cpp:5109
@@ +5108,3 @@
+                    PPTMK_BestCase,
+                RD->getSourceRange()));
+          }
----------------
If the inheritance model came from a 'full generality' pragma, we should probably use the location of the pragma as the location of the implicit attribute.

================
Comment at: test/Misc/warning-flags.c:19
@@ -18,3 +18,3 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
----------------
Please add a `-W` flag covering your warning! Maybe add one `-W` flag for all these warn_pragma_blah warnings? `-Wignored-pragma`, to match `-Wignored-attribute`?

================
Comment at: test/SemaCXX/member-pointer-ms.cpp:206-209
@@ +205,6 @@
+
+#pragma pointers_to_members(full_generality, multiple_inheritance)
+struct TrulySingleInheritance;
+static_assert(sizeof(int TrulySingleInheritance::*) == kMultipleDataSize, "");
+#pragma pointers_to_members(best_case)
+// This definition shouldn't conflict with the increased generality that the
----------------
Seems you can only have one `#pragma pointers_to_members` per TU in MSVC land. You seem to have some complexity to deal with the possibility of the pragma changing (for instance, storing the `BestCase` flag on the attribute) that may be unnecessary?

================
Comment at: include/clang/Sema/Sema.h:273
@@ +272,3 @@
+  /// \brief Controls member pointer representation format under the MS ABI.
+  PragmaMSPointersToMembersKind MSPointerToMemberRepresentationMethod;
+
----------------
You seem to be missing serialization/deserialization code for this.

How are we going to handle this w/modules?


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