Skip to content
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

Check whether EvaluatedStmt::Value is valid in VarDecl::hasInit #94515

Merged
merged 2 commits into from
Jun 14, 2024

Conversation

ahatanak
Copy link
Collaborator

@ahatanak ahatanak commented Jun 5, 2024

VarDecl::isNull() doesn't tell whether the VarDecl has an initializer as methods like ensureEvaluatedStmt can create an EvaluatedStmt even when there isn't an initializer.

Revert e1c3e16 as the change isn't needed anymore with this change.

See the discussion in #93749.

VarDecl::isNull() doesn't tell whether the VarDecl has an initializer as
methods like ensureEvaluatedStmt can create an EvaluatedStmt even when
there isn't an initializer.

Revert e1c3e16 as the change isn't
needed anymore with this change.

See the discussion in llvm#93749.
@ahatanak ahatanak requested review from mizvekov and zygoloid June 5, 2024 18:29
@ahatanak ahatanak marked this pull request as ready for review June 5, 2024 18:30
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jun 5, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 5, 2024

@llvm/pr-subscribers-clang

Author: Akira Hatanaka (ahatanak)

Changes

VarDecl::isNull() doesn't tell whether the VarDecl has an initializer as methods like ensureEvaluatedStmt can create an EvaluatedStmt even when there isn't an initializer.

Revert e1c3e16 as the change isn't needed anymore with this change.

See the discussion in #93749.


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

1 Files Affected:

  • (modified) clang/lib/AST/Decl.cpp (+6-4)
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 1f19dadafa44e..fc04f877b2268 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -2390,6 +2390,9 @@ bool VarDecl::hasInit() const {
     if (P->hasUnparsedDefaultArg() || P->hasUninstantiatedDefaultArg())
       return false;
 
+  if (auto *Eval = getEvaluatedStmt())
+    return Eval->Value.isValid();
+
   return !Init.isNull();
 }
 
@@ -2402,10 +2405,9 @@ Expr *VarDecl::getInit() {
 
   auto *Eval = getEvaluatedStmt();
 
-  return cast_if_present<Expr>(
-      Eval->Value.isOffset()
-          ? Eval->Value.get(getASTContext().getExternalSource())
-          : Eval->Value.get(nullptr));
+  return cast<Expr>(Eval->Value.isOffset()
+                        ? Eval->Value.get(getASTContext().getExternalSource())
+                        : Eval->Value.get(nullptr));
 }
 
 Stmt **VarDecl::getInitAddress() {

Copy link
Contributor

@mizvekov mizvekov left a comment

Choose a reason for hiding this comment

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

This is much better, thanks!

LGTM

Comment on lines 2408 to 2410
return cast<Expr>(Eval->Value.isOffset()
? Eval->Value.get(getASTContext().getExternalSource())
: Eval->Value.get(nullptr));
Copy link
Contributor

Choose a reason for hiding this comment

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

Small nit: It seems this could move the conditional into the argument for Eval->Value.get.

@ahatanak ahatanak merged commit 46c8f25 into llvm:main Jun 14, 2024
7 checks passed
@ahatanak ahatanak deleted the fix-has-init branch June 14, 2024 18:22
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.

3 participants