[Clang][Sema] Reject array prvalue operands #140702

languagelawyer · 2025-05-20T10:00:09Z

of unary + and *, and binary + and - operators

github-actions · 2025-05-20T10:00:30Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2025-05-20T10:01:03Z

@llvm/pr-subscribers-clang

Author: None (languagelawyer)

Changes

of unary + and *, and binary + and - operators

Fixes #54016

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

2 Files Affected:

(modified) clang/lib/Sema/SemaExpr.cpp (+21)
(modified) clang/test/CXX/expr/p8.cpp (+10-2)

diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index d1889100c382e..c0fa9bc9e895e 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11333,6 +11333,11 @@ QualType Sema::CheckAdditionOperands(ExprResult &LHS, ExprResult &RHS,
   if (!IExp->getType()->isIntegerType())
     return InvalidOperands(Loc, LHS, RHS);
 
+  if (OriginalOperand Orig(PExp); Orig.getType()->isArrayType() && Orig.Orig->isPRValue()) {
+    Diag(Loc, diag::err_typecheck_array_prvalue_operand) << PExp->getSourceRange();
+    return QualType();
+  }
+
   // Adding to a null pointer results in undefined behavior.
   if (PExp->IgnoreParenCasts()->isNullPointerConstant(
           Context, Expr::NPC_ValueDependentIsNotNull)) {
@@ -11429,6 +11434,16 @@ QualType Sema::CheckSubtractionOperands(ExprResult &LHS, ExprResult &RHS,
     return compType;
   }
 
+  OriginalOperand OrigLHS(LHS.get()), OrigRHS(RHS.get());
+  bool LHSArrayPRV = OrigLHS.getType()->isArrayType() && OrigLHS.Orig->isPRValue();
+  bool RHSArrayPRV = OrigRHS.getType()->isArrayType() && OrigRHS.Orig->isPRValue();
+  if (LHSArrayPRV || RHSArrayPRV) {
+    auto&& diag = Diag(Loc, diag::err_typecheck_array_prvalue_operand);
+    if (LHSArrayPRV) diag << LHS.get()->getSourceRange();
+    if (RHSArrayPRV) diag << RHS.get()->getSourceRange();
+    return QualType();
+  }
+
   // Either ptr - int   or   ptr - ptr.
   if (LHS.get()->getType()->isAnyPointerType()) {
     QualType lpointee = LHS.get()->getType()->getPointeeType();
@@ -15840,6 +15855,12 @@ ExprResult Sema::CreateBuiltinUnaryOp(SourceLocation OpLoc,
       InputExpr->getType()->isSpecificBuiltinType(BuiltinType::Dependent)) {
     resultType = Context.DependentTy;
   } else {
+    if (Opc == UO_Deref || Opc == UO_Plus) {
+      if (auto *expr = Input.get(); expr->getType()->isArrayType() && expr->isPRValue()) {
+        Diag(OpLoc, diag::err_typecheck_array_prvalue_operand) << expr->getSourceRange();
+        return ExprError();
+      }
+    }
     switch (Opc) {
     case UO_PreInc:
     case UO_PreDec:
diff --git a/clang/test/CXX/expr/p8.cpp b/clang/test/CXX/expr/p8.cpp
index 471d1c5a30206..f736b88b3db09 100644
--- a/clang/test/CXX/expr/p8.cpp
+++ b/clang/test/CXX/expr/p8.cpp
@@ -1,5 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11
 
 int a0;
 const volatile int a1 = 2;
@@ -16,4 +15,13 @@ int main()
   f0(a1);
   f1(a2);
   f2(a3);
+
+  using IA = int[];
+  void(+IA{ 1, 2, 3 }); // expected-error {{array prvalue}}
+  void(*IA{ 1, 2, 3 }); // expected-error {{array prvalue}}
+  void(IA{ 1, 2, 3 } + 0); // expected-error {{array prvalue}}
+  void(IA{ 1, 2, 3 } - 0); // expected-error {{array prvalue}}
+  void(0 + IA{ 1, 2, 3 }); // expected-error {{array prvalue}}
+  void(0 - IA{ 1, 2, 3 }); // expected-error {{array prvalue}}
+  void(IA{ 1, 2, 3 } - IA{ 1, 2, 3 }); // expected-error {{array prvalue}}
 }

github-actions · 2025-05-20T10:03:14Z

✅ With the latest revision this PR passed the C/C++ code formatter.

zwuis · 2025-05-20T10:49:31Z

This is related to CWG2548. CC @Endilll

languagelawyer · 2025-05-20T10:53:40Z

This is related to CWG2548

Kinda, but not really. It has never been necessary to create that (non-)issue. Or does someone really believe that it is a C++ defect that IA{ 1, 2, 3 } - IA{ 1, 2, 3 } is not allowed? So, it is just a bug since C++11

of unary + and *, and binary + and - operators

AaronBallman

Is this going to break behavior in C? https://godbolt.org/z/PsPPs8ov1

languagelawyer · 2025-05-20T13:47:36Z

Is this going to break behavior in C?

Array prvalues is C++11+ exclusive thing. Compound literals are lvalues in C https://port70.net/~nsz/c/c11/n1570.html#6.5.2.5p4
Anyway, here is a check:

$ build/bin/clang -fsyntax-only test.c
test.c:2:3: warning: expression result unused [-Wunused-value]
    2 |   *((int []){ 1, 2, 3});
      |   ^~~~~~~~~~~~~~~~~~~~~
test.c:3:24: warning: expression result unused [-Wunused-value]
    3 |   ((int []){ 1, 2, 3}) + 0;
      |   ~~~~~~~~~~~~~~~~~~~~ ^ ~
2 warnings generated.

However, it "breaks" compound literal C++ extension:

$ build/bin/clang++ -fsyntax-only test.cxx 
test.cxx:12:2: error: array prvalue is not permitted
   12 |         *((int []){ 1, 2, 3});
      |         ^~~~~~~~~~~~~~~~~~~~~
test.cxx:13:23: error: array prvalue is not permitted
   13 |         ((int []){ 1, 2, 3}) + 0;
      |         ~~~~~~~~~~~~~~~~~~~~ ^
2 errors generated.

but i.g. this is OK/expected

AaronBallman · 2025-05-20T14:12:13Z

Is this going to break behavior in C?

Array prvalues is C++11+ exclusive thing. Compound literals are lvalues in C

Ah, good point, I wasn't remembering that C and C++ are different in how they handle compound literals. And you can return a pointer to an array in C, but that's still an lvalue.

Anyway, here is a check:

$ build/bin/clang -fsyntax-only test.c
test.c:2:3: warning: expression result unused [-Wunused-value]
    2 |   *((int []){ 1, 2, 3});
      |   ^~~~~~~~~~~~~~~~~~~~~
test.c:3:24: warning: expression result unused [-Wunused-value]
    3 |   ((int []){ 1, 2, 3}) + 0;
      |   ~~~~~~~~~~~~~~~~~~~~ ^ ~
2 warnings generated.

Thank you!

AaronBallman · 2025-05-20T15:51:51Z

However, it "breaks" compound literal C++ extension:

$ build/bin/clang++ -fsyntax-only test.cxx 
test.cxx:12:2: error: array prvalue is not permitted
   12 |         *((int []){ 1, 2, 3});
      |         ^~~~~~~~~~~~~~~~~~~~~
test.cxx:13:23: error: array prvalue is not permitted
   13 |         ((int []){ 1, 2, 3}) + 0;
      |         ~~~~~~~~~~~~~~~~~~~~ ^
2 errors generated.

That probably should continue to work -- we accept it today and so does GCC. It's a bit of an oddity, to be sure. But I don't see why it should be rejected either.

languagelawyer · 2025-05-20T16:07:56Z

That probably should continue to work -- we accept it today and so does GCC

GCC started accepting it from version 14, likely when fixing https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94264 (it is mentioned in #54016 (comment)), but it also started accepting IA{ 1, 2, 3 } + 0, which is clearly wrong https://godbolt.org/z/r7Yc3o1qd @pinskia @jicama

I think, if someone wants 100% C compatibility, then they should make compound literals lvalues. If compound literals are prvalues in C++ extension, then they should be treated like other array prvalues

AaronBallman · 2025-05-20T16:16:50Z

That probably should continue to work -- we accept it today and so does GCC

GCC started accepting it from version 14, likely when fixing https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94264 (it is mentioned in #54016 (comment)), but it also started accepting IA{ 1, 2, 3 } + 0, which is clearly wrong https://godbolt.org/z/r7Yc3o1qd @pinskia @jicama

I think, if someone wants 100% C compatibility, then they should make compound literals lvalues. If compound literals are prvalues in C++ extension, then they should be treated like other array prvalues

Ugh, I was backwards again. They're prvalues in C++ already, not lvalues as in C, so yes, it should be rejected in C++ with your changes. Sorry for the confusion!

pinskia · 2025-05-20T17:27:08Z

So reading https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53220; it was decided to treat compound literals exactly the same as prvalues arrays in C++ mode of GCC. Yes breaking compatibility with C for the most part.

languagelawyer · 2025-05-20T17:59:53Z

@pinskia the thing is, GCC started accepting ill-formed code right when it was reaffirmed it is ill-formed

Sirraide · 2025-05-21T02:57:01Z

clang/test/CXX/expr/p8.cpp

@@ -16,4 +15,13 @@ int main()
  f0(a1);
  f1(a2);
  f2(a3);
+
+  using IA = int[];
+  void(+IA{ 1, 2, 3 }); // expected-error {{array prvalue}}


Can you add some tests here to make sure that we don’t complain about array glvalues?

Sirraide · 2025-05-21T02:58:47Z

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -7639,6 +7639,8 @@ def warn_param_mismatched_alignment : Warning<

 def err_objc_object_assignment : Error<
  "cannot assign to class object (%0 invalid)">;
+def err_typecheck_array_prvalue_operand : Error<
+  "array prvalue is not permitted">;


Suggested change

"array prvalue is not permitted">;

"operand of '%0' cannot be an array prvalue">;

I think it’s a bit clearer if we phrase it like this and also print out the operator rather than just saying ‘X is not permitted’.

Sirraide · 2025-05-21T03:02:05Z

clang/lib/Sema/SemaExpr.cpp

@@ -15840,6 +15859,11 @@ ExprResult Sema::CreateBuiltinUnaryOp(SourceLocation OpLoc,
      InputExpr->getType()->isSpecificBuiltinType(BuiltinType::Dependent)) {
    resultType = Context.DependentTy;
  } else {
+    if (Opc == UO_Deref || Opc == UO_Plus) {


It feels a bit weird to use an if statement here when there’s a switch statement on the same variable right after. I’d probably make this a lambda and then call it in the cases for * and + below (the + case would then also need a [[fallthrough]] annotation).

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels May 20, 2025

languagelawyer force-pushed the array_prvalues_as_operands branch 2 times, most recently from 3e262aa to f606724 Compare May 20, 2025 11:50

[Clang][Sema] Reject array prvalue operands

df91056

of unary + and *, and binary + and - operators

languagelawyer force-pushed the array_prvalues_as_operands branch from f606724 to df91056 Compare May 20, 2025 11:55

AaronBallman reviewed May 20, 2025

View reviewed changes

AaronBallman requested review from erichkeane, Endilll and Sirraide May 20, 2025 15:52

Sirraide reviewed May 21, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Clang][Sema] Reject array prvalue operands #140702

[Clang][Sema] Reject array prvalue operands #140702

languagelawyer commented May 20, 2025

github-actions bot commented May 20, 2025

llvmbot commented May 20, 2025

github-actions bot commented May 20, 2025 •

edited

Loading

zwuis commented May 20, 2025

languagelawyer commented May 20, 2025 •

edited

Loading

AaronBallman left a comment

languagelawyer commented May 20, 2025 •

edited

Loading

AaronBallman commented May 20, 2025

AaronBallman commented May 20, 2025

languagelawyer commented May 20, 2025

AaronBallman commented May 20, 2025

pinskia commented May 20, 2025

languagelawyer commented May 20, 2025

Sirraide May 21, 2025

Sirraide May 21, 2025

Sirraide May 21, 2025

	"array prvalue is not permitted">;
	"operand of '%0' cannot be an array prvalue">;

[Clang][Sema] Reject array prvalue operands #140702

Are you sure you want to change the base?

[Clang][Sema] Reject array prvalue operands #140702

Conversation

languagelawyer commented May 20, 2025

github-actions bot commented May 20, 2025

llvmbot commented May 20, 2025

github-actions bot commented May 20, 2025 • edited Loading

zwuis commented May 20, 2025

languagelawyer commented May 20, 2025 • edited Loading

AaronBallman left a comment

Choose a reason for hiding this comment

languagelawyer commented May 20, 2025 • edited Loading

AaronBallman commented May 20, 2025

AaronBallman commented May 20, 2025

languagelawyer commented May 20, 2025

AaronBallman commented May 20, 2025

pinskia commented May 20, 2025

languagelawyer commented May 20, 2025

Sirraide May 21, 2025

Choose a reason for hiding this comment

Sirraide May 21, 2025

Choose a reason for hiding this comment

Sirraide May 21, 2025

Choose a reason for hiding this comment

github-actions bot commented May 20, 2025 •

edited

Loading

languagelawyer commented May 20, 2025 •

edited

Loading

languagelawyer commented May 20, 2025 •

edited

Loading