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

Sat Sep 17 06:34:04 PDT 2022

RIscRIpt added a comment.

In D133853#3795579 <https://reviews.llvm.org/D133853#3795579>, @aaron.ballman wrote:

> but my suggestion is to only support `[[msvc::constexpr]]` with the semantic meaning of `constexpr`

Sounds good to me.

> 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.

IMO, this attribute is a clearly Microsoft's extension, thus it should be available only when `-fms-extensions` are enabled.

> If you find there's a good reason to upstream the other ones, we can certainly consider it.

I don't see any good reason for other attributes to be added.

> FWIW, one Microsoft-specific attribute I know people have been asking about is `[[msvc::no_unique_address]]`

Thanks for letting me know.

The current patch is my first attempt to contribute to LLVM; I am afraid it would take me some effort to implement semantics of `[[msvc::no_unique_address]]`, so I'd like to focus only on `[[msvc::constexpr]]` in current patch.

In D133853#3795598 <https://reviews.llvm.org/D133853#3795598>, @erichkeane wrote:

> A functional `msvc::constexpr` would be acceptable, however, I'm having a difficult time figuring out the purpose of it, it seems like it is just a 'constexpr keyword' replacement?

I don't know how it works internally, but judging by its behavior - yes it's a `constexpr` replacement with some extended functionality (my guess is that they wanted to do some constexpr magic, and staying compatible with the standard at the same time), and there's a few information available online:

> Merged an MSVC-specific attribute, allowing the compiler to improve its handling of constexpr dynamic allocation and constexpr unique_ptr. [1]

>   // Determine if we should use [[msvc::constexpr]] to allow for "extended constexpr" in Visual C++. [2]

Browsing MSVC `14.33.31629` include headers I can see that this attribute is mostly used in `operator new` functions: either as a decorator of the function or the return statement.

I guess the best way to make this attribute compatible with MSVC would be ask someone at Microsoft, but I'm not sure who to reach out.

Meanwhile I'll adjust current patch to focus only `[[msvc::constexpr]]` attribute.

> I believe the 'unknown attribute' warning for them is superior to an ignored attribute.

By the way, I'd like to take opportunity and ask what do you think of adding clang-specific compiler flag, which would add all unknown attributes to the AST in a string form (e.g. `[[attr1]] -> AST::UnknownAttr{"attr1"}`? Would you be interested in such patch?

[1] https://github.com/microsoft/STL/wiki/Changelog

[2] `14.33.31629\include\vcruntime.h`

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133853