jansvoboda11 added a comment. In D95514#2525255 <https://reviews.llvm.org/D95514#2525255>, @dexonsmith wrote:
> Can you include the use in the patch? > I also wonder what command-line option is forcing this. The use-case that requires it is here: https://github.com/llvm/llvm-project/blob/f3449ed6073cac58efd9b62d0eb285affa650238/clang/lib/Frontend/CompilerInvocation.cpp#L734. Clang accepts analyzer arguments in form of `-analyzer-config key=value`. Reference to the `key` sub-string is stored as an index in a `StringMap` (`AnalyzerOptions::Config`) and values are held by `std::string`. More entries get stored into the map in `parseAnalyzerConfigs`. > String arguments could reuse the original char*. Interesting idea. We could avoid allocation (and the need for owning strings) by grabbing the map key substring, scanning past its end until we hit `\0` and putting the whole thing into generated arguments. Keys in the `StringMap` that are not followed by `=value` most likely only come from `parseAnalyzerConfigs`, but we don't need to generate those. This could work, but I'm worried it's somewhat brittle. > It seems better to drop frivolous `=` than carrying this around. Cool, this could save us allocations in some cases. Not sure dropping `=` between key-value pairs would result in good user experience though. > 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? What exactly do you mean here? > 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. Ah, that's right. So if we don't go with the allocation-less approach, we'd need to take `StringAllocator` from the client that also ensures proper lifetimes. 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
