HazardyKnusperkeks added inline comments.
================ Comment at: clang/lib/Format/FormatTokenLexer.cpp:1031-1035 + if (it->second.Finalized) { + FormatTok->setFinalizedType(it->second.Type); + } else { + FormatTok->setType(it->second.Type); + } ---------------- owenpan wrote: > It seems we can simply do this and leave the rest of `FormatTokenLexer` alone. +1 ================ Comment at: clang/lib/Format/FormatTokenLexer.h:117-120 + struct MacroTokenInfo { + TokenType Type; + bool Finalized; + }; ---------------- ksyx wrote: > curdeius wrote: > > ksyx wrote: > > > Would making constructor of `struct MacroTokenInfo` having default > > > parameter or overloading it help avoiding the change of adding `, > > > /*Finalized=*/false` to the existing initializer lists? > > I've thought about it, but it would mean that we have a non-explicit 1-arg > > ctor. I'm not a big fan of these as they trigger implicit conversions. > > I can do though: > > ``` > > struct MacroTokenInfo { > > TokenType Type; > > bool Finalized{false}; > > }; > > ``` > > but we'd still need adding braces in: > > ``` > > Macros.insert({Identifier, {TT_ForEachMacro}}); > > ``` > Yes they are both good point to consider and my start point is just that the > `finalized` property is less frequently be `true`. I wouldn't add a CTor. And I also wouldn't add a default initializer. But the latter is better. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123676/new/ https://reviews.llvm.org/D123676 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits