[PATCH] D11233: [MS Compat] Allow _Atomic(Type) and 'struct _Atomic' to coexist

[David Majnemer]

majnemer added inline comments.

================
Comment at: lib/Parse/ParseDeclCXX.cpp:1310-1325
@@ -1309,1 +1309,18 @@
 
+  struct PreserveIdentifierInfoRAII {
+    void set(IdentifierInfo *AtomicII) {
+      II = AtomicII;
+      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:
> Oh, I see what you're doing; this also restores the change to the `IdentifierInfo`'s kind when parsing the base specifier. I'm not really happy with that approach. Saving and restoring the kind of the `_Atomic` identifier while parsing `struct _Atomic` seems OK; doing it when parsing every single struct does not seem targeted enough.
> 
> Instead, I suggest the following:
> 
>  * Only change the type of `_Atomic` within the definition of `struct _Atomic`
>  * When parsing a base specifier and you see `_Atomic`, change just that one token to an identifier
>  * When parsing a declaration and you see `_Atomic`, check whether (a) you're in a `typedef`, and (b) you've not got a type yet, and (c) the token after the next token is a semicolon, and if so, convert that `_Atomic` token to an identifier
I can implement your suggestion but I don't think it will work without some tweaks.  What I was accomplishing with my aggressive handling of `_Atomic` was getting ` typedef _Atomic<_Ty, sizeof (_Ty)> _My_base;` handled "correctly".

The next token after `_Atomic` in that typedef is not a semicolon, it is a `tok::less`.

If we are going with this implementation technique, I don't think I even need `Ident__Atomic` anymore.

================
Comment at: lib/Parse/ParseDeclCXX.cpp:1896-1898
@@ -1860,1 +1895,5 @@
   // Parse the class-name.
+  if (getLangOpts().MSVCCompat && Tok.isNot(tok::identifier) &&
+      !Tok.isAnnotation() && Tok.getIdentifierInfo() &&
+      Tok.is(tok::kw__Atomic)) {
+    Ident__Atomic->RevertTokenIDToIdentifier();
----------------
rsmith wrote:
> This can be simplified in the same way as the previous check.
Sure.

================
Comment at: lib/Parse/ParseDeclCXX.cpp:1899
@@ +1898,3 @@
+      Tok.is(tok::kw__Atomic)) {
+    Ident__Atomic->RevertTokenIDToIdentifier();
+    Tok.setKind(tok::identifier);
----------------
rsmith wrote:
> This could be null. But you can just grab the identifier info from `Tok` (both here and above).
Sure.


http://reviews.llvm.org/D11233