refactor: Remove last bits of direct connection between DVarValue and VarDeclaration

The relationship between them is at best tenuous; DVarValue should
probably be renamed to DLValue and is used right now to describe
a general lvalue, of which variables are just one example.

gen/dvalue.cpp

@@ -41,29 +41,19 @@ static bool checkVarValueType(LLType *t, bool extraDeref) {
   return true;
 }
 
-DVarValue::DVarValue(Type *t, VarDeclaration *vd, LLValue *llvmValue)
+DVarValue::DVarValue(Type *t, LLValue *llvmValue, bool isSpecialRefVar)
-    : DValue(t), var(vd), val(llvmValue) {
+    : DValue(t), val(llvmValue), isSpecialRefVar(isSpecialRefVar) {
-  assert(checkVarValueType(llvmValue->getType(), isSpecialRefVar(vd)));
+  assert(llvmValue && "Unexpected null llvm::Value.");
+  assert(checkVarValueType(llvmValue->getType(), isSpecialRefVar));
 }
 
-DVarValue::DVarValue(Type *t, LLValue *llvmValue)
+LLValue *DVarValue::getLVal() { return isSpecialRefVar ? DtoLoad(val) : val; }
-    : DValue(t), var(nullptr), val(llvmValue) {
-  assert(checkVarValueType(llvmValue->getType(), false));
-}
-
-LLValue *DVarValue::getLVal() {
-  assert(val);
-  if (var && isSpecialRefVar(var)) {
-    return DtoLoad(val);
-  }
-  return val;
-}
 
 LLValue *DVarValue::getRVal() {
   assert(val);
 
   llvm::Value *storage = val;
-  if (var && isSpecialRefVar(var)) {
+  if (isSpecialRefVar) {
     storage = DtoLoad(storage);
   }
 
@@ -83,8 +73,7 @@ LLValue *DVarValue::getRVal() {
 }
 
 LLValue *DVarValue::getRefStorage() {
-  assert(val);
+  assert(isSpecialRefVar);
-  assert(isSpecialRefVar(var));
   return val;
 }
 

gen/dvalue.h

@@ -112,11 +112,14 @@ public:
   DNullValue *isNull() override { return this; }
 };
 
-// variable d-value
+/// This is really a misnomer, DVarValue represents generic lvalues, which
+/// might or might not come from variable declarations.
+// TODO: Rename this, probably remove getLVal() from parent since this is the
+// only lvalue. The isSpecialRefVar case should probably also be its own
+// subclass.
 class DVarValue : public DValue {
 public:
-  DVarValue(Type *t, VarDeclaration *vd, llvm::Value *llvmValue);
+  DVarValue(Type *t, llvm::Value *llvmValue, bool isSpecialRefVar = false);
-  DVarValue(Type *t, llvm::Value *llvmValue);
 
   bool isLVal() override { return true; }
   llvm::Value *getLVal() override;
@@ -124,13 +127,13 @@ public:
 
   /// Returns the underlying storage for special internal ref variables.
   /// Illegal to call on any other value.
-  virtual llvm::Value *getRefStorage();
+  llvm::Value *getRefStorage();
 
   DVarValue *isVar() override { return this; }
 
 protected:
-  VarDeclaration *var;
+  llvm::Value *const val;
-  llvm::Value *val;
+  bool const isSpecialRefVar;
 };
 
 // slice d-value

gen/llvmhelpers.cpp

@@ -989,7 +989,7 @@ DValue *DtoDeclarationExp(Dsymbol *declaration) {
     } else {
       DtoVarDeclaration(vd);
     }
-    return new DVarValue(vd->type, vd, getIrValue(vd));
+    return makeVarDValue(vd->type, vd);
   }
   // struct declaration
   if (StructDeclaration *s = declaration->isStructDeclaration()) {
@@ -1496,25 +1496,29 @@ DValue *DtoSymbolAddress(Loc &loc, Type *type, Declaration *decl) {
     if (vd->ident == Id::_arguments && gIR->func()->_arguments) {
       Logger::println("Id::_arguments");
       LLValue *v = gIR->func()->_arguments;
-      return new DVarValue(type, vd, v);
+      assert(!isSpecialRefVar(vd) && "Code not expected to handle special ref "
+                                     "vars, although it can easily be made "
+                                     "to.");
+      return new DVarValue(type, v);
     }
     // _argptr
     if (vd->ident == Id::_argptr && gIR->func()->_argptr) {
       Logger::println("Id::_argptr");
       LLValue *v = gIR->func()->_argptr;
-      return new DVarValue(type, vd, v);
+      assert(!isSpecialRefVar(vd) && "Code not expected to handle special ref "
+                                     "vars, although it can easily be made "
+                                     "to.");
+      return new DVarValue(type, v);
     }
     // _dollar
     if (vd->ident == Id::dollar) {
       Logger::println("Id::dollar");
-      LLValue *val = nullptr;
+      if (isIrVarCreated(vd)) {
-      if (isIrVarCreated(vd) && (val = getIrValue(vd))) {
+        // This is the length of a range.
-        // It must be length of a range
+        return makeVarDValue(type, vd);
-        return new DVarValue(type, vd, val);
       }
       assert(!gIR->arrays.empty());
-      val = DtoArrayLen(gIR->arrays.back());
+      return new DImValue(type, DtoArrayLen(gIR->arrays.back()));
-      return new DImValue(type, val);
     }
     // typeinfo
     if (TypeInfoDeclaration *tid = vd->isTypeInfoDeclaration()) {
@@ -1551,7 +1555,10 @@ DValue *DtoSymbolAddress(Loc &loc, Type *type, Declaration *decl) {
       }
       if (vd->isRef() || vd->isOut() || DtoIsInMemoryOnly(vd->type) ||
           llvm::isa<llvm::AllocaInst>(getIrValue(vd))) {
-        return new DVarValue(type, vd, getIrValue(vd));
+        assert(!isSpecialRefVar(vd) && "Code not expected to handle special "
+                                       "ref vars, although it can easily be "
+                                       "made to.");
+        return new DVarValue(type, getIrValue(vd));
       }
       if (llvm::isa<llvm::Argument>(getIrValue(vd))) {
         return new DImValue(type, getIrValue(vd));
@@ -1562,32 +1569,11 @@ DValue *DtoSymbolAddress(Loc &loc, Type *type, Declaration *decl) {
       Logger::println("a normal variable");
 
       // take care of forward references of global variables
-      const bool isGlobal = vd->isDataseg() || (vd->storage_class & STCextern);
+      if (vd->isDataseg() || (vd->storage_class & STCextern)) {
-      if (isGlobal) {
         DtoResolveVariable(vd);
       }
 
-      assert(isIrVarCreated(vd) && "Variable not resolved.");
+      return makeVarDValue(type, vd);
-
-      llvm::Value *val = getIrValue(vd);
-      assert(val && "Variable value not set yet.");
-
-      if (isGlobal) {
-        llvm::Type *expectedType =
-            llvm::PointerType::getUnqual(DtoMemType(type));
-        // The type of globals is determined by their initializer, so
-        // we might need to cast. Make sure that the type sizes fit -
-        // '==' instead of '<=' should probably work as well.
-        if (val->getType() != expectedType) {
-          llvm::Type *t =
-              llvm::cast<llvm::PointerType>(val->getType())->getElementType();
-          assert(getTypeStoreSize(DtoType(type)) <= getTypeStoreSize(t) &&
-                 "Global type mismatch, encountered type too small.");
-          val = DtoBitCast(val, expectedType);
-        }
-      }
-
-      return new DVarValue(type, vd, val);
     }
   }
 
@@ -1782,3 +1768,28 @@ unsigned getFieldGEPIndex(AggregateDeclaration *ad, VarDeclaration *vd) {
   assert(byteOffset == 0 && "Cannot address field by a simple GEP.");
   return fieldIndex;
 }
+
+DValue *makeVarDValue(Type *type, VarDeclaration *vd, llvm::Value *storage) {
+  auto val = storage;
+  if (!val) {
+    assert(isIrVarCreated(vd) && "Variable not resolved.");
+    val = getIrValue(vd);
+  }
+
+  if (vd->isDataseg() || (vd->storage_class & STCextern)) {
+    // The type of globals is determined by their initializer, so
+    // we might need to cast. Make sure that the type sizes fit -
+    // '==' instead of '<=' should probably work as well.
+    llvm::Type *expectedType = llvm::PointerType::getUnqual(DtoMemType(type));
+
+    if (val->getType() != expectedType) {
+      llvm::Type *t =
+          llvm::cast<llvm::PointerType>(val->getType())->getElementType();
+      assert(getTypeStoreSize(DtoType(type)) <= getTypeStoreSize(t) &&
+             "Global type mismatch, encountered type too small.");
+      val = DtoBitCast(val, expectedType);
+    }
+  }
+
+  return new DVarValue(type, val, isSpecialRefVar(vd));
+}

gen/llvmhelpers.h

@@ -16,11 +16,11 @@
 #ifndef LDC_GEN_LLVMHELPERS_H
 #define LDC_GEN_LLVMHELPERS_H
 
-#include "mtype.h"
-#include "statement.h"
 #include "gen/dvalue.h"
 #include "gen/llvm.h"
 #include "ir/irfuncty.h"
+#include "mtype.h"
+#include "statement.h"
 
 // dynamic memory helpers
 LLValue *DtoNew(Loc &loc, Type *newtype);
@@ -257,4 +257,11 @@ DValue *toElem(Expression *e, bool tryGetLvalue);
 DValue *toElemDtor(Expression *e);
 LLConstant *toConstElem(Expression *e, IRState *p);
 
+/// Creates a DVarValue for the given VarDeclaration.
+///
+/// If the storage is not given explicitly, the declaration is expected to be
+/// already resolved, and the value from the associated IrVar will be used.
+DValue *makeVarDValue(Type *type, VarDeclaration *vd,
+                      llvm::Value *storage = nullptr);
+
 #endif

gen/nested.cpp

@@ -62,8 +62,7 @@ DValue *DtoNestedVariable(Loc &loc, Type *astype, VarDeclaration *vd,
     if (fd->isStatic()) {
       error(loc, "function %s cannot access frame of function %s",
             irfunc->decl->toPrettyChars(), vdparent->toPrettyChars());
-      return new DVarValue(astype, vd,
+      return new DVarValue(astype, llvm::UndefValue::get(DtoPtrToType(astype)));
-                           llvm::UndefValue::get(DtoPtrToType(astype)));
     }
     fd = getParentFunc(fd, false);
     assert(fd);
@@ -71,8 +70,7 @@ DValue *DtoNestedVariable(Loc &loc, Type *astype, VarDeclaration *vd,
 
   // is the nested variable in this scope?
   if (vdparent == irfunc->decl) {
-    LLValue *val = getIrValue(vd);
+    return makeVarDValue(astype, vd);
-    return new DVarValue(astype, vd, val);
   }
 
   LLValue *dwarfValue = nullptr;
@@ -120,8 +118,9 @@ DValue *DtoNestedVariable(Loc &loc, Type *astype, VarDeclaration *vd,
     Logger::cout() << "of type: " << *irfunc->frameType << '\n';
   }
 
-  unsigned vardepth = getIrLocal(vd)->nestedDepth;
+  IrLocal *const irLocal = getIrLocal(vd);
-  unsigned funcdepth = irfunc->depth;
+  const auto vardepth = irLocal->nestedDepth;
+  const auto funcdepth = irfunc->depth;
 
   IF_LOG {
     Logger::cout() << "Variable: " << vd->toChars() << '\n';
@@ -148,7 +147,7 @@ DValue *DtoNestedVariable(Loc &loc, Type *astype, VarDeclaration *vd,
     IF_LOG Logger::cout() << "Frame: " << *val << '\n';
   }
 
-  int idx = getIrLocal(vd)->nestedIndex;
+  const auto idx = irLocal->nestedIndex;
   assert(idx != -1 && "Nested context not yet resolved for variable.");
 
   if (dwarfValue && global.params.symdebug) {
@@ -165,8 +164,8 @@ DValue *DtoNestedVariable(Loc &loc, Type *astype, VarDeclaration *vd,
     val = DtoAlignedLoad(val);
     // dwarfOpDeref(dwarfAddr);
     IF_LOG {
-      Logger::cout() << "Was byref, now: " << *val << '\n';
+      Logger::cout() << "Was byref, now: " << *irLocal->value << '\n';
-      Logger::cout() << "of type: " << *val->getType() << '\n';
+      Logger::cout() << "of type: " << *irLocal->value->getType() << '\n';
     }
   }
 
@@ -174,7 +173,7 @@ DValue *DtoNestedVariable(Loc &loc, Type *astype, VarDeclaration *vd,
     gIR->DBuilder.EmitLocalVariable(dwarfValue, vd, nullptr, false, dwarfAddr);
   }
 
-  return new DVarValue(astype, vd, val);
+  return makeVarDValue(astype, vd, val);
 }
 
 void DtoResolveNestedContext(Loc &loc, AggregateDeclaration *decl,

gen/toir.cpp

@@ -156,7 +156,9 @@ static void write_struct_literal(Loc loc, LLValue *mem, StructDeclaration *sd,
     }
 
     // get a pointer to this field
-    DVarValue field(vd->type, vd, DtoIndexAggregate(mem, sd, vd));
+    assert(!isSpecialRefVar(vd) && "Code not expected to handle special ref "
+                                   "vars, although it can easily be made to.");
+    DVarValue field(vd->type, DtoIndexAggregate(mem, sd, vd));
 
     // store the initializer there
     DtoAssign(loc, &field, val, TOKconstruct, true);
@@ -1143,9 +1145,10 @@ public:
     if (e->cachedLvalue) {
       Logger::println("using cached lvalue");
       LLValue *V = e->cachedLvalue;
-      VarDeclaration *vd = e->var->isVarDeclaration();
+      assert(!isSpecialRefVar(e->var->isVarDeclaration()) &&
-      assert(vd);
+             "Code not expected to handle special ref vars, although it can "
-      result = new DVarValue(e->type, vd, V);
+             "easily be made to.");
+      result = new DVarValue(e->type, V);
       return;
     }
 
@@ -1178,7 +1181,7 @@ public:
       }
 
       // Logger::cout() << "mem: " << *arrptr << '\n';
-      result = new DVarValue(e->type, vd, arrptr);
+      result = new DVarValue(e->type, arrptr);
     } else if (FuncDeclaration *fdecl = e->var->isFuncDeclaration()) {
       DtoResolveFunction(fdecl);
 
@@ -1240,7 +1243,10 @@ public:
         Logger::println("normal this exp");
         v = p->func()->thisArg;
       }
-      result = new DVarValue(e->type, vd, v);
+      assert(!isSpecialRefVar(vd) && "Code not expected to handle special ref "
+                                     "vars, although it can easily be made "
+                                     "to.");
+      result = new DVarValue(e->type, v);
     } else {
       llvm_unreachable("No VarDeclaration in ThisExp.");
     }