[clang] [clang][bytecode] Fix rejecting invalid sizeof expressions (PR #123332)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

<details>
<summary>Changes</summary>

Emit the invalid note instead of nothing.

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


2 Files Affected:

- (modified) clang/lib/AST/ByteCode/Compiler.cpp (+4-5) 
- (modified) clang/test/AST/ByteCode/literals.cpp (+10-15) 


``````````diff
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index 4bfb80589620c1..ebef706d5fbb6a 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -2056,6 +2056,9 @@ bool Compiler<Emitter>::VisitUnaryExprOrTypeTraitExpr(
   if (Kind == UETT_SizeOf || Kind == UETT_DataSizeOf) {
     QualType ArgType = E->getTypeOfArgument();
 
+    if (DiscardResult)
+      return true;
+
     // C++ [expr.sizeof]p2: "When applied to a reference or a reference type,
     //   the result is the size of the referenced type."
     if (const auto *Ref = ArgType->getAs<ReferenceType>())
@@ -2066,17 +2069,13 @@ bool Compiler<Emitter>::VisitUnaryExprOrTypeTraitExpr(
       Size = CharUnits::One();
     else {
       if (ArgType->isDependentType() || !ArgType->isConstantSizeType())
-        return false;
+        return this->emitInvalid(E);
 
       if (Kind == UETT_SizeOf)
         Size = ASTCtx.getTypeSizeInChars(ArgType);
       else
         Size = ASTCtx.getTypeInfoDataSizeInChars(ArgType).Width;
     }
-
-    if (DiscardResult)
-      return true;
-
     return this->emitConst(Size.getQuantity(), E);
   }
 
diff --git a/clang/test/AST/ByteCode/literals.cpp b/clang/test/AST/ByteCode/literals.cpp
index 3d415a93a392a5..fdf1a6820e4466 100644
--- a/clang/test/AST/ByteCode/literals.cpp
+++ b/clang/test/AST/ByteCode/literals.cpp
@@ -261,31 +261,26 @@ namespace SizeOf {
   }
 
 #if __cplusplus >= 201402L
-  constexpr int IgnoredRejected() { // ref-error {{never produces a constant expression}}
+  constexpr int IgnoredRejected() { // both-error {{never produces a constant expression}}
     int n = 0;
     sizeof(int[n++]); // both-warning {{expression result unused}} \
-                      // ref-note 2{{subexpression not valid in a constant expression}}
+                      // both-note 2{{subexpression not valid in a constant expression}}
     return n;
   }
-  /// FIXME: This is rejected because the parameter so sizeof() is not constant.
-  ///   produce a proper diagnostic.
   static_assert(IgnoredRejected() == 0, ""); // both-error {{not an integral constant expression}} \
-                                             // ref-note {{in call to 'IgnoredRejected()'}}
+                                             // both-note {{in call to 'IgnoredRejected()'}}
 #endif
 
 
 #if __cplusplus >= 202002L
   /// FIXME: The following code should be accepted.
-  consteval int foo(int n) { // ref-error {{consteval function never produces a constant expression}}
-    return sizeof(int[n]); // ref-note 3{{not valid in a constant expression}}
-  }
-  constinit int var = foo(5); // ref-error {{not a constant expression}} \
-                              // ref-note 2{{in call to}} \
-                              // ref-error {{does not have a constant initializer}} \
-                              // ref-note {{required by 'constinit' specifier}} \
-                              // expected-error  {{is not a constant expression}} \
-                              // expected-error {{does not have a constant initializer}} \
-                              // expected-note {{required by 'constinit' specifier}} \
+  consteval int foo(int n) { // both-error {{consteval function never produces a constant expression}}
+    return sizeof(int[n]); // both-note 3{{not valid in a constant expression}}
+  }
+  constinit int var = foo(5); // both-error {{not a constant expression}} \
+                              // both-note 2{{in call to}} \
+                              // both-error {{does not have a constant initializer}} \
+                              // both-note {{required by 'constinit' specifier}}
 
 #endif
 };

``````````

</details>


https://github.com/llvm/llvm-project/pull/123332