<div dir="ltr">IDK much about Tablegen, but this code looks functionally equivalent but strictly better sylistically, so lgtm</div><br><div class="gmail_quote"><div dir="ltr">On Mon, Nov 20, 2017 at 6:04 AM George Rimar via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">grimar updated this revision to Diff 123572.<br>
grimar retitled this revision from "[llvm-tblgen] - Stop using std:string in RecordKeeper." to "[llvm-tblgen] - Stop using std::string in RecordKeeper.".<br>
grimar added a comment.<br>
<br>
- Use StringRef in `Tag` instead of `std::string*'. (`Tag` remembers ID of Classes/Defs from `Record` and since patch changes type of map key from `std::string` to `StringRef`, this place should be updated.)<br>
<br>
<br>
<a href="https://reviews.llvm.org/D40170" rel="noreferrer" target="_blank">https://reviews.llvm.org/D40170</a><br>
<br>
Files:<br>
  include/llvm/TableGen/Record.h<br>
  utils/TableGen/CTagsEmitter.cpp<br>
<br>
<br>
Index: utils/TableGen/CTagsEmitter.cpp<br>
===================================================================<br>
--- utils/TableGen/CTagsEmitter.cpp<br>
+++ utils/TableGen/CTagsEmitter.cpp<br>
@@ -28,18 +28,17 @@<br>
<br>
 class Tag {<br>
 private:<br>
-  const std::string *Id;<br>
+  StringRef Id;<br>
   SMLoc Loc;<br>
 public:<br>
-  Tag(const std::string &Name, const SMLoc Location)<br>
-      : Id(&Name), Loc(Location) {}<br>
-  int operator<(const Tag &B) const { return *Id < *B.Id; }<br>
+  Tag(StringRef Name, const SMLoc Location) : Id(Name), Loc(Location) {}<br>
+  int operator<(const Tag &B) const { return Id < B.Id; }<br>
   void emit(raw_ostream &OS) const {<br>
     const MemoryBuffer *CurMB =<br>
         SrcMgr.getMemoryBuffer(SrcMgr.FindBufferContainingLoc(Loc));<br>
     auto BufferName = CurMB->getBufferIdentifier();<br>
     std::pair<unsigned, unsigned> LineAndColumn = SrcMgr.getLineAndColumn(Loc);<br>
-    OS << *Id << "\t" << BufferName << "\t" << LineAndColumn.first << "\n";<br>
+    OS << Id << "\t" << BufferName << "\t" << LineAndColumn.first << "\n";<br>
   }<br>
 };<br>
<br>
Index: include/llvm/TableGen/Record.h<br>
===================================================================<br>
--- include/llvm/TableGen/Record.h<br>
+++ include/llvm/TableGen/Record.h<br>
@@ -1525,7 +1525,7 @@<br>
 };<br>
<br>
 class RecordKeeper {<br>
-  using RecordMap = std::map<std::string, std::unique_ptr<Record>>;<br>
+  using RecordMap = std::map<StringRef, std::unique_ptr<Record>>;<br>
   RecordMap Classes, Defs;<br>
<br>
 public:<br>
<br>
<br>
</blockquote></div>