Skip to content

Conversation

@tbaederr
Copy link
Contributor

@tbaederr tbaederr commented Mar 8, 2025

Same thing we now do in if statements. Check the condition of a conditional operator for a statically known true/false value.

Same thing we now do in if statements. Check the condition of a
conditional operator for a statically known true/false value.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Mar 8, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 8, 2025

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

Same thing we now do in if statements. Check the condition of a conditional operator for a statically known true/false value.


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

1 Files Affected:

  • (modified) clang/lib/AST/ByteCode/Compiler.cpp (+29-19)
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index d192b8b91c72e..9a52dd4105437 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -25,6 +25,16 @@ using APSInt = llvm::APSInt;
 namespace clang {
 namespace interp {
 
+static std::optional<bool> getBoolValue(const Expr *E) {
+  if (const auto *CE = dyn_cast_if_present<ConstantExpr>(E);
+      CE && CE->hasAPValueResult() &&
+      CE->getResultAPValueKind() == APValue::ValueKind::Int) {
+    return CE->getResultAsAPSInt().getBoolValue();
+  }
+
+  return std::nullopt;
+}
+
 /// Scope used to handle temporaries in toplevel variable declarations.
 template <class Emitter> class DeclScope final : public LocalScope<Emitter> {
 public:
@@ -2286,6 +2296,19 @@ bool Compiler<Emitter>::VisitAbstractConditionalOperator(
   const Expr *TrueExpr = E->getTrueExpr();
   const Expr *FalseExpr = E->getFalseExpr();
 
+  auto visitChildExpr = [&](const Expr *E) -> bool {
+    LocalScope<Emitter> S(this);
+    if (!this->delegate(E))
+      return false;
+    return S.destroyLocals();
+  };
+
+  if (std::optional<bool> BoolValue = getBoolValue(Condition)) {
+    if (BoolValue)
+      return visitChildExpr(TrueExpr);
+    return visitChildExpr(FalseExpr);
+  }
+
   LabelTy LabelEnd = this->getLabel();   // Label after the operator.
   LabelTy LabelFalse = this->getLabel(); // Label for the false expr.
 
@@ -2295,26 +2318,16 @@ bool Compiler<Emitter>::VisitAbstractConditionalOperator(
   if (!this->jumpFalse(LabelFalse))
     return false;
 
-  {
-    LocalScope<Emitter> S(this);
-    if (!this->delegate(TrueExpr))
-      return false;
-    if (!S.destroyLocals())
-      return false;
-  }
+  if (!visitChildExpr(TrueExpr))
+    return false;
 
   if (!this->jump(LabelEnd))
     return false;
 
   this->emitLabel(LabelFalse);
 
-  {
-    LocalScope<Emitter> S(this);
-    if (!this->delegate(FalseExpr))
-      return false;
-    if (!S.destroyLocals())
-      return false;
-  }
+  if (!visitChildExpr(FalseExpr))
+    return false;
 
   this->fallthrough(LabelEnd);
   this->emitLabel(LabelEnd);
@@ -5207,11 +5220,8 @@ template <class Emitter> bool Compiler<Emitter>::visitIfStmt(const IfStmt *IS) {
   // stataically known to be either true or false. We could look at more cases
   // here, but I think all the ones that actually happen are using a
   // ConstantExpr.
-  if (const auto *CE = dyn_cast_if_present<ConstantExpr>(IS->getCond());
-      CE && CE->hasAPValueResult() &&
-      CE->getResultAPValueKind() == APValue::ValueKind::Int) {
-    APSInt Value = CE->getResultAsAPSInt();
-    if (Value.getBoolValue())
+  if (std::optional<bool> BoolValue = getBoolValue(IS->getCond())) {
+    if (*BoolValue)
       return visitChildStmt(IS->getThen());
     else if (const Stmt *Else = IS->getElse())
       return visitChildStmt(Else);

@tbaederr tbaederr merged commit e4fe22a into llvm:main Mar 8, 2025
14 checks passed
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