[PATCH] D88600: [TableGen] New backend to print fully detailed records

[Chris Lattner via Phabricator via llvm-commits]

lattner added a comment.

Looks nice to me, please tidy up the SourceMgr stuff and I'll take another look.



================
Comment at: llvm/lib/Support/SourceMgr.cpp:207
+  } else {
+    auto I = FileSpec.find_last_of("/\\");
+    I = (I == FileSpec.size()) ? 0 : (I + 1);
----------------
Is it standard to drop the path prefix?  Is this what happens when sourcemgr emits errors and warning diagnostics?


================
Comment at: llvm/lib/Support/SourceMgr.cpp:210
+    return FileSpec.substr(I).str() + ":" +
+           std::to_string(FindLineNumber(Loc, BufferID));
+  }
----------------
Shouldn't this also include column numbers?

Making this utility accessible to other clients of SourceMgr is nice, but if so, please change SourceMgr itself to use it for its internal formatting of the location information.  This ensures that things stay consistent.


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

https://reviews.llvm.org/D88600