rsmith 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; + }; + ---------------- 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`?
================ Comment at: lib/Parse/ParseDeclCXX.cpp:1311 @@ +1310,3 @@ + struct PreserveIdentifierInfoRAII { + void set(IdentifierInfo *ToPreserve) { + II = ToPreserve; ---------------- Maybe use a constructor with a `bool Enabled` rather than a setter? ================ Comment at: lib/Parse/ParseDeclCXX.cpp:1327-1339 @@ +1326,15 @@ + + PreserveIdentifierInfoRAII AtomicTokenGuard; + if (getLangOpts().MSVCCompat) { + if (!Ident__Atomic) + Ident__Atomic = &PP.getIdentifierTable().get("_Atomic"); + AtomicTokenGuard.set(Ident__Atomic); + } + + if (getLangOpts().MSVCCompat && TagType == DeclSpec::TST_struct && + Tok.isNot(tok::identifier) && !Tok.isAnnotation() && + Tok.getIdentifierInfo() && Tok.is(tok::kw__Atomic)) { + Ident__Atomic->RevertTokenIDToIdentifier(); + Tok.setKind(tok::identifier); + } + ---------------- I think this can be simplified to: if (getLangOpts().MSVCCompat && Tok.is(tok::kw__Atomic) && TagType == DeclSpec::TST_struct) ================ Comment at: lib/Parse/ParseDeclCXX.cpp:1334 @@ +1333,3 @@ + + if (getLangOpts().MSVCCompat && TagType == DeclSpec::TST_struct && + Tok.isNot(tok::identifier) && !Tok.isAnnotation() && ---------------- 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`.) ================ 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; ---------------- You don't appear to have any testcases for this. ================ Comment at: test/SemaCXX/MicrosoftCompatibility.cpp:12-13 @@ -11,7 +11,4 @@ -#if _MSC_VER >= 1900 _Atomic(int) z; -#else struct _Atomic {}; ---------------- 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?). http://reviews.llvm.org/D11233 _______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits