aaron.ballman added inline comments.

================
Comment at: lib/Parse/ParseDecl.cpp:2973
+      // recognize that scenario and recover gracefully.
+      if (!getLangOpts().MicrosoftExt && Tok.is(tok::identifier) &&
+          Tok.getIdentifierInfo()->getName().equals("__declspec")) {
----------------
compnerd wrote:
> Shouldn't this be `getLangOpts().DeclSpecKeyword` since you don't need the 
> Microsoft extensions as much as you need the declspec keyword to be 
> processed.  Having `MicrosoftExt` implicitly enable `DeclSpecKeyword` (just 
> as borland mode enables it as well).  I suppose that it would fail anyways as 
> `Tok.is(tok::identifier)` would fail.
That's a good point, I've corrected.


================
Comment at: lib/Parse/ParseDecl.cpp:2989
+
+          Diag(Loc, diag::err_ms_attributes_not_enabled);
+          continue;
----------------
compnerd wrote:
> I think that we want to emit the diagnostic even if there is no parenthesis 
> as `__declspec` is a reserved identifier, and we would normally diagnose the 
> bad `__declspec` (expected '(' after '__declspec').
Yes, but it could also lead to a rejecting code that we used to accept and 
properly handle when __declspec is an identifier rather than a keyword. e.g.,
```
struct __declspec {};

__declspec some_func(void);
```
By looking for the paren, we run less risk of breaking working code, even if 
that code abuses the implementation namespace (after all, __declspec it not a 
keyword in this scenario).


https://reviews.llvm.org/D29868



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to