[PATCH] D94472: [WIP][clang][cli] Command line round-trip for HeaderSearch options

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

Thanks for putting your time into the comment, @dexonsmith.

I agree that adding a `-round-trip-args` flag to enable/disable round-tripping is more sensible than hard-coding it via something like `#ifndef NDEBUG`.
Providing `-round-trip-args-debug-mode` to help with debugging would be a great addition too. Thanks for pointing that out.

I would be interested to hear why exactly are you concerned by the current approach being too low level.
>From my point of view, having to opt-in into round-tripping a part of `CompilerInvocation` is a feature and I think it should work relatively well with the current roll-out plan:

1. Write documentation on porting existing options to the new marshalling system.
2. Send a message to the mailing list pointing out that we'll be enabling round-tripping in assert builds by default and what that means. Downstream projects can prepare to port their custom options or disable the round-tripping in assert builds.
3. Some time later, commit this patch. Downstream projects already have enough information how to deal with the change. Because we've enabled round-tripping only for `HeaderSearchOptions`, adopting it isn't that time-consuming if a downstream project decides to do so.
4. (Any patches that add a command line option affecting `HeaderSearchOptions` will from now on be forced to include the generating/marshalling code.)
5. Slowly commit following patches, each enabling round-tripping for another chunk of `CompilerInvocation`.
6. After round-tripping has been enabled for all of `CompilerInvocation`, simplify the code (remove `ParseHS`, `GenerateHS`, `ResetHS`, etc.) and round-trip everything (when told to do so with `-round-trip-args`).

I'd be interested in hearing your thoughts on that ^.

To help me understand the reasoning behind your proposal, can you answer these questions?

1. What is the benefit or "relaxed" round-tripping compared to the selective strict round-tripping in this patch?
2. What does `GeneratedArgs1 != GeneratedArgs2` prove? If we forget to generate some option, it will be in neither `GeneratedArgs1` nor `GeneratedArgs2`. We'll catch that only in "strict" mode anyways. I guess this can help us discover other kinds of lossy generation, but I think the guarantees of "strict" mode are nicer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94472