majnemer added inline comments.
================
Comment at: lib/Parse/ParseDecl.cpp:2989
+
+ Diag(Loc, diag::err_ms_attributes_not_enabled);
+ continue;
----------------
aaron.ballman wrote:
> majnemer wrote:
> > aaron.ballman wrote:
> > > compnerd wrote:
> > > > aaron.ballman wrote:
> > > > > 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).
> > > > But we would reject that code under `-fdeclspec` anyways. I think
> > > > having the code be more portable is a bit nicer.
> > > After discussing in IRC, I decided that I agree with @compnerd on this
> > > and have changed the patch accordingly.
> > What if somebody wants to use __declspec and are using the compiler in a
> > freestanding mode? Also, we aren't the only member of the implementor's
> > namespace.
> Users using __declspec in a freestanding mode is a concern that I share.
> However, I imagine there are *far* more users who accidentally forget to pass
> `-fdeclspec` than there are users who are writing code in freestanding mode
> that wish to use `__declspec` as an identifier in a situation that proves
> problematic. That being said, do you think the approach in the patch would
> work with a warning rather than an error? I went with an error because I felt
> that a warning would require tentative parsing to be properly implemented,
> which felt like a heavy solution for a problem I didn't think anyone would
> run into in practice.
>
> I'm not overly sympathetic to others in the implementor's namespace who use
> `__declspec` but not to implement attributes under that name. However, I
> could be convinced to be sympathetic if there was some conflict in practice.
> Do you have a case in mind?
I suppose what you've got should be fine in practice. Heck, we used to have
__declspec attributes available all the time...
https://reviews.llvm.org/D29868
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
