Skip to content

[clang-cl] Add support for [[msvc::constexpr]] C++11 attribute #71300

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Dec 9, 2023

Conversation

RIscRIpt
Copy link
Member

@RIscRIpt RIscRIpt commented Nov 4, 2023

As per agreement with @AaronBallman and @erichkeane, I am re-opening this patch here. Migrated from differential revision: https://reviews.llvm.org/D134475

The current version of the patch has undergone numerous revisions before it became the version I am sending to GitHub.

Below you can find a summary of the history of changes.
The full history of all comments (excluding code-inline comments) is available in this file:
msvc-constexpr-D134475-comments.md


Initially I submitted this patch to make [[msvc::constexpr]] visible in the AST, with no semantics, and no tests at all. @AaronBallman has told me that you typically don't accept such patches, so I started figuring out semantics of [[msvc::constexpr]].

At first I tried to kind of reverse-engineer it myself, I made several observations, some of them were incorrect.

Later, @cpplearner noted:

It appears that [[msvc::constexpr]] does not make a function constexpr, but if [[msvc::constexpr]] is used in a function definition and in a call to that function, then the annotated function call can be evaluated during constant evaluation: https://godbolt.org/z/3MPTsz6Yn

Apparently this is used to implement constexpr std::construct_at, which needs to call placement operator new, but the latter is not constexpr.

Additionally I asked MSVC team to comment about [[msvc::constexpr]]. Cody Miller from Microsoft has shared semantics of [[msvc::constexpr]] here.

Further testing and experiments revealed that, unfortunately not all of his points were correct.

After some trials and errors, I came up with the following definition of semantics for [[msvc::constexpr]]

The `[[msvc::constexpr]]` attribute can be applied only to a function definition or a `return` statement.
It does not impact function declarations.
A `[[msvc::constexpr]]` function cannot be `constexpr` or `consteval`.
A `[[msvc::constexpr]]` function is treated in the same way as a `constexpr` function
if it is evaluated in a constant context of `[[msvc::constexpr]] return` statement.
Otherwise, it is treated as a regular function.

This doc-note was added to MSConstexprDocs.

As per this definition, we need to be able to track whether current invocation of [[msvc::constexpr]] function comes from [[msvc::constexpr]] return statement, which in turn comes from a constant evaluation context (i.e. a constexpr function).
Thus I went with the approach, which I saw being used in some other cases: a RAII helper (MSConstexprContextRAII), and a bool flag in CallStackFrame.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Nov 4, 2023
@llvmbot
Copy link
Member

llvmbot commented Nov 4, 2023

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Richard Dzenis (RIscRIpt)

Changes

As per agreement with @AaronBallman and @erichkeane, I am re-opening this patch here.

The current version of the patch has undergone numerous revisions before it became the version I am sending to GitHub.

Below you can find a summary of the history of changes.
The full history of all comments (excluding code-inline comments) is available in this file:
msvc-constexpr-D134475-comments.md


Initially I submitted this patch to make [[msvc::constexpr]] visible in the AST, with no semantics, and no tests at all. @AaronBallman has told me that you typically don't accept such patches, so I started figuring out semantics of [[msvc::constexpr]].

At first I tried to kind of reverse-engineer it myself, I made several observations, some of them were incorrect.

Later, @cpplearner noted:
> It appears that [[msvc::constexpr]] does not make a function constexpr, but if [[msvc::constexpr]] is used in a function definition and in a call to that function, then the annotated function call can be evaluated during constant evaluation: https://godbolt.org/z/3MPTsz6Yn
>
> Apparently this is used to implement constexpr std::construct_at, which needs to call placement operator new, but the latter is not constexpr.

Additionally I asked MSVC team to comment about [[msvc::constexpr]]. Cody Miller from Microsoft has shared semantics of [[msvc::constexpr]] here.

Further testing and experiments revealed that, unfortunately not all of his points were correct.

After some trials and errors, I came up with the following definition of semantics for [[msvc::constexpr]]

The `[[msvc::constexpr]]` attribute can be applied only to a function definition or a `return` statement.
It does not impact function declarations.
A `[[msvc::constexpr]]` function cannot be `constexpr` or `consteval`.
A `[[msvc::constexpr]]` function is treated in the same way as a `constexpr` function
if it is evaluated in a constant context of `[[msvc::constexpr]] return` statement.
Otherwise, it is treated as a regular function.

This doc-note was added to MSConstexprDocs.

As per this definition, we need to be able to track whether current invocation of [[msvc::constexpr]] function comes from [[msvc::constexpr]] return statement, which in turn comes from a constant evaluation context (i.e. a constexpr function).
Thus I went with the approach, which I saw being used in some other cases: a RAII helper (MSConstexprContextRAII), and a bool flag in CallStackFrame.


Patch is 23.88 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/71300.diff

19 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+2)
  • (modified) clang/docs/UsersManual.rst (+2-2)
  • (modified) clang/include/clang/Basic/Attr.td (+8)
  • (modified) clang/include/clang/Basic/AttrDocs.td (+16)
  • (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+8-2)
  • (modified) clang/include/clang/Basic/LangOptions.h (+1)
  • (modified) clang/lib/AST/ExprConstant.cpp (+28-6)
  • (modified) clang/lib/Basic/Targets/OSTargets.cpp (+3)
  • (modified) clang/lib/Driver/ToolChains/MSVC.cpp (+2-2)
  • (modified) clang/lib/Sema/SemaDecl.cpp (+3-1)
  • (modified) clang/lib/Sema/SemaDeclAttr.cpp (+24)
  • (modified) clang/lib/Sema/SemaDeclCXX.cpp (+4-1)
  • (modified) clang/lib/Sema/SemaStmtAttr.cpp (+16)
  • (added) clang/test/AST/ms-constexpr.cpp (+28)
  • (modified) clang/test/Driver/cl-options.c (+2-2)
  • (modified) clang/test/Misc/pragma-attribute-supported-attributes-list.test (+1)
  • (added) clang/test/SemaCXX/ms-constexpr-invalid.cpp (+52)
  • (added) clang/test/SemaCXX/ms-constexpr-new.cpp (+16)
  • (added) clang/test/SemaCXX/ms-constexpr.cpp (+37)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index afe7e2e79c2d087..ed0bfaf67aee449 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -211,6 +211,8 @@ C23 Feature Support
 
 Non-comprehensive list of changes in this release
 -------------------------------------------------
+- The default value of `_MSC_VER` was raised from 1920 to 1933.
+  MSVC 19.33 added undocumented attribute ``[[msvc::constexpr]]``.
 
 * Clang now has a ``__builtin_vectorelements()`` function that determines the number of elements in a vector.
   For fixed-sized vectors, e.g., defined via ``__attribute__((vector_size(N)))`` or ARM NEON's vector types
diff --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
index edc2bce6a964dc4..b849464e6dc3ca6 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -3333,8 +3333,8 @@ default for Windows targets.
 
 For compatibility with existing code that compiles with MSVC, clang defines the
 ``_MSC_VER`` and ``_MSC_FULL_VER`` macros. When on Windows, these default to
-either the same value as the currently installed version of cl.exe, or ``1920``
-and ``192000000`` (respectively). The ``-fms-compatibility-version=`` flag
+either the same value as the currently installed version of cl.exe, or ``1933``
+and ``193300000`` (respectively). The ``-fms-compatibility-version=`` flag
 overrides these values.  It accepts a dotted version tuple, such as 19.00.23506.
 Changing the MSVC compatibility version makes clang behave more like that
 version of MSVC. For example, ``-fms-compatibility-version=19`` will enable
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 60b549999c155e5..771ac5575e25d8c 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -3602,6 +3602,14 @@ def : MutualExclusions<[Owner, Pointer]>;
 
 // Microsoft-related attributes
 
+def MSConstexpr : InheritableAttr {
+  let LangOpts = [MicrosoftExt];
+  let Spellings = [CXX11<"msvc", "constexpr">];
+  let Subjects = SubjectList<[Function, Stmt], ErrorDiag,
+                             "functions and statements">;
+  let Documentation = [MSConstexprDocs];
+}
+
 def MSNoVTable : InheritableAttr, TargetSpecificAttr<TargetMicrosoftCXXABI> {
   let Spellings = [Declspec<"novtable">];
   let Subjects = SubjectList<[CXXRecord]>;
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 05703df2129f612..ce44bd7d5b21490 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -3612,6 +3612,22 @@ an error:
 }];
 }
 
+def MSConstexprDocs : Documentation {
+  let Category = DocCatStmt;
+  let Content = [{
+The ``[[msvc::constexpr]]`` attribute can be applied only to a function
+definition or a ``return`` statement. It does not impact function declarations.
+A ``[[msvc::constexpr]]`` function cannot be ``constexpr`` or ``consteval``.
+A ``[[msvc::constexpr]]`` function is treated in the same way as a ``constexpr``
+function if it is evaluated in a constant context of
+``[[msvc::constexpr]] return`` statement.
+Otherwise, it is treated as a regular function.
+
+Semantics of this attribute is enabled only under MSVC compatibility
+(``-fms-compatibility-version``) 19.33 and later.
+  }];
+}
+
 def MSNoVTableDocs : Documentation {
   let Category = DocCatDecl;
   let Content = [{
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 474afc2fb99c1f0..1cfe22227c731da 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2861,8 +2861,8 @@ def warn_cxx17_compat_constexpr_local_var_no_init : Warning<
   "is incompatible with C++ standards before C++20">,
   InGroup<CXXPre20Compat>, DefaultIgnore;
 def ext_constexpr_function_never_constant_expr : ExtWarn<
-  "%select{constexpr|consteval}1 %select{function|constructor}0 never produces a "
-  "constant expression">, InGroup<DiagGroup<"invalid-constexpr">>, DefaultError;
+  "%select{constexpr|consteval|[[msvc::constexpr]]}1 %select{function|constructor}0 "
+  "never produces a constant expression">, InGroup<DiagGroup<"invalid-constexpr">>, DefaultError;
 def err_attr_cond_never_constant_expr : Error<
   "%0 attribute expression never produces a constant expression">;
 def err_diagnose_if_invalid_diagnostic_type : Error<
@@ -2884,6 +2884,12 @@ def warn_cxx11_compat_constexpr_body_multiple_return : Warning<
   InGroup<CXXPre14Compat>, DefaultIgnore;
 def note_constexpr_body_previous_return : Note<
   "previous return statement is here">;
+def err_ms_constexpr_not_distinct : Error<
+  "[[msvc::constexpr]] cannot be applied to a %select{constexpr|consteval}0 function %1">;
+def err_ms_constexpr_virtual : Error<"virtual function cannot be [[msvc::constexpr]]">;
+def warn_ms_constexpr_no_effect : Warning<
+  "[[msvc::constexpr]] has effect only on function definitions and return statements">,
+  InGroup<IgnoredAttributes>;
 
 // C++20 function try blocks in constexpr
 def ext_constexpr_function_try_block_cxx20 : ExtWarn<
diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h
index 20a8ada60e0fe51..f5d2cfcafcd198c 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -152,6 +152,7 @@ class LangOptions : public LangOptionsBase {
     MSVC2019 = 1920,
     MSVC2019_5 = 1925,
     MSVC2019_8 = 1928,
+    MSVC2022_3 = 1933,
   };
 
   enum SYCLMajorVersion {
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index b6b1e6617dffaa9..6aa10ac61f87d6a 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -640,6 +640,10 @@ namespace {
       return false;
     }
 
+    /// Whether we're in a context where [[msvc::constexpr]] evaluation is
+    /// permitted. See MSConstexprDocs for description of permitted contexts.
+    bool CanEvalMSConstexpr = false;
+
   private:
     APValue &createLocal(APValue::LValueBase Base, const void *Key, QualType T,
                          ScopeKind Scope);
@@ -673,6 +677,19 @@ namespace {
   private:
     llvm::TimeTraceScope TimeScope;
   };
+
+  /// RAII object used to change the current ability of
+  /// [[msvc::constexpr]] evaulation.
+  struct MSConstexprContextRAII {
+    CallStackFrame &Frame;
+    bool OldValue;
+    explicit MSConstexprContextRAII(CallStackFrame &Frame, bool Value)
+        : Frame(Frame), OldValue(Frame.CanEvalMSConstexpr) {
+      Frame.CanEvalMSConstexpr = Value;
+    }
+
+    ~MSConstexprContextRAII() { Frame.CanEvalMSConstexpr = OldValue; }
+  };
 }
 
 static bool HandleDestruction(EvalInfo &Info, const Expr *E,
@@ -5535,11 +5552,14 @@ static EvalStmtResult EvaluateStmt(StmtResult &Result, EvalInfo &Info,
   case Stmt::LabelStmtClass:
     return EvaluateStmt(Result, Info, cast<LabelStmt>(S)->getSubStmt(), Case);
 
-  case Stmt::AttributedStmtClass:
-    // As a general principle, C++11 attributes can be ignored without
-    // any semantic impact.
-    return EvaluateStmt(Result, Info, cast<AttributedStmt>(S)->getSubStmt(),
-                        Case);
+  case Stmt::AttributedStmtClass: {
+    const auto *AS = cast<AttributedStmt>(S);
+    const auto *SS = AS->getSubStmt();
+    MSConstexprContextRAII msConstexprContext(
+        *Info.CurrentCall, hasSpecificAttr<MSConstexprAttr>(AS->getAttrs()) &&
+                               isa<ReturnStmt>(SS));
+    return EvaluateStmt(Result, Info, SS, Case);
+  }
 
   case Stmt::CaseStmtClass:
   case Stmt::DefaultStmtClass:
@@ -5610,7 +5630,9 @@ static bool CheckConstexprFunction(EvalInfo &Info, SourceLocation CallLoc,
   }
 
   // Can we evaluate this function call?
-  if (Definition && Definition->isConstexpr() && Body)
+  if (Definition && Body &&
+      (Definition->isConstexpr() || Info.CurrentCall->CanEvalMSConstexpr &&
+                                        Definition->hasAttr<MSConstexprAttr>()))
     return true;
 
   if (Info.getLangOpts().CPlusPlus11) {
diff --git a/clang/lib/Basic/Targets/OSTargets.cpp b/clang/lib/Basic/Targets/OSTargets.cpp
index 627bc912fa23104..899aefa6173acfe 100644
--- a/clang/lib/Basic/Targets/OSTargets.cpp
+++ b/clang/lib/Basic/Targets/OSTargets.cpp
@@ -224,6 +224,9 @@ static void addVisualCDefines(const LangOptions &Opts, MacroBuilder &Builder) {
       else if (Opts.CPlusPlus14)
         Builder.defineMacro("_MSVC_LANG", "201402L");
     }
+
+    if (Opts.isCompatibleWithMSVC(LangOptions::MSVC2022_3))
+      Builder.defineMacro("_MSVC_CONSTEXPR_ATTRIBUTE");
   }
 
   if (Opts.MicrosoftExt) {
diff --git a/clang/lib/Driver/ToolChains/MSVC.cpp b/clang/lib/Driver/ToolChains/MSVC.cpp
index 4966d102c51f1ac..9809ae39da2e4e1 100644
--- a/clang/lib/Driver/ToolChains/MSVC.cpp
+++ b/clang/lib/Driver/ToolChains/MSVC.cpp
@@ -787,11 +787,11 @@ VersionTuple MSVCToolChain::computeMSVCVersion(const Driver *D,
   if (MSVT.empty() &&
       Args.hasFlag(options::OPT_fms_extensions, options::OPT_fno_ms_extensions,
                    IsWindowsMSVC)) {
-    // -fms-compatibility-version=19.20 is default, aka 2019, 16.x
+    // -fms-compatibility-version=19.33 is default, aka 2022, 17.3
     // NOTE: when changing this value, also update
     // clang/docs/CommandGuide/clang.rst and clang/docs/UsersManual.rst
     // accordingly.
-    MSVT = VersionTuple(19, 20);
+    MSVT = VersionTuple(19, 33);
   }
   return MSVT;
 }
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 396566a8f10a9b7..e753b721729e51b 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -16163,7 +16163,9 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
         ActivePolicy = &WP;
       }
 
-      if (!IsInstantiation && FD && FD->isConstexpr() && !FD->isInvalidDecl() &&
+      if (!IsInstantiation && FD &&
+          (FD->isConstexpr() || FD->hasAttr<MSConstexprAttr>()) &&
+          !FD->isInvalidDecl() &&
           !CheckConstexprFunctionDefinition(FD, CheckConstexprKind::Diagnose))
         FD->setInvalidDecl();
 
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index cdb769a883550d0..689d69d4b94ae84 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -7337,6 +7337,27 @@ static void handleDeclspecThreadAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   D->addAttr(::new (S.Context) ThreadAttr(S.Context, AL));
 }
 
+static void handleMSConstexprAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+  if (!S.getLangOpts().isCompatibleWithMSVC(LangOptions::MSVC2022_3)) {
+    S.Diag(AL.getLoc(), diag::warn_unknown_attribute_ignored)
+        << AL << AL.getRange();
+    return;
+  }
+  auto *FD = cast<FunctionDecl>(D);
+  if (FD->isConstexprSpecified() || FD->isConsteval()) {
+    S.Diag(AL.getLoc(), diag::err_ms_constexpr_not_distinct)
+        << FD->isConsteval() << FD;
+    return;
+  }
+  if (auto *MD = dyn_cast<CXXMethodDecl>(FD)) {
+    if (!S.getLangOpts().CPlusPlus20 && MD->isVirtual()) {
+      S.Diag(AL.getLoc(), diag::err_ms_constexpr_virtual);
+      return;
+    }
+  }
+  D->addAttr(::new (S.Context) MSConstexprAttr(S.Context, AL));
+}
+
 static void handleAbiTagAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   SmallVector<StringRef, 4> Tags;
   for (unsigned I = 0, E = AL.getNumArgs(); I != E; ++I) {
@@ -9439,6 +9460,9 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
   case ParsedAttr::AT_Thread:
     handleDeclspecThreadAttr(S, D, AL);
     break;
+  case ParsedAttr::AT_MSConstexpr:
+    handleMSConstexprAttr(S, D, AL);
+    break;
 
   // HLSL attributes:
   case ParsedAttr::AT_HLSLNumThreads:
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 397b7a00e453126..c294c946a254e96 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -2458,9 +2458,12 @@ static bool CheckConstexprFunctionBody(Sema &SemaRef, const FunctionDecl *Dcl,
   SmallVector<PartialDiagnosticAt, 8> Diags;
   if (Kind == Sema::CheckConstexprKind::Diagnose &&
       !Expr::isPotentialConstantExpr(Dcl, Diags)) {
+    int ConstKind = Dcl->hasAttr<MSConstexprAttr>() ? 2
+                    : Dcl->isConsteval()            ? 1
+                                                    : 0;
     SemaRef.Diag(Dcl->getLocation(),
                  diag::ext_constexpr_function_never_constant_expr)
-        << isa<CXXConstructorDecl>(Dcl) << Dcl->isConsteval()
+        << isa<CXXConstructorDecl>(Dcl) << ConstKind
         << Dcl->getNameInfo().getSourceRange();
     for (size_t I = 0, N = Diags.size(); I != N; ++I)
       SemaRef.Diag(Diags[I].first, Diags[I].second);
diff --git a/clang/lib/Sema/SemaStmtAttr.cpp b/clang/lib/Sema/SemaStmtAttr.cpp
index ad20bc8871f103a..7b8742b7687e031 100644
--- a/clang/lib/Sema/SemaStmtAttr.cpp
+++ b/clang/lib/Sema/SemaStmtAttr.cpp
@@ -322,6 +322,20 @@ static Attr *handleUnlikely(Sema &S, Stmt *St, const ParsedAttr &A,
   return ::new (S.Context) UnlikelyAttr(S.Context, A);
 }
 
+static Attr *handleMSConstexprAttr(Sema &S, Stmt *St, const ParsedAttr &A,
+                                   SourceRange Range) {
+  if (!S.getLangOpts().isCompatibleWithMSVC(LangOptions::MSVC2022_3)) {
+    S.Diag(A.getLoc(), diag::warn_unknown_attribute_ignored)
+        << A << A.getRange();
+    return nullptr;
+  }
+  if (!isa<ReturnStmt>(St)) {
+    S.Diag(A.getLoc(), diag::warn_ms_constexpr_no_effect);
+    return nullptr;
+  }
+  return ::new (S.Context) MSConstexprAttr(S.Context, A);
+}
+
 #define WANT_STMT_MERGE_LOGIC
 #include "clang/Sema/AttrParsedAttrImpl.inc"
 #undef WANT_STMT_MERGE_LOGIC
@@ -523,6 +537,8 @@ static Attr *ProcessStmtAttribute(Sema &S, Stmt *St, const ParsedAttr &A,
     return handleLikely(S, St, A, Range);
   case ParsedAttr::AT_Unlikely:
     return handleUnlikely(S, St, A, Range);
+  case ParsedAttr::AT_MSConstexpr:
+    return handleMSConstexprAttr(S, St, A, Range);
   default:
     // N.B., ClangAttrEmitter.cpp emits a diagnostic helper that ensures a
     // declaration attribute is not written on a statement, but this code is
diff --git a/clang/test/AST/ms-constexpr.cpp b/clang/test/AST/ms-constexpr.cpp
new file mode 100644
index 000000000000000..e85af8494f3344a
--- /dev/null
+++ b/clang/test/AST/ms-constexpr.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -fms-compatibility -fms-compatibility-version=19.33 -std=c++20 -ast-dump -verify %s | FileCheck %s
+// expected-no-diagnostics
+
+// CHECK: used f1 'bool ()'
+// CHECK: MSConstexprAttr 0x{{[0-9a-f]+}} <col:3, col:9>
+[[msvc::constexpr]] bool f1() { return true; }
+
+// CHECK: used constexpr f2 'bool ()'
+// CHECK-NEXT: CompoundStmt 0x{{[0-9a-f]+}} <col:21, col:56>
+// CHECK-NEXT: AttributedStmt 0x{{[0-9a-f]+}} <col:23, col:53>
+// CHECK-NEXT: MSConstexprAttr 0x{{[0-9a-f]+}} <col:25, col:31>
+// CHECK-NEXT: ReturnStmt 0x{{[0-9a-f]+}} <col:43, col:53>
+constexpr bool f2() { [[msvc::constexpr]] return f1(); }
+static_assert(f2());
+
+struct S1 {
+    // CHECK: used vm 'bool ()' virtual
+    // CHECK: MSConstexprAttr 0x{{[0-9a-f]+}} <col:7, col:13>
+    [[msvc::constexpr]] virtual bool vm() { return true; }
+
+    // CHECK: used constexpr cm 'bool ()'
+    // CHECK-NEXT: CompoundStmt 0x{{[0-9a-f]+}} <col:25, col:60>
+    // CHECK-NEXT: AttributedStmt 0x{{[0-9a-f]+}} <col:27, col:57>
+    // CHECK-NEXT: MSConstexprAttr 0x{{[0-9a-f]+}} <col:29, col:35>
+    // CHECK-NEXT: ReturnStmt 0x{{[0-9a-f]+}} <col:47, col:57>
+    constexpr bool cm() { [[msvc::constexpr]] return vm(); }
+};
+static_assert(S1{}.cm());
diff --git a/clang/test/Driver/cl-options.c b/clang/test/Driver/cl-options.c
index 6d929b19e7e2ef8..81d1b907eced188 100644
--- a/clang/test/Driver/cl-options.c
+++ b/clang/test/Driver/cl-options.c
@@ -747,7 +747,7 @@
 
 // Validate that the default triple is used when run an empty tools dir is specified
 // RUN: %clang_cl -vctoolsdir "" -### -- %s 2>&1 | FileCheck %s --check-prefix VCTOOLSDIR
-// VCTOOLSDIR: "-triple" "{{[a-zA-Z0-9_-]*}}-pc-windows-msvc19.20.0"
+// VCTOOLSDIR: "-triple" "{{[a-zA-Z0-9_-]*}}-pc-windows-msvc19.33.0"
 
 // Validate that built-in include paths are based on the supplied path
 // RUN: %clang_cl --target=aarch64-pc-windows-msvc -vctoolsdir "/fake" -winsdkdir "/foo" -winsdkversion 10.0.12345.0 -### -- %s 2>&1 | FileCheck %s --check-prefix FAKEDIR
@@ -787,7 +787,7 @@
 
 // RUN: %clang_cl -vctoolsdir "" /arm64EC /c -### -- %s 2>&1 | FileCheck --check-prefix=ARM64EC %s 
 // ARM64EC-NOT: /arm64EC has been overridden by specified target
-// ARM64EC: "-triple" "arm64ec-pc-windows-msvc19.20.0"
+// ARM64EC: "-triple" "arm64ec-pc-windows-msvc19.33.0"
 
 // RUN: %clang_cl -vctoolsdir "" /arm64EC /c -target x86_64-pc-windows-msvc  -### -- %s 2>&1 | FileCheck --check-prefix=ARM64EC_OVERRIDE %s
 // ARM64EC_OVERRIDE: warning: /arm64EC has been overridden by specified target: x86_64-pc-windows-msvc; option ignored
diff --git a/clang/test/Misc/pragma-attribute-supported-attributes-list.test b/clang/test/Misc/pragma-attribute-supported-attributes-list.test
index f48126775c8685c..67a74f7a4d13b6f 100644
--- a/clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ b/clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -85,6 +85,7 @@
 // CHECK-NEXT: LoaderUninitialized (SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: Lockable (SubjectMatchRule_record)
 // CHECK-NEXT: MIGServerRoutine (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_block)
+// CHECK-NEXT: MSConstexpr (SubjectMatchRule_function)
 // CHECK-NEXT: MSStruct (SubjectMatchRule_record)
 // CHECK-NEXT: MaybeUndef (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: MicroMips (SubjectMatchRule_function)
diff --git a/clang/test/SemaCXX/ms-constexpr-invalid.cpp b/clang/test/SemaCXX/ms-constexpr-invalid.cpp
new file mode 100644
index 000000000000000..9e88d8097e02a04
--- /dev/null
+++ b/clang/test/SemaCXX/ms-constexpr-invalid.cpp
@@ -0,0 +1,52 @@
+// RUN: %clang_cc1 -fms-compatibility -fms-compatibility-version=19.33 -std=c++20 -verify %s
+// RUN: %clang_cc1 -fms-compatibility -fms-compatibility-version=19.33 -std=c++17 -verify %s
+
+// Check explicitly invalid code
+
+void runtime() {} // expected-note {{declared here}}
+
+[[msvc::constexpr]] void f0() { runtime(); } // expected-error {{[[msvc::constexpr]] function never produces a constant expression}} \
+                                             // expected-note {{non-constexpr function 'runtime' cannot be used in a constant expression}}
+[[msvc::constexpr]] constexpr void f1() {} // expected-error {{[[msvc::constexpr]] cannot be applied to a constexpr function 'f1'}}
+#if __cplusplus >= 202202L
+[[msvc::constexpr]] consteval void f2() {} // expected-error {{[[msvc::constexpr]] cannot be applied to a consteval function 'f2'}}
+#endif
+
+struct B1 {};
+struct D1 : virtual B1 { // expected-note {{virtual base class declared here}}
+    [[msvc::constexpr]] D1() {} // expected-error {{constexpr constructor not allowed in struct with virtual base class}}
+};
+
+struct [[msvc::constexpr]] S2{}; // expected-error {{'constexpr' attribute only applies to functions and statements}}
+
+// Check invalid code mixed with valid code
+
+[[msvc::constexpr]] int f4(int x) { return x > 1 ? 1 + f4(x / 2) : 0; } // expected-note {{non-constexpr function 'f4' cannot be used in a constant expression}} \
+                                                                        // expected-note {{declared here}} \
+                                                                        // expected-note {{declared here}} \
+                                                                        // expected-note {{declared here}}
+constexpr bool f5() { [[msvc::constexpr]] return f4(32) == 5; } // expected-note {{in call to 'f4(32)'}}
+static_assert(f5()); // expected-error {{static assertion expression is not an integral constant expression}} \
+                     // expected-note {{in call to 'f5()'}}
+
+int f6(int x) { [[msvc::constexpr]] return x > 1 ? 1 + f6(x / 2) : 0; } // expected-note {{declared here}} \
+                                                                        // expected-note {{declared here}}
+constexpr bool f7() { [[msvc::constexpr]] return f6(32) == 5; } // expected-error {{constexpr function never produces a constant expression}} \
+                                   ...
[truncated]

@RIscRIpt
Copy link
Member Author

RIscRIpt commented Nov 4, 2023

Initially I replicated semantics of [[msvc::constexpr]] from MSVC, so that it was possible to use it the same way as in MSVC, even [[msvc::constexpr]] return ::new from non-std namespace. E.g. https://godbolt.org/z/7eKh5Envz

// RUN: %clang_cc1 -fms-compatibility -fms-compatibility-version=19.33 -std=c++20 -ast-dump %s | FileCheck %s

// CHECK: used operator new
// CHECK: MSConstexprAttr 0x{{[0-9a-f]+}} <col:17, col:23>
[[nodiscard]] [[msvc::constexpr]] inline void* __cdecl operator new(decltype(sizeof(void*)), void* p) noexcept { return p; }

// CHECK: used constexpr construct_at
// CHECK: AttributedStmt 0x{{[0-9a-f]+}} <col:46, col:88>
// CHECK-NEXT: MSConstexprAttr 0x{{[0-9a-f]+}} <col:48, col:54>
// CHECK-NEXT: ReturnStmt 0x{{[0-9a-f]+}} <col:66, col:88>
constexpr int* construct_at(int* p, int v) { [[msvc::constexpr]] return ::new (p) int(v); }

constexpr bool check_construct_at() { int x; return *construct_at(&x, 42) == 42; }

static_assert(check_construct_at());

However @zygoloid raised a concern over changes in PointerExprEvaluator::VisitCXXNewExpr:

   bool IsNothrow = false;
   bool IsPlacement = false;
+  bool IsMSConstexpr = Info.CurrentCall->CanEvalMSConstexpr &&
+                       OperatorNew->hasAttr<MSConstexprAttr>();
  if (OperatorNew->isReservedGlobalPlacementOperator() &&
-     Info.CurrentCall->isStdFunction() && !E->isArray()) {
+     (Info.CurrentCall->isStdFunction() || IsMSConstexpr) && !E->isArray()) {

Do we really need this change? Was our existing check of whether the caller is in namespace std not sufficient for MS' standard library? I'd strongly prefer not to have a documented, user-visible attribute that gives permission to use placement new directly.

If I could choose, I would opt to fully replicate the behavior of MSVC. However, I also acknowledge the concerns that have been raised.

@AaronBallman, @erichkeane, any comments?

@RIscRIpt
Copy link
Member Author

RIscRIpt commented Nov 4, 2023

Discussion initiated by @AaronBallman of one of important design decisions, which was forgotten (?):

@AaronBallman:
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.

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

Note: in the current implementation I enable [[msvc::constexpr]] only under -fms-compatibility -fms-compatibility-version=19.33, (MSVC 19.33), where this attribute has first appeared.

Copy link
Collaborator

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is pretty close, I think skipping on constructors for now is acceptable. However, i think we should enable the test, and record the diagnostic to ensure we show it is invalid for now (along with better showing the TODO).

@RIscRIpt
Copy link
Member Author

Oof, sorry, I forgot I should push new changes and rebased changes separately (otherwise "Compare" link does not make sense). If you had checkout 829f8098af96 locally, you can see the diff diff as follows:

git range-diff 829f8098af96~2..829f8098af96 7d8b1c0fae42~2..7d8b1c0fae42

@RIscRIpt
Copy link
Member Author

Rebased onto main, resolved conflicts, re-run lit locally.
Please let me know if I could speed-up review in any way.

@RIscRIpt RIscRIpt requested a review from erichkeane November 25, 2023 09:14
@tru
Copy link
Collaborator

tru commented Dec 7, 2023

@rnk @amykhuang might be interested in reviewing as well.

Copy link
Member Author

@RIscRIpt RIscRIpt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed review comments; updated my tests to use changed diagnostic messages.

Next I'll upload my branch rebased onto main.

@erichkeane
Copy link
Collaborator

Also, please don't 'force-push', just push an additional commit to your review. It erases history and makes reviewing it about 3x harder since history/context gets lost.

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm really liking the direction here -- this was a much more simple solution than I was expecting. Aside from some tiny nits, this looks good to me, but I leave it to Erich for the final sign-off.

@RIscRIpt
Copy link
Member Author

RIscRIpt commented Dec 8, 2023

Also, please don't 'force-push', just push an additional commit to your review. It erases history and makes reviewing it about 3x harder since history/context gets lost.

Sorry. I did this, because I am used to such workflow and reviewing range-diff; but to make it work one have to follow strict rule of pushing changes and rebased branch separately (which I failed to do previous time).

Edit: I think I see what you mean 😕

@RIscRIpt
Copy link
Member Author

RIscRIpt commented Dec 8, 2023

Thank you!

@RIscRIpt
Copy link
Member Author

RIscRIpt commented Dec 9, 2023

I don't have write access, but I believe it is ready to be merged.

@RIscRIpt
Copy link
Member Author

RIscRIpt commented Dec 9, 2023

Suggested commit message:

Add Support for 'msvc::constexpr' C++11 Attribute

This commit introduces support for the MSVC-specific C++11-style
attribute `[[msvc::constexpr]]`, which was introduced in MSVC 14.33.
The semantics of this attribute are enabled only under
MSVC compatibility (`-fms-compatibility-version`) 14.33 and higher.
Additionally, the default value of `_MSC_VER` has been raised to 1433.

The current implementation lacks support for:
- `[[msvc::constexpr]]` constructors (see #72149);
  at the time of this implementation, such support would have required
  an unreasonable number of changes in Clang.
- `[[msvc::constexpr]] return ::new` (constexpr placement new) from
  non-std namespaces (see #74924).

Relevant to: #57696

Edit: styling/grammar.

@Endilll Endilll merged commit b3e6ff3 into llvm:main Dec 9, 2023
@RIscRIpt RIscRIpt deleted the msvc-constexpr branch December 9, 2023 10:52
Copy link
Contributor

@Endilll Endilll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some bots are not happy with one of the tests. I contacted you on Discord about your next steps.

I'll have to revert the patch if I won't hear from you soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category extension:microsoft
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants