From 5c285c8fa3d707c9c8dc1dc80ff17e2d826b47f7 Mon Sep 17 00:00:00 2001
From: Dennis Korpel <dennis@sarc.nl>
Date: Tue, 24 Dec 2024 11:25:35 +0100
Subject: [PATCH] Refactor 'expressionSemanticDone' logic into a function

---
 compiler/src/dmd/expression.d    |  7 ++++++-
 compiler/src/dmd/expressionsem.d | 31 +++++++++++++++----------------
 2 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/compiler/src/dmd/expression.d b/compiler/src/dmd/expression.d
index 82d1d446515..355bdc42468 100644
--- a/compiler/src/dmd/expression.d
+++ b/compiler/src/dmd/expression.d
@@ -293,7 +293,12 @@ enum WANTexpand = 1;    // expand const/immutable variables if possible
  */
 extern (C++) abstract class Expression : ASTNode
 {
-    Type type;      // !=null means that semantic() has been run
+    /// Usually, this starts out as `null` and gets set to the final expression type by
+    /// `expressionSemantic`. However, for some expressions (such as `TypeExp`,`RealExp`,
+    /// `VarExp`), the field can get set to an assigned type before running semantic.
+    /// See `expressionSemanticDone`
+    Type type;
+
     Loc loc;        // file location
     const EXP op;   // to minimize use of dynamic_cast
     bool parens;    // if this is a parenthesized expression
diff --git a/compiler/src/dmd/expressionsem.d b/compiler/src/dmd/expressionsem.d
index ce9cb75b75b..c42dedf85d9 100644
--- a/compiler/src/dmd/expressionsem.d
+++ b/compiler/src/dmd/expressionsem.d
@@ -14010,25 +14010,24 @@ Expression binSemanticProp(BinExp e, Scope* sc)
     return null;
 }
 
+/// Returns: whether expressionSemantic() has been run on expression `e`
+private bool expressionSemanticDone(Expression e)
+{
+    // Usually, Expression.type gets set by expressionSemantic and is `null` beforehand
+    // There are some exceptions however:
+    return e.type !is null && !(
+        e.isRealExp() // type sometimes gets set already before semantic
+        || e.isTypeExp() // stores its type in the Expression.type field
+        || e.isCompoundLiteralExp() // stores its `(type) {}` in type field, gets rewritten to struct literal
+        || e.isVarExp() // type sometimes gets set already before semantic
+    );
+}
+
 // entrypoint for semantic ExpressionSemanticVisitor
 Expression expressionSemantic(Expression e, Scope* sc)
 {
-    if (e.type)
-    {
-        // When the expression has a type, it usually means that semantic
-        // has already been run yet.
-        // For these expressions, expressionsemantic is not idempotent yet,
-        // and the test suite fails without these. TODO: fix the code/tests
-        // so that it doesn't rely on semantic running multiple times anymore.
-        if (!e.isRealExp()
-            && !e.isCompoundLiteralExp()
-            && !e.isTypeExp()
-            && !e.isVarExp()
-        )
-        {
-            return e;
-        }
-    }
+    if (e.expressionSemanticDone)
+        return e;
 
     scope v = new ExpressionSemanticVisitor(sc);
     e.accept(v);