[PATCH] D83296: [clang-format] Add a MacroExpander.

klimek added a comment. In D83296#2299062 , @nridge wrote: > What does this change mean for users of clang-format -- better formatting of > complicated (e.g. multi-line) macro invocations? In addition to what Sam said, this also attempts to be an

[PATCH] D83296: [clang-format] Add a MacroExpander.

sammccall added a comment. In D83296#2299062 , @nridge wrote: > What does this change mean for users of clang-format -- better formatting of > complicated (e.g. multi-line) macro invocations? Nothing from this change is exposed yet, it's part of a

[PATCH] D83296: [clang-format] Add a MacroExpander.

nridge added a comment. What does this change mean for users of clang-format -- better formatting of complicated (e.g. multi-line) macro invocations? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83296/new/ https://reviews.llvm.org/D83296

[PATCH] D83296: [clang-format] Add a MacroExpander.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. klimek marked 5 inline comments as done. Closed by commit rGe336b74c995d: [clang-format] Add a MacroExpander. (authored by klimek). Changed prior to commit:

[PATCH] D83296: [clang-format] Add a MacroExpander.

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Ship it! Comment at: clang/lib/Format/FormatToken.h:177 +/// EndOfExpansion: 0 0 0 21 0 1 +struct MacroContext { + MacroContext(MacroRole Role) :

[PATCH] D83296: [clang-format] Add a MacroExpander.

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

[PATCH] D83296: [clang-format] Add a MacroExpander.

klimek added inline comments. Comment at: clang/lib/Format/FormatToken.h:177 +/// EndOfExpansion: 0 0 0 21 0 1 +struct MacroContext { + MacroContext(MacroRole Role) : Role(Role) {} sammccall wrote: > "context" is often pretty vague - "MacroSource"

[PATCH] D83296: [clang-format] Add a MacroExpander.

sammccall added a comment. Just checking this is waiting on you rather than me... If the multiple-expansion thing is blocking progress, I think we're much better off getting a limited version of this feature landed than losing momentum trying to solve it. Repository: rG LLVM Github

[PATCH] D83296: [clang-format] Add a MacroExpander.

klimek added inline comments. Comment at: clang/lib/Format/MacroExpander.cpp:186 +Tok->MacroCtx = MacroContext(MR_ExpandedArg); + pushToken(Tok); +} sammccall wrote: > klimek wrote: > > sammccall wrote: > > > you're pushing here without copying.

[PATCH] D83296: [clang-format] Add a MacroExpander.

sammccall added inline comments. Comment at: clang/lib/Format/MacroExpander.cpp:186 +Tok->MacroCtx = MacroContext(MR_ExpandedArg); + pushToken(Tok); +} klimek wrote: > sammccall wrote: > > you're pushing here without copying. This means the

[PATCH] D83296: [clang-format] Add a MacroExpander.

klimek added inline comments. Comment at: clang/lib/Format/MacroExpander.cpp:186 +Tok->MacroCtx = MacroContext(MR_ExpandedArg); + pushToken(Tok); +} sammccall wrote: > you're pushing here without copying. This means the original tokens from the

[PATCH] D83296: [clang-format] Add a MacroExpander.

sammccall added a comment. Somehow I missed the email from your replies. Mostly nits that you can take or leave, but I think potential bugs around functionlike-vs-objectlike and multiple-expansion of args. Comment at: clang/lib/Format/FormatToken.h:177 +/// EndOfExpansion:

[PATCH] D83296: [clang-format] Add a MacroExpander.

klimek added inline comments. Comment at: clang/lib/Format/MacroExpander.cpp:35 + SmallVector Params; + SmallVector Tokens; +}; sammccall wrote: > Tokens -> Expansion? (semantics rather than type) Changed to "Body". Comment at:

[PATCH] D83296: [clang-format] Add a MacroExpander.

klimek updated this revision to Diff 281611. klimek marked 27 inline comments as done. klimek added a comment. Addressed code review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83296/new/ https://reviews.llvm.org/D83296 Files:

[PATCH] D83296: [clang-format] Add a MacroExpander.

curdeius added inline comments. Comment at: clang/lib/Format/MacroExpander.cpp:46 + + // Parse the token stream and return the corresonding Defintion object. + // Returns an empty definition object with a null-Name on error. Nit: typo "corresponding

[PATCH] D83296: [clang-format] Add a MacroExpander.

sammccall added a comment. Sorry for the long delay, I've made up for it with extra comments :-\ This looks really well-thought-out and I'm rationalizing my pickiness as: - this is conceptually complicated - I expect this code to live a long time and be read and "modified around" by lots of

[PATCH] D83296: [clang-format] Add a MacroExpander.

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/MacroExpander.cpp:150 +Tok->MacroCtx.ExpandedFrom.push_back(ID); +if (First) { + Tok->MacroCtx.StartOfExpansion = true; elide braces? Comment at:

[PATCH] D83296: [clang-format] Add a MacroExpander.

klimek added a comment. In D83296#2146970 , @sammccall wrote: > In D83296#2146870 , @klimek wrote: > > > Monday-morning ping. > > > Thanks for the reminder here... however this is taking me a bit to get my > head

[PATCH] D83296: [clang-format] Add a MacroExpander.

sammccall added a comment. In D83296#2146870 , @klimek wrote: > Monday-morning ping. Thanks for the reminder here... however this is taking me a bit to get my head around, and we've got a release branch cut scheduled for a couple of days that we're

[PATCH] D83296: [clang-format] Add a MacroExpander.

klimek added a comment. Monday-morning ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83296/new/ https://reviews.llvm.org/D83296 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D83296: [clang-format] Add a MacroExpander.

klimek created this revision. klimek added a reviewer: sammccall. Herald added subscribers: cfe-commits, mgorny. Herald added a project: clang. The MacroExpander allows to expand simple (non-resursive) macro definitions from a macro identifier token and macro arguments. It annotates the tokens