[PATCH] D116290: [clang-format] Add enforcement of consistent `class`/typename` keyword for template arguments

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

Thanks for addressing the comments quickly. I'll have one more thorough look in the coming days.



================
Comment at: clang/lib/Format/TemplateArgumentKeywordFixer.cpp:55
+    // For `auto` language version, be conservative and assume we are < C++17
+    KeepTemplateTemplateKW = (Style.Standard == FormatStyle::LS_Auto) ||
+                             (Style.Standard < FormatStyle::LS_Cpp17);
----------------
avogelsgesang wrote:
> curdeius wrote:
> > Isn't it a better name?
> This flag is actually about the usage of the `class` keyword instead of the `typename` keyword for template-template arguments.
> `true` means: "Keep using the `class` instead of the `typename` keyword for template-template arguments."
> 
> I think the name `KeepTemplateTypenameKW` is wrong. "[...]TypenameKW = true" would mean "use `typename` instead of `class`" to me, and that's exactly the opposite way around.
> 
> As such, I think `KeepTemplateTemplateKW` is in fact the better name. If we want to make it even more explicit, we could also use `KeepTemplateTemplateClassKW`. What do you think?
I did understand it correctly that it is about class keyword in template template parameters, but my brain somehow melted down the road:). You can keep the name as is.


================
Comment at: clang/lib/Format/TemplateArgumentKeywordFixer.cpp:65
+  }
+
+  for (size_t I = 0, E = AnnotatedLines.size(); I != E; ++I) {
----------------
avogelsgesang wrote:
> curdeius wrote:
> > Could we check `KeepTemplateTypenameKW` and early return here?
> > The optimizer might do some job, but I doubt it will get rid of the unnecessary finding of template keyword etc.
> > We could maybe avoid this variable altogether and return inside the switch, no?
> I think you misunderstood the semantics of `KeepTemplateTypenameKW`? Or did I misunderstand your comment?
> 
> For both `KeepTemplateTemplateKW = true` and `KeepTemplateTemplateKW = false`, the loop below still reformats `class` to `typename`.
> 
> E.g., for the input
> ```
> template<template<class> class X>
> ```
> and `KeepTemplateTemplateKW = true`, we produce
> ```
> template<template<typename> class X>
> ```
> For the same input with `KeepTemplateTemplateKW = false`, we produce
> ```
> template<template<typename> typename X>
> ```
As above, forget my previous comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116290