[PATCH] D119900: [PDLL] Add support for tablegen includes and importing ODS information

[Jacques Pienaar via Phabricator via llvm-commits]

jpienaar added a comment.

Just half a scan (sorry have to context switch again but asking some in interim)



================
Comment at: llvm/include/llvm/Support/SourceMgr.h:103
 
+  /// Return the include directories of this source manager.
+  ArrayRef<std::string> getIncludeDirs() const { return IncludeDirectories; }
----------------
Could we split these out?


================
Comment at: mlir/include/mlir/Tools/PDLL/ODS/Constraint.h:42
+  /// The name of the constraint.
+  std::string name;
+  /// A summary of the constraint.
----------------
Are these std::string due to life time concerns?


================
Comment at: mlir/include/mlir/Tools/PDLL/ODS/Context.h:65
+  /// Print the contents of this context to the provided stream.
+  void print(raw_ostream &os) const;
+
----------------
Contrary to MLIR IR side is there any expectation that this is useful beyond debugging?


================
Comment at: mlir/include/mlir/Tools/PDLL/ODS/Operation.h:115
+
+/// This class represents an opaque ODS representation of a specific operation.
+/// This includes all of the information necessary for use by the PDL frontend
----------------
What does opaque here signify?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119900/new/

https://reviews.llvm.org/D119900