Tablegen: Remove the error for duplicate include files.

[rriddle]

Summary

This error was originally added a while(7 years) ago when including multiple files was basically always an error. Tablegen now has preprocessor support, which allows for building nice c/c++ style include guards. With the current error being reported, we unfortunately need to double guard when including files:

• In user of MyFile.td

#ifndef MYFILE_TD
include MyFile.td
#endif

• In MyFile.td

#ifndef MYFILE_TD
#define MYFILE_TD
...
#endif

[mehdi_amini]

Adding Sean since they added this originally

[mehdi_amini]

That is fine with me but I don't have confidence to LGTM this, it'd be nice to get more buy-in!

[arsenm]

Needs testcase. TableGen traditionally is lacking many standalone tests and I find it’s an obstacle to development

[rriddle]

Added test case.

[rriddle]

In D70410#1750889, @arsenm wrote:

Needs testcase. TableGen traditionally is lacking many standalone tests and I find it’s an obstacle to development

Done, thanks for the reminder Matt!

[arsenm]

llvm/test/TableGen/duplicate-include.inc

2

Does this need to be moved into an Inputs directory to avoid lit complaining about the lack of run line?

[rriddle]

llvm/test/TableGen/duplicate-include.inc

2

No, the file suffix is '.inc' which isn't marked in the lit.config as being a test suffix. This also matches the convention of some of the other tests in this directory, examples:

https://github.com/llvm/llvm-project/blob/master/llvm/test/TableGen/prep-diag11-include.inc

https://github.com/llvm/llvm-project/blob/master/llvm/test/TableGen/ConstraintChecking.inc

[rriddle]

Tidy up test.

[silvas]

Thanks. This makes sense now that we have include guards.

[silvas]

llvm/lib/TableGen/Main.cpp

75–77

Make sure this is iterated in a deterministic order.

[rriddle]

Ensure deterministic iteration.

[rriddle]

If there are no other comments, could someone submit this for me?

llvm/lib/TableGen/Main.cpp

if (OutputFilename == "-")
     return reportError(argv0, "the option -d must be used together with -o\n");
 
   std::error_code EC;
   ToolOutputFile DepOut(DependFilename, EC, sys::fs::OF_None);
   if (EC)
     return reportError(argv0, "error opening " + DependFilename + ":" +
                                   EC.message() + "\n");
   DepOut.os() << OutputFilename << ":";
   for (const auto &Dep : Parser.getDependencies()) {
-    DepOut.os() << ' ' << Dep.first;
+    DepOut.os() << ' ' << Dep;
   }
   DepOut.os() << "\n";
   DepOut.keep();
   return 0;
 }
 
 int llvm::TableGenMain(char *argv0, TableGenMainFn *MainFn) {
   RecordKeeper Records;
 

llvm/lib/TableGen/TGLexer.h

 #define LLVM_LIB_TABLEGEN_TGLEXER_H
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/DataTypes.h"
 #include "llvm/Support/SMLoc.h"
 #include <cassert>
-#include <map>
 #include <memory>
+#include <set>
 #include <string>
 
 namespace llvm {
 class SourceMgr;
 class SMLoc;
 class Twine;
 
 namespace tgtok {
class TGLexer {
   std::string CurStrVal;  // This is valid for ID, STRVAL, VARNAME, CODEFRAGMENT
   int64_t CurIntVal = 0;  // This is valid for INTVAL.
 
   /// CurBuffer - This is the current buffer index we're lexing from as managed
   /// by the SourceMgr object.
   unsigned CurBuffer = 0;
 
 public:
-  typedef std::map<std::string, SMLoc> DependenciesMapTy;
+  typedef std::set<std::string> DependenciesSetTy;
+
 private:
   /// Dependencies - This is the list of all included files.
-  DependenciesMapTy Dependencies;
+  DependenciesSetTy Dependencies;
 
 public:
   TGLexer(SourceMgr &SrcMgr, ArrayRef<std::string> Macros);
 
   tgtok::TokKind Lex() {
     return CurCode = LexToken(CurPtr == CurBuf.begin());
   }
 
-  const DependenciesMapTy &getDependencies() const {
+  const DependenciesSetTy &getDependencies() const {
     return Dependencies;
   }
 
   tgtok::TokKind getCode() const { return CurCode; }
 
   const std::string &getCurStrVal() const {
     assert((CurCode == tgtok::Id || CurCode == tgtok::StrVal ||
             CurCode == tgtok::VarName || CurCode == tgtok::CodeFragment) &&

llvm/lib/TableGen/TGLexer.cpp

bool TGLexer::LexInclude() {
 
   CurBuffer = SrcMgr.AddIncludeFile(Filename, SMLoc::getFromPointer(CurPtr),
                                     IncludedFile);
   if (!CurBuffer) {
     PrintError(getLoc(), "Could not find include file '" + Filename + "'");
     return true;
   }
 
-  DependenciesMapTy::const_iterator Found = Dependencies.find(IncludedFile);
-  if (Found != Dependencies.end()) {
-    PrintError(getLoc(),
-               "File '" + IncludedFile + "' has already been included.");
-    SrcMgr.PrintMessage(Found->second, SourceMgr::DK_Note,
-                        "previously included here");
-    return true;
-  }
-  Dependencies.insert(std::make_pair(IncludedFile, getLoc()));
+  Dependencies.insert(IncludedFile);
   // Save the line number and lex buffer of the includer.
   CurBuf = SrcMgr.getMemoryBuffer(CurBuffer)->getBuffer();
   CurPtr = CurBuf.begin();
 
   PrepIncludeStack.push_back(
       std::make_unique<std::vector<PreprocessorControlDesc>>());
   return false;
 }

llvm/lib/TableGen/TGParser.h

public:
 
   bool Error(SMLoc L, const Twine &Msg) const {
     PrintError(L, Msg);
     return true;
   }
   bool TokError(const Twine &Msg) const {
     return Error(Lex.getLoc(), Msg);
   }
-  const TGLexer::DependenciesMapTy &getDependencies() const {
+  const TGLexer::DependenciesSetTy &getDependencies() const {
     return Lex.getDependencies();
   }
 
 private:  // Semantic analysis methods.
   bool AddValue(Record *TheRec, SMLoc Loc, const RecordVal &RV);
   bool SetValue(Record *TheRec, SMLoc Loc, Init *ValName,
                 ArrayRef<unsigned> BitList, Init *V,
                 bool AllowSelfAssignment = false);

llvm/test/TableGen/duplicate-include.inc

+#ifndef DUPLICATE_INCLUDE
+#define DUPLICATE_INCLUDE
+
+// This is used by the duplicate-include.td test
+def InInclude;
+
+#endif

llvm/test/TableGen/duplicate-include.td

+// RUN: llvm-tblgen -I %p %s | FileCheck %s
+
+// CHECK: def InInclude
+
+include "duplicate-include.inc"
+include "duplicate-include.inc"
+