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

Reply via email to