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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
