[TableGen] Unify the priority of variables

[• pcwang-thead]

Summary

In D148197, we have made defvar statement able to refer to class
template arguments. However, the priority of class/multiclass
template argument is higher than variables defined by defvar, which
is a little counterintuitive.

In this patch, we unify the priority of variables. Each pair of
braces introduces a new scope, which may contain some additional
variables like template arguments, loop iterators, etc. We can
define local variables inside this scope via defvar and these
variables are of higher priority than additional variables. This
means that defvar will shadow additional variables with the same
name. The scope can be nested, and we use the innermost variable.

This make variables defined by defvar prior to class/multiclass
template arguments, loop iterators, etc. The shadow rules now are:

• V in a record body shadows a global V.

• V in a record body shadows template argument V.

• V in template arguments shadows a global V.

• V in a foreach statement list shadows any V in surrounding record or global scopes.

[tra]

LGTM with a test nit.

llvm/test/TableGen/defvar.td

155

Can we make the checks more granular? While the failure here will catch the problem, it will be hard to tell which part, exactly got broken.

Having an individual test for each of the shadowing scenarious would be useful, IMO.

[• pcwang-thead]

• Rebase
• Add more tests.
• Unify the priority of variables.

[• pcwang-thead]

@tra Can you review this again? I have made some big changes to unify the implementation. Thanks in advance!

[• pcwang-thead]

Fix typo.

[• pcwang-thead]

Fix wrong qualified argument name inside multiclass.

[• pcwang-thead]

Ping

[tra]

Still LGTM.

[• pcwang-thead]

In D149016#4365064, @tra wrote:

Still LGTM.

Thanks!

llvm/docs/TableGen/ProgRef.rst

 The following bang operators are useful for working with DAGs:
 ``!con``, ``!dag``, ``!empty``, ``!foreach``, ``!getdagop``, ``!setdagop``, ``!size``.
 
 Defvar in a record body
 -----------------------
 
 In addition to defining global variables, the ``defvar`` statement can
 be used inside the :token:`Body` of a class or record definition to define
-local variables. The scope of the variable extends from the ``defvar``
-statement to the end of the body. It cannot be set to a different value
-within its scope. The ``defvar`` statement can also be used in the statement
+local variables. Template arguments of ``class`` or ``multiclass`` can be
+used in the value expression. The scope of the variable extends from the
+``defvar`` statement to the end of the body. It cannot be set to a different
+value within its scope. The ``defvar`` statement can also be used in the statement
 list of a ``foreach``, which establishes a scope.
 
 A variable named ``V`` in an inner scope shadows (hides) any variables ``V``
-in outer scopes. In particular, ``V`` in a record body shadows a global
-``V``, and ``V`` in a ``foreach`` statement list shadows any ``V`` in
-surrounding record or global scopes.
+in outer scopes. In particular, there are several cases:
+
+* ``V`` in a record body shadows a global ``V``.
+
+* ``V`` in a record body shadows template argument ``V``.
+
+* ``V`` in template arguments shadows a global ``V``.
+
+* ``V`` in a ``foreach`` statement list shadows any ``V`` in surrounding record or
+  global scopes.
 
 Variables defined in a ``foreach`` go out of scope at the end of
 each loop iteration, so their value in one iteration is not available in
 the next iteration.  The following ``defvar`` will not work::
 
   defvar i = !add(i, 1)
 
 How records are built

llvm/lib/TableGen/TGParser.h

namespace llvm {
   };
 
   struct DefsetRecord {
     SMLoc Loc;
     RecTy *EltTy = nullptr;
     SmallVector<Init *, 16> Elements;
   };
 
-class TGLocalVarScope {
-  // A scope to hold local variable definitions from defvar.
-  std::map<std::string, Init *, std::less<>> vars;
-  std::unique_ptr<TGLocalVarScope> parent;
+  struct MultiClass {
+    Record Rec; // Placeholder for template args and Name.
+    std::vector<RecordsEntry> Entries;
 
+    void dump() const;
+
+    MultiClass(StringRef Name, SMLoc Loc, RecordKeeper &Records)
+        : Rec(Name, Loc, Records) {}
+  };
+
+  class TGVarScope {
   public:
-  TGLocalVarScope() = default;
-  TGLocalVarScope(std::unique_ptr<TGLocalVarScope> parent)
-      : parent(std::move(parent)) {}
+    enum ScopeKind { SK_Local, SK_Record, SK_ForeachLoop, SK_MultiClass };
+
+  private:
+    ScopeKind Kind;
+    std::unique_ptr<TGVarScope> Parent;
+    // A scope to hold variable definitions from defvar.
+    std::map<std::string, Init *, std::less<>> Vars;
+    Record *CurRec = nullptr;
+    ForeachLoop *CurLoop = nullptr;
+    MultiClass *CurMultiClass = nullptr;
 
-  std::unique_ptr<TGLocalVarScope> extractParent() {
+  public:
+    TGVarScope(std::unique_ptr<TGVarScope> Parent)
+        : Kind(SK_Local), Parent(std::move(Parent)) {}
+    TGVarScope(std::unique_ptr<TGVarScope> Parent, Record *Rec)
+        : Kind(SK_Record), Parent(std::move(Parent)), CurRec(Rec) {}
+    TGVarScope(std::unique_ptr<TGVarScope> Parent, ForeachLoop *Loop)
+        : Kind(SK_ForeachLoop), Parent(std::move(Parent)), CurLoop(Loop) {}
+    TGVarScope(std::unique_ptr<TGVarScope> Parent, MultiClass *Multiclass)
+        : Kind(SK_MultiClass), Parent(std::move(Parent)),
+          CurMultiClass(Multiclass) {}
+
+    std::unique_ptr<TGVarScope> extractParent() {
       // This is expected to be called just before we are destructed, so
       // it doesn't much matter what state we leave 'parent' in.
-    return std::move(parent);
+      return std::move(Parent);
     }
 
-  Init *getVar(StringRef Name) const {
-    auto It = vars.find(Name);
-    if (It != vars.end())
-      return It->second;
-    if (parent)
-      return parent->getVar(Name);
-    return nullptr;
-  }
+    Init *getVar(RecordKeeper &Records, MultiClass *ParsingMultiClass,
+                 StringInit *Name, SMRange NameLoc,
+                 bool TrackReferenceLocs) const;
 
     bool varAlreadyDefined(StringRef Name) const {
       // When we check whether a variable is already defined, for the purpose of
       // reporting an error on redefinition, we don't look up to the parent
       // scope, because it's all right to shadow an outer definition with an
       // inner one.
-    return vars.find(Name) != vars.end();
+      return Vars.find(Name) != Vars.end();
     }
 
     void addVar(StringRef Name, Init *I) {
-    bool Ins = vars.insert(std::make_pair(std::string(Name), I)).second;
+      bool Ins = Vars.insert(std::make_pair(std::string(Name), I)).second;
       (void)Ins;
       assert(Ins && "Local variable already exists");
     }
-};
-
-struct MultiClass {
-  Record Rec;  // Placeholder for template args and Name.
-  std::vector<RecordsEntry> Entries;
 
-  void dump() const;
-
-  MultiClass(StringRef Name, SMLoc Loc, RecordKeeper &Records) :
-    Rec(Name, Loc, Records) {}
+    bool isOutermost() const { return Parent == nullptr; }
   };
 
 class TGParser {
   TGLexer Lex;
   std::vector<SmallVector<LetRecord, 4>> LetStack;
   std::map<std::string, std::unique_ptr<MultiClass>> MultiClasses;
 
   /// Loops - Keep track of any foreach loops we are within.
   ///
   std::vector<std::unique_ptr<ForeachLoop>> Loops;
 
   SmallVector<DefsetRecord *, 2> Defsets;
 
   /// CurMultiClass - If we are parsing a 'multiclass' definition, this is the
   /// current value.
   MultiClass *CurMultiClass;
 
-  /// CurLocalScope - Innermost of the current nested scopes for 'defvar' local
-  /// variables.
-  std::unique_ptr<TGLocalVarScope> CurLocalScope;
+  /// CurScope - Innermost of the current nested scopes for 'defvar' variables.
+  std::unique_ptr<TGVarScope> CurScope;
 
   // Record tracker
   RecordKeeper &Records;
 
   // A "named boolean" indicating how to parse identifiers.  Usually
   // identifiers map to some existing object but in special cases
   // (e.g. parsing def names) no such object exists yet because we are
   // in the middle of creating in.  For those situations, allow the
public:
   }
   bool TokError(const Twine &Msg) const {
     return Error(Lex.getLoc(), Msg);
   }
   const TGLexer::DependenciesSetTy &getDependencies() const {
     return Lex.getDependencies();
   }
 
-  TGLocalVarScope *PushLocalScope() {
-    CurLocalScope = std::make_unique<TGLocalVarScope>(std::move(CurLocalScope));
+  TGVarScope *PushScope() {
+    CurScope = std::make_unique<TGVarScope>(std::move(CurScope));
     // Returns a pointer to the new scope, so that the caller can pass it back
-    // to PopLocalScope which will check by assertion that the pushes and pops
+    // to PopScope which will check by assertion that the pushes and pops
     // match up properly.
-    return CurLocalScope.get();
+    return CurScope.get();
+  }
+  TGVarScope *PushScope(Record *Rec) {
+    CurScope = std::make_unique<TGVarScope>(std::move(CurScope), Rec);
+    return CurScope.get();
+  }
+  TGVarScope *PushScope(ForeachLoop *Loop) {
+    CurScope = std::make_unique<TGVarScope>(std::move(CurScope), Loop);
+    return CurScope.get();
+  }
+  TGVarScope *PushScope(MultiClass *Multiclass) {
+    CurScope = std::make_unique<TGVarScope>(std::move(CurScope), Multiclass);
+    return CurScope.get();
   }
-  void PopLocalScope(TGLocalVarScope *ExpectedStackTop) {
-    assert(ExpectedStackTop == CurLocalScope.get() &&
+  void PopScope(TGVarScope *ExpectedStackTop) {
+    assert(ExpectedStackTop == CurScope.get() &&
            "Mismatched pushes and pops of local variable scopes");
-    CurLocalScope = CurLocalScope->extractParent();
+    CurScope = CurScope->extractParent();
   }
 
 private: // Semantic analysis methods.
   bool AddValue(Record *TheRec, SMLoc Loc, const RecordVal &RV);
   /// Set the value of a RecordVal within the given record. If `OverrideDefLoc`
   /// is set, the provided location overrides any existing location of the
   /// RecordVal.
   bool SetValue(Record *TheRec, SMLoc Loc, Init *ValName,

llvm/lib/TableGen/TGParser.cpp

static Init *QualifiedNameOfImplicitName(Record &Rec,
   return QualifyName(Rec, MC, StringInit::get(Rec.getRecords(), "NAME"),
                      MC ? "::" : ":");
 }
 
 static Init *QualifiedNameOfImplicitName(MultiClass *MC) {
   return QualifiedNameOfImplicitName(MC->Rec, MC);
 }
 
+Init *TGVarScope::getVar(RecordKeeper &Records, MultiClass* ParsingMultiClass,
+                         StringInit *Name, SMRange NameLoc,
+                         bool TrackReferenceLocs) const {
+  // First, we search in local variables.
+  auto It = Vars.find(Name->getValue());
+  if (It != Vars.end())
+    return It->second;
+
+  std::function<Init *(Record *, StringInit *, StringRef)> FindValueInArgs =
+      [&](Record *Rec, StringInit *Name, StringRef Scoper) -> Init * {
+    if (!Rec)
+      return nullptr;
+    Init *ArgName = QualifyName(*Rec, ParsingMultiClass, Name, Scoper);
+    if (Rec->isTemplateArg(ArgName)) {
+      RecordVal *RV = Rec->getValue(ArgName);
+      assert(RV && "Template arg doesn't exist??");
+      RV->setUsed(true);
+      if (TrackReferenceLocs)
+        RV->addReferenceLoc(NameLoc);
+      return VarInit::get(ArgName, RV->getType());
+    }
+    return Name->getValue() == "NAME"
+               ? VarInit::get(ArgName, StringRecTy::get(Records))
+               : nullptr;
+  };
+
+  // If not found, we try to find the variable in additional variables like
+  // arguments, loop iterator, etc.
+  switch (Kind) {
+  case SK_Local:
+    break; /* do nothing. */
+  case SK_Record: {
+    if (CurRec) {
+      // The variable is a record field?
+      if (RecordVal *RV = CurRec->getValue(Name)) {
+        if (TrackReferenceLocs)
+          RV->addReferenceLoc(NameLoc);
+        return VarInit::get(Name, RV->getType());
+      }
+
+      // The variable is a class template argument?
+      if (CurRec->isClass())
+        if (auto *V = FindValueInArgs(CurRec, Name, ":"))
+          return V;
+    }
+    break;
+  }
+  case SK_ForeachLoop: {
+    // The variable is a loop iterator?
+    if (CurLoop->IterVar) {
+      VarInit *IterVar = dyn_cast<VarInit>(CurLoop->IterVar);
+      if (IterVar && IterVar->getNameInit() == Name)
+        return IterVar;
+    }
+    break;
+  }
+  case SK_MultiClass: {
+    // The variable is a multiclass template argument?
+    if (CurMultiClass)
+      if (auto *V = FindValueInArgs(&CurMultiClass->Rec, Name, "::"))
+        return V;
+    break;
+  }
+  }
+
+  // Then, we try to find the name in parent scope.
+  if (Parent)
+    return Parent->getVar(Records, ParsingMultiClass, Name, NameLoc,
+                          TrackReferenceLocs);
+
+  return nullptr;
+}
+
 bool TGParser::AddValue(Record *CurRec, SMLoc Loc, const RecordVal &RV) {
   if (!CurRec)
     CurRec = &CurMultiClass->Rec;
 
   if (RecordVal *ERV = CurRec->getValue(RV.getNameInit())) {
     // The value already exists in the class, treat this as a set.
     if (ERV->setValue(RV.getValue()))
       return Error(Loc, "New definition of '" + RV.getName() + "' of type '" +
case tgtok::List: {
     return ListRecTy::get(SubType);
   }
   }
 }
 
 /// ParseIDValue
 Init *TGParser::ParseIDValue(Record *CurRec, StringInit *Name, SMRange NameLoc,
                              IDParseMode Mode) {
-  if (CurRec) {
-    if (RecordVal *RV = CurRec->getValue(Name)) {
-      if (TrackReferenceLocs)
-        RV->addReferenceLoc(NameLoc);
-      return VarInit::get(Name, RV->getType());
-    }
-  }
-
-  if ((CurRec && CurRec->isClass()) || CurMultiClass) {
-    Init *TemplateArgName;
-    if (CurMultiClass) {
-      TemplateArgName =
-          QualifyName(CurMultiClass->Rec, CurMultiClass, Name, "::");
-    } else
-      TemplateArgName = QualifyName(*CurRec, CurMultiClass, Name, ":");
-
-    Record *TemplateRec = CurMultiClass ? &CurMultiClass->Rec : CurRec;
-    if (TemplateRec->isTemplateArg(TemplateArgName)) {
-      RecordVal *RV = TemplateRec->getValue(TemplateArgName);
-      assert(RV && "Template arg doesn't exist??");
-      RV->setUsed(true);
-      if (TrackReferenceLocs)
-        RV->addReferenceLoc(NameLoc);
-      return VarInit::get(TemplateArgName, RV->getType());
-    } else if (Name->getValue() == "NAME") {
-      return VarInit::get(TemplateArgName, StringRecTy::get(Records));
-    }
-  }
-
-  if (CurLocalScope)
-    if (Init *I = CurLocalScope->getVar(Name->getValue()))
-      return I;
+  if (Init *I = CurScope->getVar(Records, CurMultiClass, Name, NameLoc,
+                                 TrackReferenceLocs))
 
-  // If this is in a foreach loop, make sure it's not a loop iterator
-  for (const auto &L : Loops) {
-    if (L->IterVar) {
-      VarInit *IterVar = dyn_cast<VarInit>(L->IterVar);
-      if (IterVar && IterVar->getNameInit() == Name)
-        return IterVar;
-    }
-  }
-
   if (Mode == ParseNameMode)
     return Name;
 
   if (Init *I = Records.getGlobal(Name->getValue())) {
     // Add a reference to the global if it's a record.
     if (TrackReferenceLocs) {
       if (auto *Def = dyn_cast<DefInit>(I))
         Def->getDef()->appendReferenceLoc(NameLoc);
case tgtok::XFoldl: {
     // two variables.
     std::unique_ptr<Record> ParseRecTmp;
     Record *ParseRec = CurRec;
     if (!ParseRec) {
       ParseRecTmp = std::make_unique<Record>(".parse", ArrayRef<SMLoc>{}, Records);
       ParseRec = ParseRecTmp.get();
     }
 
+    TGVarScope *FoldScope = PushScope(ParseRec);
     ParseRec->addValue(RecordVal(A, Start->getType(), RecordVal::FK_Normal));
-    ParseRec->addValue(RecordVal(B, ListType->getElementType(),
-                                 RecordVal::FK_Normal));
+    ParseRec->addValue(
+        RecordVal(B, ListType->getElementType(), RecordVal::FK_Normal));
     Init *ExprUntyped = ParseValue(ParseRec);
     ParseRec->removeValue(A);
     ParseRec->removeValue(B);
+    PopScope(FoldScope);
     if (!ExprUntyped)
       return nullptr;
 
     TypedInit *Expr = dyn_cast<TypedInit>(ExprUntyped);
     if (!Expr) {
       TokError("could not get type of !foldl expression");
       return nullptr;
     }
Init *TGParser::ParseOperationForEachFilter(Record *CurRec, RecTy *ItemType) {
   // iteration variable.
   std::unique_ptr<Record> ParseRecTmp;
   Record *ParseRec = CurRec;
   if (!ParseRec) {
     ParseRecTmp =
         std::make_unique<Record>(".parse", ArrayRef<SMLoc>{}, Records);
     ParseRec = ParseRecTmp.get();
   }
-
+  TGVarScope *TempScope = PushScope(ParseRec);
   ParseRec->addValue(RecordVal(LHS, InEltType, RecordVal::FK_Normal));
   Init *RHS = ParseValue(ParseRec, ExprEltType);
   ParseRec->removeValue(LHS);
+  PopScope(TempScope);
   if (!RHS)
     return nullptr;
 
   if (!consume(tgtok::r_paren)) {
     TokError("expected ')' in !foreach/!filter");
     return nullptr;
   }
 
Init *TGParser::ParseDeclaration(Record *CurRec,
   }
 
   std::string Str = Lex.getCurStrVal();
   if (Str == "NAME") {
     TokError("'" + Str + "' is a reserved variable name");
     return nullptr;
   }
 
+  if (!ParsingTemplateArgs && CurScope->varAlreadyDefined(Str)) {
+    TokError("local variable of this name already exists");
+    return nullptr;
+  }
+
   SMLoc IdLoc = Lex.getLoc();
   Init *DeclName = StringInit::get(Records, Str);
   Lex.Lex();
 
   bool BadField;
   if (!ParsingTemplateArgs) { // def, possibly in a multiclass
     BadField = AddValue(CurRec, IdLoc,
                         RecordVal(DeclName, IdLoc, Type,

 bool TGParser::ParseBody(Record *CurRec) {
   // If this is a null definition, just eat the semi and return.
   if (consume(tgtok::semi))
     return false;
 
   if (!consume(tgtok::l_brace))
     return TokError("Expected '{' to start body or ';' for declaration only");
 
-  // An object body introduces a new scope for local variables.
-  TGLocalVarScope *BodyScope = PushLocalScope();
-
   while (Lex.getCode() != tgtok::r_brace)
     if (ParseBodyItem(CurRec))
       return true;
 
-  PopLocalScope(BodyScope);
-
   // Eat the '}'.
   Lex.Lex();
 
   // If we have a semicolon, print a gentle error.
   SMLoc SemiLoc = Lex.getLoc();
   if (consume(tgtok::semi)) {
     PrintError(SemiLoc, "A class or def body should not end with a semicolon");
     PrintNote("Semicolon ignored; remove to eliminate this error");    

 /// that is being parsed.
 ///
 ///   ObjectBody      ::= BaseClassList Body
 ///   BaseClassList   ::= /*empty*/
 ///   BaseClassList   ::= ':' BaseClassListNE
 ///   BaseClassListNE ::= SubClassRef (',' SubClassRef)*
 ///
 bool TGParser::ParseObjectBody(Record *CurRec) {
+  // An object body introduces a new scope for local variables.
+  TGVarScope *ObjectScope = PushScope(CurRec);
   // If there is a baseclass list, read it.
   if (consume(tgtok::colon)) {
 
     // Read all of the subclasses.
     SubClassReference SubClass = ParseSubClassReference(CurRec, false);
     while (true) {
       // Check for error.
       if (!SubClass.Rec) return true;
 
       // Add it.
       if (AddSubClass(CurRec, SubClass))
         return true;
 
       if (!consume(tgtok::comma))
         break;
       SubClass = ParseSubClassReference(CurRec, false);
     }
   }
 
   if (ApplyLetStack(CurRec))
     return true;
 
-  return ParseBody(CurRec);
+  bool Result = ParseBody(CurRec);
+  PopScope(ObjectScope);
+  return Result;
 }
 
 /// ParseDef - Parse and return a top level or multiclass record definition.
 /// Return false if okay, true if error.
 ///
 ///   DefInst ::= DEF ObjectName ObjectBody
 ///
 bool TGParser::ParseDef(MultiClass *CurMultiClass) {

 ///
 bool TGParser::ParseDefvar(Record *CurRec) {
   assert(Lex.getCode() == tgtok::Defvar);
   Lex.Lex(); // Eat the 'defvar' token
 
   if (Lex.getCode() != tgtok::Id)
     return TokError("expected identifier");
   StringInit *DeclName = StringInit::get(Records, Lex.getCurStrVal());
-  if (CurLocalScope) {
-    if (CurLocalScope->varAlreadyDefined(DeclName->getValue()))
-      return TokError("local variable of this name already exists");
-  } else {
-    if (Records.getGlobal(DeclName->getValue()))
-      return TokError("def or global variable of this name already exists");
+  if (CurScope->varAlreadyDefined(DeclName->getValue()))
+
+  // The name should not be conflicted with existed field names.
+  if (CurRec) {
+    auto *V = CurRec->getValue(DeclName->getValue());
+    if (V && !V->isTemplateArg())
+      return TokError("field of this name already exists");
   }
 
+  // If this defvar is in the top level, the name should not be conflicted
+  // with existed global names.
+  if (CurScope->isOutermost() && Records.getGlobal(DeclName->getValue()))
+    return TokError("def or global variable of this name already exists");
+
   Lex.Lex();
   if (!consume(tgtok::equal))
     return TokError("expected '='");
 
   Init *Value = ParseValue(CurRec);
   if (!Value)
     return true;
 
   if (!consume(tgtok::semi))
     return TokError("expected ';'");
 
-  if (CurLocalScope)
-    CurLocalScope->addVar(DeclName->getValue(), Value);
+  if (!CurScope->isOutermost())
+    CurScope->addVar(DeclName->getValue(), Value);
   else
     Records.addExtraGlobal(DeclName->getValue(), Value);
 
   return false;
 }
 
 /// ParseForeach - Parse a for statement.  Return the record corresponding
 /// to it.  This returns true on error.
bool TGParser::ParseForeach(MultiClass *CurMultiClass) {
   VarInit *IterName = ParseForeachDeclaration(ListValue);
   if (!IterName)
     return TokError("expected declaration in for");
 
   if (!consume(tgtok::In))
     return TokError("Unknown tok");
 
   // Create a loop object and remember it.
-  Loops.push_back(std::make_unique<ForeachLoop>(Loc, IterName, ListValue));
-
+  auto TheLoop = std::make_unique<ForeachLoop>(Loc, IterName, ListValue);
   // A foreach loop introduces a new scope for local variables.
-  TGLocalVarScope *ForeachScope = PushLocalScope();
+  TGVarScope *ForeachScope = PushScope(TheLoop.get());
+  Loops.push_back(std::move(TheLoop));
 
   if (Lex.getCode() != tgtok::l_brace) {
     // FOREACH Declaration IN Object
     if (ParseObject(CurMultiClass))
       return true;
   } else {
     SMLoc BraceLoc = Lex.getLoc();
     // Otherwise, this is a group foreach.
     Lex.Lex();  // eat the '{'.
 
     // Parse the object list.
     if (ParseObjectList(CurMultiClass))
       return true;
 
     if (!consume(tgtok::r_brace)) {
       TokError("expected '}' at end of foreach command");
       return Error(BraceLoc, "to match this '{'");
     }
   }
 
-  PopLocalScope(ForeachScope);
+  PopScope(ForeachScope);
 
   // Resolve the loop or store it for later resolution.
   std::unique_ptr<ForeachLoop> Loop = std::move(Loops.back());
   Loops.pop_back();
 
   return addEntry(std::move(Loop));
 }
 

 }
 
 /// ParseIfBody - Parse the then-clause or else-clause of an if statement.
 ///
 ///   IfBody ::= Object
 ///   IfBody ::= '{' ObjectList '}'
 ///
 bool TGParser::ParseIfBody(MultiClass *CurMultiClass, StringRef Kind) {
-  TGLocalVarScope *BodyScope = PushLocalScope();
+  // An if-statement introduces a new scope for local variables.
+  TGVarScope *BodyScope = PushScope();
 
   if (Lex.getCode() != tgtok::l_brace) {
     // A single object.
     if (ParseObject(CurMultiClass))
       return true;
   } else {
     SMLoc BraceLoc = Lex.getLoc();
     // A braced block.
     Lex.Lex(); // eat the '{'.
 
     // Parse the object list.
     if (ParseObjectList(CurMultiClass))
       return true;
 
     if (!consume(tgtok::r_brace)) {
       TokError("expected '}' at end of '" + Kind + "' clause");
       return Error(BraceLoc, "to match this '{'");
     }
   }
 
-  PopLocalScope(BodyScope);
+  PopScope(BodyScope);
   return false;
 }
 
 /// ParseAssert - Parse an assert statement.
 ///
 ///   Assert ::= ASSERT condition , message ;
 bool TGParser::ParseAssert(MultiClass *CurMultiClass, Record *CurRec) {
   assert(Lex.getCode() == tgtok::Assert && "Unknown tok");
if (CurRec) {
     auto NewRec =
         std::make_unique<Record>(Lex.getCurStrVal(), Lex.getLoc(), Records,
                                   /*Class=*/true);
     CurRec = NewRec.get();
     Records.addClass(std::move(NewRec));
   }
   Lex.Lex(); // eat the name.
 
+  // A class definition introduces a new scope.
+  TGVarScope *ClassScope = PushScope(CurRec);
   // If there are template args, parse them.
   if (Lex.getCode() == tgtok::less)
     if (ParseTemplateArgList(CurRec))
       return true;
 
   if (ParseObjectBody(CurRec))
     return true;
 
   if (!NoWarnOnUnusedTemplateArgs)
     CurRec->checkUnusedTemplateArgs();
+
+  PopScope(ClassScope);
   return false;
 }
 
 /// ParseLetList - Parse a non-empty list of assignment expressions into a list
 /// of LetRecords.
 ///
 ///   LetList ::= LetItem (',' LetItem)*
 ///   LetItem ::= ID OptionalRangeList '=' Value
bool TGParser::ParseTopLevelLet(MultiClass *CurMultiClass) {
   SmallVector<LetRecord, 8> LetInfo;
   ParseLetList(LetInfo);
   if (LetInfo.empty()) return true;
   LetStack.push_back(std::move(LetInfo));
 
   if (!consume(tgtok::In))
     return TokError("expected 'in' at end of top-level 'let'");
 
-  TGLocalVarScope *LetScope = PushLocalScope();
-
   // If this is a scalar let, just handle it now
   if (Lex.getCode() != tgtok::l_brace) {
     // LET LetList IN Object
     if (ParseObject(CurMultiClass))
       return true;
   } else {   // Object ::= LETCommand '{' ObjectList '}'
     SMLoc BraceLoc = Lex.getLoc();
     // Otherwise, this is a group let.
     Lex.Lex();  // eat the '{'.
 
+    // A group let introduces a new scope for local variables.
+    TGVarScope *LetScope = PushScope();
+
     // Parse the object list.
     if (ParseObjectList(CurMultiClass))
       return true;
 
     if (!consume(tgtok::r_brace)) {
       TokError("expected '}' at end of top level let command");
       return Error(BraceLoc, "to match this '{'");
     }
-  }
 
-  PopLocalScope(LetScope);
+    PopScope(LetScope);
+  }
 
   // Outside this let scope, this let block is not active.
   LetStack.pop_back();
   return false;
 }
 
 /// ParseMultiClass - Parse a multiclass definition.
 ///
MultiClasses.insert(std::make_pair(Name,
                     std::make_unique<MultiClass>(Name, Lex.getLoc(),Records)));
 
   if (!Result.second)
     return TokError("multiclass '" + Name + "' already defined");
 
   CurMultiClass = Result.first->second.get();
   Lex.Lex();  // Eat the identifier.
 
+  // A multiclass body introduces a new scope for local variables.
+  TGVarScope *MulticlassScope = PushScope(CurMultiClass);
+
   // If there are template args, parse them.
   if (Lex.getCode() == tgtok::less)
     if (ParseTemplateArgList(nullptr))
       return true;
 
   bool inherits = false;
 
   // If there are submulticlasses, parse them.
if (Lex.getCode() != tgtok::l_brace) {
     if (!inherits)
       return TokError("expected '{' in multiclass definition");
     if (!consume(tgtok::semi))
       return TokError("expected ';' in multiclass definition");
   } else {
     if (Lex.Lex() == tgtok::r_brace)  // eat the '{'.
       return TokError("multiclass must contain at least one def");
 
-    // A multiclass body introduces a new scope for local variables.
-    TGLocalVarScope *MulticlassScope = PushLocalScope();
-
     while (Lex.getCode() != tgtok::r_brace) {
       switch (Lex.getCode()) {
       default:
         return TokError("expected 'assert', 'def', 'defm', 'defvar', "
                         "'foreach', 'if', or 'let' in multiclass body");
 
       case tgtok::Assert:
       case tgtok::Def:
if (Lex.getCode() != tgtok::l_brace) {
     Lex.Lex();  // eat the '}'.
 
     // If we have a semicolon, print a gentle error.
     SMLoc SemiLoc = Lex.getLoc();
     if (consume(tgtok::semi)) {
       PrintError(SemiLoc, "A multiclass body should not end with a semicolon");
       PrintNote("Semicolon ignored; remove to eliminate this error");    
     }
-
-    PopLocalScope(MulticlassScope);
   }
 
   if (!NoWarnOnUnusedTemplateArgs)
     CurMultiClass->Rec.checkUnusedTemplateArgs();
 
+  PopScope(MulticlassScope);
   CurMultiClass = nullptr;
   return false;
 }
 
 /// ParseDefm - Parse the instantiation of a multiclass.
 ///
 ///   DefMInst ::= DEFM ID ':' DefmSubClassRef ';'
 ///
while (isObjectStart(Lex.getCode())) {
     if (ParseObject(MC))
       return true;
   }
   return false;
 }
 
 bool TGParser::ParseFile() {
   Lex.Lex(); // Prime the lexer.
-  if (ParseObjectList()) return true;
+  TGVarScope *GlobalScope = PushScope();
+  if (ParseObjectList())
+    return true;
+  PopScope(GlobalScope);
 
   // If we have unread input at the end of the file, report it.
   if (Lex.getCode() == tgtok::Eof)
     return false;
 
   return TokError("Unexpected token at top level");
 }
 

llvm/test/TableGen/defvar.td

 // RUN: llvm-tblgen %s | FileCheck %s
 // RUN: not llvm-tblgen -DERROR1 %s 2>&1 | FileCheck --check-prefix=ERROR1 %s
 // RUN: not llvm-tblgen -DERROR2 %s 2>&1 | FileCheck --check-prefix=ERROR2 %s
 // RUN: not llvm-tblgen -DERROR3 %s 2>&1 | FileCheck --check-prefix=ERROR3 %s
+// RUN: not llvm-tblgen -DERROR4 %s 2>&1 | FileCheck --check-prefix=ERROR4 %s
+// RUN: not llvm-tblgen -DERROR5 %s 2>&1 | FileCheck --check-prefix=ERROR5 %s
 
 #ifdef ERROR1
 // Refer to a variable we haven't defined *yet*, expecting an error.
 // ERROR1: [[@LINE+1]]:22: error: Variable not defined: 'myvar'
 def bad { dag x = (? myvar); }
 #endif
 
 // Define a global variable.
 defvar myvar = "foo";
 
 #ifdef ERROR2
 // Demonstrate an error when a global variable is redefined.
 // ERROR2: [[@LINE+1]]:8: error: def or global variable of this name already exists
 defvar myvar = "another value";
 #endif
 
-// These variables should be overrided by template arguments.
-defvar a = 2333;
-defvar b = 2333;
-class VarScopeTest<int a, int b> {
-  defvar c = !add(a, b);
-  int value = !add(c, c);
-}
-
-// CHECK: def aaa_scope_test {
-// CHECK-NEXT: int value = 10;
-def aaa_scope_test: VarScopeTest<2, 3>;
-
 multiclass Test<int x> {
   // Refer to a global variable, while inside a local scope like a multiclass.
   def _with_global_string { string s = myvar; }
 
   // Define some variables local to this multiclass, and prove we can refer to
   // those too.
   defvar myvar = !add(x, 100);
   defvar myvar2 = "string of " # myvar;
if !eq(first, 1) then {
     defvar shadowedVariable = 3;
     def shadowInnerIf # first { int var = shadowedVariable; }
   }
 
   // Now the outer variable is back in scope again.
   def shadowOuterBelowIf # first { int var = shadowedVariable; }
 }
 
+class RedefinitionTest<int a, int b> {
+  #ifdef ERROR4
+  defvar value = !add(a, b);
+  #endif
+  // ERROR4: [[@LINE+1]]:7: error: local variable of this name already exists
+  int value = !add(a, b);
+  #ifdef ERROR5
+  // ERROR5: [[@LINE+1]]:10: error: field of this name already exists
+  defvar value = !add(a, b);
+  #endif
+}
+
+// These variables should be shadowed by class/multiclass template arguments.
+defvar a = 2333;
+defvar b = 2333;
+defvar c = 2333;
+class ShadowGlobalsTest<int a, int b, int c> {
+  // Template arguments have higher priorities than global variables.
+  int value = !add(a, b, c);
+}
+
+class ShadowClassArgumentTest<int a, int b, int c> {
+  // Local variable 'c' has higher priority than class template argument 'c'.
+  defvar c = !add(c, c);
+  int value = !add(a, b, c);
+}
+
+multiclass ShadowMulticlassArgumentTest<int a, int b, int c> {
+  // Local variable 'c' has higher priority than multiclass template argument 'c'.
+  defvar c = !add(c, c);
+  def "" {
+    int value = !add(a, b, c);
+  }
+}
+
+// CHECK: def shadowTestOfClassArgument {
+// CHECK-NEXT: int value = 11;
+// CHECK: def shadowTestOfGlobals {
+// CHECK-NEXT: int value = 8;
+// CHECK: def shadowTestOfLoopIterator0 {
+// CHECK-NEXT: int value = 10;
+// CHECK: def shadowTestOfLoopIterator1 {
+// CHECK-NEXT: int value = 11;
+// CHECK: def shadowTestOfMulticlassArgument {
+// CHECK-NEXT: int value = 11;
+def shadowTestOfClassArgument: ShadowClassArgumentTest<2, 3, 3>;
+def shadowTestOfGlobals: ShadowGlobalsTest<2, 3, 3>;
+foreach i = 0...1 in {
+  // Local variable 'i' has higher priority than loop iterator 'i'.
+  def shadowTestOfLoopIterator # i {
+    defvar i = !add(10, i);
+    int value = i;
+  }
+}
+defm shadowTestOfMulticlassArgument: ShadowMulticlassArgumentTest<2, 3, 3>;
+
 // Test that a top-level let statement also makes a variable scope (on the
 // general principle of consistency, because it defines a braced sub-block).
 
 let someVariable = "some value" in {
   defvar myvar = "override the definition from above and expect no error";
 }
 // CHECK: def topLevelLetTest {
 // CHECK-NEXT: string val = "foo";
 def topLevelLetTest { string val = myvar; }