[PATCH] D53840: Preprocessing support in tablegen

[Nicolai Hähnle via Phabricator via llvm-commits]

nhaehnle added a comment.

I have to say I'm feeling a bit ambivalent about this. I'd say it would be nicer to have a mechanism that integrates with the rest of the TableGen language, but that's admittedly non-trivial. So I guess this approach is okay.

The handling of end-of-files is a bit wonky. Have you considered just returning EOF from getNextChar at end of file, even if there's a parent file, and changing the EOF case in LexToken to just loop (or tail-recurse, I suppose) if it was the EOF of an included file?



================
Comment at: lib/TableGen/TGLexer.cpp:605-608
+void TGLexer::prepNewPreprocessorControl() {
+  PreprocessorControl =
+      make_unique<std::vector<PreprocessorControlDesc> >();
+}
----------------
Please just inline this function where it's used. Having to jump here increases the cognitive overhead of reading the code.


================
Comment at: lib/TableGen/TGLexer.h:169
+    //      ^ - SrcPos
+    const char *SrcPos;
+  };
----------------
Should be an SMLoc.


================
Comment at: lib/TableGen/TGLexer.h:185
+  // must be located in the same file.
+  std::unique_ptr<std::vector<PreprocessorControlDesc> > PreprocessorControl;
+
----------------
Coding style: Redundant space between closing angle brackets.

Also, just remove PreprocessorControl and instead refer to PrepIncludeStack.back() for the current one.


================
Comment at: test/TableGen/prep-diag1.td:9
+
+// RUN: not llvm-tblgen -DDIAG4 -I %p %s 2>&1 | FileCheck --check-prefixes=DIAG4 %s
+
----------------
Please keep all RUN lines at the top of the file.


Repository:
  rL LLVM

https://reviews.llvm.org/D53840