[PATCH] D134475: Add C++11 attribute msvc::constexpr

[Richard Dzenis via Phabricator via cfe-commits]

RIscRIpt added a comment.

> TODO: I think, I'll need to read more about constexpr for functions in standard (and LLVM code), to add relevant restrictions here.

In the standard I was able to find the following paragraphs about `constexpr`:

1. Neither constructor nor destructor can be constexpr if the class has some virtual base class; this holds for `[[msvc::constexpr]]` in MSVC 14.33
2. `constexpr` is not a part of function signature, so same non-constexpr function cannot be defined. The same holds for C++11 attributes, and `[[msvc::constexpr]]` in MSVC 14.33

I will add two relevant tests in `msvc-attrs-invalid.cpp`

Regarding LLVM code, I made the following observations:

- `FunctionDecl::isConstexprSpecified()` is mostly used for printing code, in other words, to check that the function is `constexpr` (and not `consteval`).
- `FunctionDecl::getConstexprKind()` is used by:
- `ASTImporter.cpp` - seems that, it should take care of attributes independently from the `getConstexprKind()`
- `ASTWriterDecl.cpp` - seems that, it should take care of attributes independently from the `getConstexprKind()`
- `SemaDecl.cpp` / `SemaDeclCXX.cpp` - checks that re-declaration was not performed with different constexpr specifier. MSVC and clang with current implementation allows re-declaration with different attributes (there are tests with `f2-f5` in `msvc-attrs.cpp`)
- `SemaDeclCXX` - enables C++ constexpr constructor inheritance - problem - my current implementation supports it, whereas MSVC doesn't:

  // Check '[[msvc::constexpr]]' is not taken into account during constructor inheritance
  struct s2 {
      [[msvc::constexpr]] s2() {}
      constexpr operator bool() { return false; };
  };
  
  struct s3 : s2 {
      constexpr operator bool() { return true; };
  };
  static_assert(s3()); // MSVC: C2131: expression did not evaluate to a constant
  static_assert(s3()); // clang with my patch: OK

- `SemaTemplate.cpp` - enables C++ template re-specialization with different constexpr specifier, not a problem, because we are allowed to do the same with attributes (don't we?)
- `SemaTemplateInstantiateDecl.cpp` (`TemplateDeclInstantiator`) - enables C++ template instantiation with same constexpr specifier

  template<typename T>
  struct s1 {
      [[msvc::constexpr]] s1() {}
      constexpr operator bool() { return true; };
  };
  
  static_assert(s1<void>()); // MSVC: C2131: expression did not evaluate to a constant
  static_assert(s1<void>()); // clang with my patch: OK

- `TreeTransform.h` - code is related to lambdas - out-of-scope of the current patch.

Regarding `MSVC:C2131` I found one more problem:

  [[msvc::constexpr]] int C() { return 0; }
  constexpr int V = C(); // MSVC: C2131: expression did not evaluate to a constant
  constexpr int W = C(); // clang with my patch: OK

I am starting to question my patch - does LLVM really need it, if

1. There's no documentation for `[[msvc::constexpr]]`
2. The trivial implementation makes it an alias, but the semantic behavior is different - we could fix that, but there are lots of things to check

@aaron.ballman WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134475