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

[Richard Smith]

================
Comment at: lib/Parse/ParsePragma.cpp:820
@@ +819,3 @@
+  }
+  PP.Lex(Tok);
+  const IdentifierInfo *Arg = Tok.getIdentifierInfo();
----------------
David Majnemer wrote:
> Richard Smith wrote:
> > Have you checked whether macro expansion should be performed on this token?
> Yes, the following is supposed to work:
> 
>   #define X best_case
>   #pragma pointers_to_members(X)
Crumbs. What about this:

    #define X full_generality, single_inheritance
    #pragma pointers_to_members(X)

... and ...

    #define X (best_case)
    #pragma pointers_to_members X

?

================
Comment at: lib/Parse/ParsePragma.cpp:857-858
@@ +856,4 @@
+
+  if (!HaveRepresentationMethod ||
+      RepresentationMethod != Sema::PPTMK_BestCase) {
+    if (Arg->isStr("single_inheritance")) {
----------------
David Majnemer wrote:
> Richard Smith wrote:
> > Maybe fold this into the `if` above too?
> This `if` handles the cases where we don't have a representation method or the representation method is full generality.
> 
> The above `if` only handles the full generality case.
Ah, the 'full_generality' is optional if there's an inheritance model. I think this'd be clearer if this code is part of the `else` for 'best_case'. Also, a comment describing the grammar for this pragma would help a lot.

================
Comment at: lib/Parse/Parser.cpp:112
@@ -111,1 +111,3 @@
   }
+  if (getLangOpts().MSVCCompat) {
+    MSPointersToMembers.reset(new PragmaMSPointersToMembers());
----------------
David Majnemer wrote:
> Richard Smith wrote:
> > Why `MSVCCompat` rather than `MicrosoftExt`, or something based on whether we're using the MS ABI?
> I felt that it is a non-conforming extension. I can key it off of the ABI if you think that is more appropriate.
Well, it's automatically conforming since it's a non-standard pragma. Choosing between `MSVCCompat` and `MicrosoftExt`, I think the only reason we'd have to pick the former is that we think this is a heinous extension (which, to be fair, it is, but we support the inheritance attribute keywords in `MicrosoftExt`).

I think the question is, does it make sense to have this pragma (and the keywords, for that matter) when building for the MS ABI with MS extensions disabled? Is that even a mode we care about?

(I'm happy with doing the most-restrictive thing for now and putting this under `MSVCCompat`, but I'm not sure that's the right long-term approach, and I'd like for us to have a more principled decision-making process here.)

================
Comment at: test/SemaCXX/member-pointer-ms.cpp:214
@@ +213,3 @@
+
+// Even if a definition proceedes the first mention of a pointer to member, we
+// still give the record the fully general representation.
----------------
"precedes"


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