erichkeane added a comment. In D133853#3795579 <https://reviews.llvm.org/D133853#3795579>, @aaron.ballman wrote:
> 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. I agree with Aaron here, there isn't much value in the "do nothing" attributes, I believe the 'unknown attribute' warning for them is superior to an ignored attribute. 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 'constepr keyword' replacement? ================ Comment at: clang/lib/Sema/SemaStmtAttr.cpp:296 + // Validation is in Sema::ActOnAttributedStmt(). + return ::new (S.Context) MSConstexprAttr(S.Context, A); +} ---------------- Typically we try to do the ::Create function that is generated for attributes, rather than placement new. I realize we are consistently inconsistent... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133853/new/ https://reviews.llvm.org/D133853 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
