sstwcw marked 2 inline comments as done.
sstwcw added a comment.

In D121907#3390226 <https://reviews.llvm.org/D121907#3390226>, @MyDeveloperDay 
wrote:

> So out of interest, what is the reason? my assumption is that you wanted to 
> add more for Verilog and you felt adding the extra bools was the wrong design 
> and its better an an enum

You are right.

>   bool InCpp11AttributeSpecifier = false;
>   bool InCSharpAttributeSpecifier = false;
>
> Does the fact that some aren't exclusive make you think its ok to split it 
> into enums and bools is ok?  (no real opinion just wondered what you and 
> others think?)

Does it make me think it's ok? Yes. Good? No. I am lazy and I chose this way 
which doesn't require examining those two options.



================
Comment at: clang/lib/Format/TokenAnnotator.cpp:116
     // template parameter, not an argument.
-    Contexts.back().InTemplateArgument =
-        Left->Previous && Left->Previous->isNot(tok::kw_template);
+    if (Left->Previous && Left->Previous->isNot(tok::kw_template))
+      Contexts.back().ContextType = Context::TemplateArgument;
----------------
owenpan wrote:
> If this was bug, it should be in a separate patch with test cases added.
Sorry that the previous patch did not include more context.  Pun intended.  Now 
you can scroll up and see that the context was just initialized, so 
`InTemplateArgument` starts being false, so it didn't matter whether the 
original code was `InTemplateArgument = ...;` or `if (...) InTemplateArgument = 
true;`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121907/new/

https://reviews.llvm.org/D121907

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to