[PATCH] D88299: [clang-format] Add MacroUnexpander.

owenpan added a comment. Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay. @klimek can you have a look at https://github.com/llvm/llvm-project/issues/64275? Comment at: clang/lib/Format/MacroCallReconstructor.cpp:223 + } +

[PATCH] D88299: [clang-format] Add MacroUnexpander.

owenpan added a comment. @klimek np! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88299/new/ https://reviews.llvm.org/D88299 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D88299: [clang-format] Add MacroUnexpander.

klimek added a comment. In D88299#3804910 , @owenpan wrote: > @sstwcw thanks for pointing it out. See D134329 > . Thanks Owen, really appreciate this! And sorry for not getting to it yet myself :( Repository: rG

[PATCH] D88299: [clang-format] Add MacroUnexpander.

owenpan added subscribers: MyDeveloperDay, owenpan. owenpan added a comment. @sstwcw thanks for pointing it out. See D134329 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88299/new/

[PATCH] D88299: [clang-format] Add MacroUnexpander.

sstwcw added a comment. It looks like some of the braces in the code should be removed for example those surrounding one-line for bodies. Sorry if it is not my job to point this out, but MyDeveloperDay has not said anything. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D88299: [clang-format] Add MacroUnexpander.

HazardyKnusperkeks added a comment. A bit surprising such a big change... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88299/new/ https://reviews.llvm.org/D88299 ___ cfe-commits mailing list

[PATCH] D88299: [clang-format] Add MacroUnexpander.

klimek added a comment. In D88299#3660779 , @nridge wrote: > Thanks! (I was intrigued by Sam's "solves a whole class of clang-format's > biggest problems" comment :-)) The end-result hopefully will :) Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D88299: [clang-format] Add MacroUnexpander.

nridge added a comment. Thanks! (I was intrigued by Sam's "solves a whole class of clang-format's biggest problems" comment :-)) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88299/new/ https://reviews.llvm.org/D88299

[PATCH] D88299: [clang-format] Add MacroUnexpander.

klimek added a comment. In D88299#3660772 , @nridge wrote: > Does this patch change the formatting behaviour of clang-format? > > If so, are there any test cases that show before/after formatting? The > MacroCallReconstructorTest unit test seems like

[PATCH] D88299: [clang-format] Add MacroUnexpander.

nridge added a comment. Does this patch change the formatting behaviour of clang-format? If so, are there any test cases that show before/after formatting? The MacroCallReconstructorTest unit test seems like it's testing an internal interface. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D88299: [clang-format] Add MacroUnexpander.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd6d0dc1f4537: [clang-format] Add MacroUnexpander. (authored by klimek). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D88299: [clang-format] Add MacroUnexpander.

klimek updated this revision to Diff 443729. klimek added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88299/new/ https://reviews.llvm.org/D88299 Files: clang/lib/Format/CMakeLists.txt

[PATCH] D88299: [clang-format] Add MacroUnexpander.

klimek marked 7 inline comments as done. klimek added inline comments. Comment at: clang/lib/Format/Macros.h:201 + /// Generally, this line tries to have the same structure as the expanded, + /// formatted unwrapped lines handed in via \c addLine(), with the exception + ///

[PATCH] D88299: [clang-format] Add MacroUnexpander.

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Herald added a project: All. OK, I think we've reached the point where: - the impact is very clear, this solves a whole class of clang-format's biggest problems - the idea clearly works

[PATCH] D88299: [clang-format] Add MacroUnexpander.

klimek added a comment. Noticed I should have waiting with the renaming of the file until the review is done :( Sorry for the extra confusion. Comment at: clang/lib/Format/MacroUnexpander.cpp:77 + // stream, we need to continue the unexpansion until we find the right token +

[PATCH] D88299: [clang-format] Add MacroUnexpander.

klimek updated this revision to Diff 390057. klimek marked 49 inline comments as done. klimek added a comment. Work in review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88299/new/ https://reviews.llvm.org/D88299 Files:

[PATCH] D88299: [clang-format] Add MacroUnexpander.

sammccall added a comment. Thanks a lot for all the time explaining, I get this at least at a high level now. I have trouble following the ins and outs of the algorithm, but I think I understand most of it and tests cover the rest. Regarding tests: I have trouble reasoning about whether all

[PATCH] D88299: [clang-format] Add MacroUnexpander.

klimek added inline comments. Comment at: clang/lib/Format/FormatToken.h:449 + /// When macro expansion introduces parents, those are marked as + /// \c MacroParent, so formatting knows their children need to be formatted. sammccall wrote: > I can't really

[PATCH] D88299: [clang-format] Add MacroUnexpander.

klimek updated this revision to Diff 301253. klimek marked 3 inline comments as done. klimek added a comment. Adapted based on review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88299/new/ https://reviews.llvm.org/D88299 Files:

[PATCH] D88299: [clang-format] Add MacroUnexpander.

sammccall added a comment. This is magnificently difficult :-) I've sunk a fair few hours into understanding it now, and need to send some comments based on the interface+tests. Some of these will be misguided or answered by the implementation, but I'm afraid I can't fit it all into my head (on

[PATCH] D88299: [clang-format] Add MacroUnexpander.

klimek created this revision. klimek added a reviewer: sammccall. Herald added subscribers: cfe-commits, mgorny. Herald added a project: clang. klimek requested review of this revision. MacroUnexpander applies the structural formatting of expanded lines into UnwrappedLines to the corresponding