majnemer added inline comments. ================ Comment at: lib/Parse/ParseDeclCXX.cpp:1310-1325 @@ -1309,1 +1309,18 @@ + struct PreserveIdentifierInfoRAII { + void set(IdentifierInfo *ToPreserve) { + II = ToPreserve; + TK = II->getTokenID(); + } + ~PreserveIdentifierInfoRAII() { + if (II) { + if (TK == tok::identifier && II->getTokenID() != tok::identifier) + II->RevertTokenIDToIdentifier(); + else if (TK != tok::identifier && II->getTokenID() == tok::identifier) + II->RevertIdentifierToTokenID(TK); + } + } + IdentifierInfo *II = nullptr; + tok::TokenKind TK; + }; + ---------------- rsmith wrote: > This seems a lot more general than you need: can you just make the `set` > function revert to identifier and the destructor revert to `tok::kw__Atomic`? If I unconditionally revert to `kw__Atomic`, I will not correctly handle cases where `ParseClassSpecifier` is called recursively. If this happens, the `_Atomic` identifier will have `getTokenID()` already set to `tok::identifier` and we should let the caller's RAII object revert it back to `kw__Atomic` instead.
================ Comment at: lib/Parse/ParseDeclCXX.cpp:1334 @@ +1333,3 @@ + + if (getLangOpts().MSVCCompat && TagType == DeclSpec::TST_struct && + Tok.isNot(tok::identifier) && !Tok.isAnnotation() && ---------------- rsmith wrote: > Please add a comment explaining why you're doing this. We conventionally use > a comment containing "HACK" for workarounds for standard library issues. > (Though we're missing that in the comment above, in the call to > `TryKeywordIdentFallback`.) Sure, done. ================ Comment at: lib/Parse/ParseDeclCXX.cpp:1892-1897 @@ -1860,2 +1891,8 @@ // Parse the class-name. + if (getLangOpts().MSVCCompat && Tok.isNot(tok::identifier) && + !Tok.isAnnotation() && Tok.getIdentifierInfo() && + Tok.is(tok::kw__Atomic)) { + Ident__Atomic->RevertTokenIDToIdentifier(); + Tok.setKind(tok::identifier); + } SourceLocation EndLocation; ---------------- rsmith wrote: > You don't appear to have any testcases for this. Fixed. ================ Comment at: test/SemaCXX/MicrosoftCompatibility.cpp:12-13 @@ -11,7 +11,4 @@ -#if _MSC_VER >= 1900 _Atomic(int) z; -#else struct _Atomic {}; ---------------- rsmith wrote: > Please add a testcase that uses `_Atomic` as a keyword after it's defined as > a type. Please also add a testcase showing the uses of the `_Atomic` type > that we need to be compatible with (as a base class, maybe?). Done. ================ Comment at: test/SemaCXX/MicrosoftCompatibility.cpp:13 @@ -13,5 +12,3 @@ _Atomic(int) z; -#else struct _Atomic {}; ---------------- rnk wrote: > Did you want to add a test for the base specifier case? Done. http://reviews.llvm.org/D11233 _______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits