Skip to content

Conversation

@tbaederr
Copy link
Contributor

Not doing this is wrong in general and we need to reject expressions where it would matter differently.

Not doing this is wrong in general and we need to reject expressions
where it would matter differently.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Oct 10, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 10, 2024

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

Not doing this is wrong in general and we need to reject expressions where it would matter differently.


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

3 Files Affected:

  • (modified) clang/lib/AST/ByteCode/Compiler.cpp (+10-6)
  • (modified) clang/lib/AST/ByteCode/Pointer.cpp (-5)
  • (modified) clang/test/AST/ByteCode/cxx1z.cpp (+3)
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index ba4c5600d613b0..0a3b38b0dc6e57 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -6006,6 +6006,9 @@ bool Compiler<Emitter>::visitDeclRef(const ValueDecl *D, const Expr *E) {
 
       return this->emitGetPtrParam(It->second.Offset, E);
     }
+
+    if (D->getType()->isReferenceType())
+      return false; // FIXME: Do we need to emit InvalidDeclRef?
   }
 
   // In case we need to re-visit a declaration.
@@ -6042,9 +6045,7 @@ bool Compiler<Emitter>::visitDeclRef(const ValueDecl *D, const Expr *E) {
         const auto typeShouldBeVisited = [&](QualType T) -> bool {
           if (T.isConstant(Ctx.getASTContext()))
             return true;
-          if (const auto *RT = T->getAs<ReferenceType>())
-            return RT->getPointeeType().isConstQualified();
-          return false;
+          return T->isReferenceType();
         };
 
         // DecompositionDecls are just proxies for us.
@@ -6060,9 +6061,12 @@ bool Compiler<Emitter>::visitDeclRef(const ValueDecl *D, const Expr *E) {
         // other words, we're evaluating the initializer, just to know if we can
         // evaluate the initializer.
         if (VD->isLocalVarDecl() && typeShouldBeVisited(VD->getType()) &&
-            VD->getInit() && !VD->getInit()->isValueDependent() &&
-            VD->evaluateValue())
-          return revisit(VD);
+            VD->getInit() && !VD->getInit()->isValueDependent()) {
+
+          if (VD->evaluateValue())
+            return revisit(VD);
+          return this->emitInvalidDeclRef(cast<DeclRefExpr>(E), E);
+        }
       }
     } else {
       if (const auto *VD = dyn_cast<VarDecl>(D);
diff --git a/clang/lib/AST/ByteCode/Pointer.cpp b/clang/lib/AST/ByteCode/Pointer.cpp
index a52f0e336ef298..75b00dcb2ab242 100644
--- a/clang/lib/AST/ByteCode/Pointer.cpp
+++ b/clang/lib/AST/ByteCode/Pointer.cpp
@@ -253,11 +253,6 @@ APValue Pointer::toAPValue(const ASTContext &ASTCtx) const {
     }
   }
 
-  // FIXME(perf): We compute the lvalue path above, but we can't supply it
-  // for dummy pointers (that causes crashes later in CheckConstantExpression).
-  if (isDummy())
-    Path.clear();
-
   // We assemble the LValuePath starting from the innermost pointer to the
   // outermost one. SO in a.b.c, the first element in Path will refer to
   // the field 'c', while later code expects it to refer to 'a'.
diff --git a/clang/test/AST/ByteCode/cxx1z.cpp b/clang/test/AST/ByteCode/cxx1z.cpp
index 2b5d215f016548..1a06597fa348fe 100644
--- a/clang/test/AST/ByteCode/cxx1z.cpp
+++ b/clang/test/AST/ByteCode/cxx1z.cpp
@@ -10,3 +10,6 @@ namespace Temp {
   A<int &, addr({}).n> c; // both-error {{reference to subobject of temporary object}}
   A<int *, &addr({}).n> d; // both-error {{pointer to subobject of temporary object}}
 }
+
+char arr[3];
+A<const char*, &arr[1]> d; // both-error {{refers to subobject '&arr[1]'}}

@tbaederr tbaederr merged commit 36b0707 into llvm:main Oct 11, 2024
DanielCChen pushed a commit to DanielCChen/llvm-project that referenced this pull request Oct 16, 2024
Not doing this is wrong in general and we need to reject expressions
where it would matter differently.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants