dexonsmith added a comment.

Can you include the use in the patch? I’m not sure having the AllocateString 
API totally makes sense, vs moving in a string pool that was independently used 
during generation, but maybe in context?

I also wonder what command-line option is forcing this. Ideally we could 
arrange the generated -cc1 command line to only use generated strings for 
numbers / enums, which will not need long-term storage as stringrefs. String 
arguments could reuse the original char*. It seems better to drop frivolous `=` 
than carrying this around.

Consider that some callers may parse the full command-line and then drop 
everything but one options class. That pattern wouldn’t work anymore since the 
StringRefs won’t be valid.



================
Comment at: clang/include/clang/Frontend/CompilerInvocation.h:144
+  /// Container holding strings allocated during round-trip.
+  mutable std::forward_list<std::string> AllocatedStrings;
+
----------------
I suggest using llvm::StringPool instead, which also uniques the strings and is 
used elsewhere for this kind of thing. Probably in an Optional to make it more 
clear it’s not always used.

Maybe the name RoundTrippedStrings? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95514

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to