================
Comment at: lib/Parse/ParsePragma.cpp:820
@@ +819,3 @@
+ }
+ PP.Lex(Tok);
+ const IdentifierInfo *Arg = Tok.getIdentifierInfo();
----------------
Richard Smith wrote:
> 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
>
> ?
MSVC thinks this is perfectly cromulent.
================
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.
----------------
Richard Smith wrote:
> "precedes"
Will do.
================
Comment at: lib/Parse/Parser.cpp:112
@@ -111,1 +111,3 @@
}
+ if (getLangOpts().MSVCCompat) {
+ MSPointersToMembers.reset(new PragmaMSPointersToMembers());
----------------
Richard Smith wrote:
> 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.)
My opinion has swayed, I think it makes sense for it to be `MicrosoftExt`.
================
Comment at: lib/Parse/ParsePragma.cpp:857-858
@@ +856,4 @@
+
+ if (!HaveRepresentationMethod ||
+ RepresentationMethod != Sema::PPTMK_BestCase) {
+ if (Arg->isStr("single_inheritance")) {
----------------
Richard Smith wrote:
> 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.
I'll try to move it.
http://llvm-reviews.chandlerc.com/D2723
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
