[PATCH] D95514: [clang][cli] Teach CompilerInvocation to allocate strings on its own

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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