[clang] The ms-extension __noop should return zero in a constexpr context.

[c8ef]

Fixes #106713.

[llvmbot]

@llvm/pr-subscribers-clang

Author: None (c8ef)

Changes

Fixes #106713.

---

Full diff: https://github.com/llvm/llvm-project/pull/106849.diff

3 Files Affected:

• (modified) clang/lib/AST/ExprConstant.cpp (+3-3)
• (added) clang/test/AST/atomic-expr.c (+10)
• (modified) clang/test/SemaCXX/builtins.cpp (+1)

diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index e8a4d1d3c74102..69d2707aed9171 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -12720,8 +12720,8 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E,
   }
 
   case Builtin::BI__noop:
-  // __noop always evaluates successfully
-    return true;
+    // __noop always evaluates successfully
+    return false;
 
   case Builtin::BI__builtin_is_constant_evaluated: {
     const auto *Callee = Info.CurrentCall->getCallee();
@@ -14458,7 +14458,6 @@ bool IntExprEvaluator::VisitCastExpr(const CastExpr *E) {
   case CK_IntegralComplexToFloatingComplex:
   case CK_BuiltinFnToFnPtr:
   case CK_ZeroToOCLOpaqueType:
-  case CK_NonAtomicToAtomic:
   case CK_AddressSpaceConversion:
   case CK_IntToOCLSampler:
   case CK_FloatingToFixedPoint:
@@ -14482,6 +14481,7 @@ bool IntExprEvaluator::VisitCastExpr(const CastExpr *E) {
   case CK_UserDefinedConversion:
   case CK_LValueToRValue:
   case CK_AtomicToNonAtomic:
+  case CK_NonAtomicToAtomic:
   case CK_NoOp:
   case CK_LValueToRValueBitCast:
   case CK_HLSLArrayRValue:
diff --git a/clang/test/AST/atomic-expr.c b/clang/test/AST/atomic-expr.c
new file mode 100644
index 00000000000000..0826a6491e8a6a
--- /dev/null
+++ b/clang/test/AST/atomic-expr.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify
+// expected-no-diagnostics
+
+typedef _Atomic char atomic_char;
+
+atomic_char counter;
+
+char load_plus_one() {
+  return ({ counter; }) + 1;
+}
\ No newline at end of file
diff --git a/clang/test/SemaCXX/builtins.cpp b/clang/test/SemaCXX/builtins.cpp
index c6fbb8b514d671..78344c45092a79 100644
--- a/clang/test/SemaCXX/builtins.cpp
+++ b/clang/test/SemaCXX/builtins.cpp
@@ -178,4 +178,5 @@ static void __builtin_cpu_init(); // expected-error {{static declaration of '__b
 
 #ifdef _MSC_VER
 constexpr int x = []{ __noop; return 0; }(); // expected-no-diagnostics
+static_assert([] { return __noop; }() == 0);
 #endif

[MitalAshok]

Could you also add tests that calling it returns zero and doesn't evaluate its arguments:

static_assert([]{ return __noop(4); }() == 0);
extern int not_accessed;
void not_called();
static_assert([]{ return __noop(not_accessed *= 6); }() == 0);
static_assert([]{ return __noop(not_called()); }() == 0);
static_assert([]{ return __noop(throw ""); }() == 0);
static_assert([]{ return __noop(throw "", throw ""); }() == 0);
static_assert([]{ int a = 5; __noop(++a); return a; }() == 5);

This passes on MSVC (https://godbolt.org/z/nvxr3GxT4) but currently crashes Clang.

[c8ef]

Could you also add tests that calling it returns zero and doesn't evaluate its arguments:

static_assert([]{ return __noop(4); }() == 0);
extern int not_accessed;
void not_called();
static_assert([]{ return __noop(not_accessed *= 6); }() == 0);
static_assert([]{ return __noop(not_called()); }() == 0);
static_assert([]{ return __noop(throw ""); }() == 0);
static_assert([]{ return __noop(throw "", throw ""); }() == 0);
static_assert([]{ int a = 5; __noop(++a); return a; }() == 5);

This passes on MSVC (https://godbolt.org/z/nvxr3GxT4) but currently crashes Clang.

Done.

[c8ef]

Hi, could you please take a look? @tbaederr

[cor3ntin]

LGTM