[PATCH] D57196: Fix error in Visual Studio due to __has_cpp_attribute

[Aaron Ballman via Phabricator via llvm-commits]

aaron.ballman added a comment.

In D57196#1374340 <https://reviews.llvm.org/D57196#1374340>, @haozhun wrote:

> Thank you for your review.
>
> > This is a Visual Studio bug with their IntelliSense implementation. http://eel.is/c++draft/cpp.cond#5 says the pp-tokens are to be interpreted as an attribute-token, which includes attribute-scoped-tokens.
>
> Thank you for pointing this out and providing citations. I understand that Windows is  a supported platform of LLVM. As a result, I suppose it doesn't matter whose fault this is.


Sort of. We should make sure the issue gets reported to Microsoft, and if it causes us tangible pain, we could work around it locally.

>> IntelliSense squiggly lines often do not match the compiler behavior (IntelliSense uses a different frontend than cl, from what I understand); why do we need to work around it here? I guess I don't understand what problem this is solving given that the compilation succeeds.
> 
> If there isn't already a policy/consensus on whether IntelliSense errors should be avoided (especially in header files), it would be a judgement call.

To the best of my knowledge, no one in the project worries about IntelliSense issues. I'm not certain we've had an explicit policy decision on this, but that's unsurprising given that IntelliSense is not required to build the project.

> However, I would expect that the choice had been made in the past. If IntelliSense misbehavior are indeed "often" (as you said), and given that there aren't other IntelliSense complaints of header files in the code base (as far as I can tell), it is likely that a choice had been made in the past that IntelliSense errors should be avoided.

I think there aren't complaints about it because it doesn't have any impact other than putting up red squiggles where it shouldn't (or not having red squiggles where it should) in a text editor. I use the Visual Studio editor all day every day and do not find the IntelliSense issues to be anything more than a very minor distraction.

MSVC *does* support attribute namespaces in their implementation of `__has_cpp_attribute` (the compiler will parse and appropriately handle it); this is purely an IntelliSense thing, which is a cosmetic editor issue. The proposed changes disallow the macro from gracefully upgrading should Microsoft choose to implement any of the vendor attributes we use in some future version of MSVC. While I don't think that's a likely scenario for the attributes we currently use, I also don't think working around IntelliSense bugs is worth the effort unless there is some appreciable impact from the bug fix.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57196/new/

https://reviews.llvm.org/D57196