[PATCH] D133853: [AST] Add msvc-specific C++11 attributes

Fri Sep 16 08:10:09 PDT 2022

aaron.ballman added a reviewer: erichkeane.
aaron.ballman added a subscriber: erichkeane.
aaron.ballman added a comment.

In D133853#3793284 <https://reviews.llvm.org/D133853#3793284>, @RIscRIpt wrote:

> In D133853#3792518 <https://reviews.llvm.org/D133853#3792518>, @aaron.ballman wrote:
>
>> I'm wondering what the goal is for these changes. ... Are you intending to add semantics for these attributes in follow-up patches?
>
> To be honest, I wasn't planning to do any of follow-up patches. I made a patch for internal usage at my job, and decided to submit it upstream.
> The main reason I (we) need this patch is that we need to be able to parse MSVC-specific code (in the current case - detect `constexpr` functions). Since Visual Studio 17.3 (MSVC 14.33.31629), Microsoft's STL library added `[[msvc::constexpr]]` attribute, which is not documented yet, but makes a function to act like a `constexpr` function: see this godbolt sample <https://godbolt.org/z/76fYq145d> (i.e. forbids non-constexpr statements inside).
>
> To make the patch complete, I decided to browse previous Microsoft's STL versions and see which vendor specific (`msvc::`) attributes they added previously; in this patch I added all attributes I was able to find.
>
>> We don't typically add attributes to Clang that don't have any effect unless there's a very compelling reason to do so.
>
> Theoretically, I could re-submit (or adjust this) patch, which would add support for `[[msvc::constexpr]]` attribute with semantic meaning of `constexpr` for functions (for code parsed with `-fms-extensions` flag). Regarding other attributes - unfortunately they are either poorly documented, or not documented at all, so I can drop commits for these attributes.
>
> *Edit:* Please, let me know how we can proceed - either I abandon the patch; or add support only for `[[msvc::constexpr]]`, or any other way of proceeding further?

Thanks for the information! Roping in @erichkeane as attribute code owner to make sure he's on board with the idea, but my suggestion is to only support `[[msvc::constexpr]]` with the semantic meaning of `constexpr`. It's a good question as to whether we want to support that only when passing `-fms-extensions` or not (it seems like a generally useful attribute); we don't do the same for GNU attributes, but maybe we don't want to follow that pattern? This will be the first attribute we add with the `msvc` vendor namespace.

If you find there's a good reason to upstream the other ones, we can certainly consider it. FWIW, one Microsoft-specific attribute I know people have been asking about is `[[msvc::no_unique_address]]`. See https://github.com/llvm/llvm-project/issues/49358 for more details. Not suggesting you're on the hook for that or anything, just pointing it out in case you wanted to work on that one at some point.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133853