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

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.


================
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);
----------------
curdeius wrote:
> 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.
IIUC, it feels like the boolean should be named `UseClassKWInTemplateTemplates`, and it shouldn't have anything to do with `Keep`ing the human's choice. If the human feeds you some C++17 code using `template<template<class> typename T>` and asks you to format it as C++14 with `TAS_Typename`, I think it would be quite appropriate to output `template<template<typename> class T>`. (Change `class` to `typename` because the human asked for `TAS_Typename`; change `typename` to `class` because C++14 implies `UseClassKWInTemplateTemplates`.)
Anyway, this should have a unit test too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116290