[PATCH] D78845: [COFF] Add a fastpath for /INCLUDE: in .drective sections

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

In D78845#2008164 <https://reviews.llvm.org/D78845#2008164>, @rnk wrote:

> As a next step, I noticed that `cl::TokenizeWindowsCommandLine` copies all strings. ;_; When I initially wrote it, I had intended that it would only copy an argument in the case that it had to deal with quotations, but it looks like some developer has "helpfully" fixed a use after free by making it always copy. :(


Nevermind, ignore that part of the comment. I just imagined that I had implemented this optimization. :) I wasn't able to implement this optimization because the Option library really wants to work with null-terminated strings, and we have to copy to get that null termination.

To avoid copying all strings during tokenization, we would need to start by relaxing that null terminated string requirement in the Option library. The string copies should already use BumpPtrAllocator, since they use StringSaver. This optimization should meaningfully reduce memory usage for PGO instrumented builds, because these strings currently live forever, and that is unnecessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78845