curdeius added a comment.
Looks nice already. Thanks for working on this.
================
Comment at: clang/include/clang/Format/Format.h:3691
+ /// COULD lead to incorrect code formatting due to incorrect decisions made
+ /// due to clang-formats lack of complete semantic information. As such
extra
+ /// care should be taken to review code changes made by the use of this
----------------
Nit.
================
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);
----------------
Isn't it a better name?
================
Comment at: clang/lib/Format/TemplateArgumentKeywordFixer.cpp:65
+ }
+
+ for (size_t I = 0, E = AnnotatedLines.size(); I != E; ++I) {
----------------
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?
================
Comment at: clang/lib/Format/TemplateArgumentKeywordFixer.cpp:69
+
+ // Find the first `template` keyword on this line
+ while (Tok && Tok->isNot(tok::kw_template))
----------------
Please finish comments with full stops, here and below.
================
Comment at: clang/lib/Format/TemplateArgumentKeywordFixer.cpp:75
+
+ // Find the corresponding `<`. Might be on the next line
+ Tok = Tok->Next;
----------------
================
Comment at: clang/lib/Format/TemplateArgumentKeywordFixer.h:12
+/// This file declares TemplateArgumentKeywordFixer, a TokenAnalyzer that
+// /enforces / consistent usage of `typename`/`class` for template arguments.
+///
----------------
Or even better, make it closer to what's in cpp file.
================
Comment at: clang/lib/Format/TemplateArgumentKeywordFixer.h:36
+} // end namespace format
+} // end namespace clang
+
----------------
Please make the comments consistent with other files.
================
Comment at: clang/unittests/Format/TemplateArgumentKeywordFixerTest.cpp:87
+ // `typename` outside of a template should stay unchanged
+ verifyFormat("typename T::nested", "typename T::nested", Style);
+ // `typename` inside a template should stay unchanged if it refers to a
nested
----------------
Second argument can be removed if you add an overload having Expected=Code, so
as to match `verifyFormat` in other files.
================
Comment at: clang/unittests/Format/TemplateArgumentKeywordFixerTest.cpp:155
+ // We also format template argument lists for `using`.
+ // There are no special semantics re. deduction guides and the normal
+ // implementation just works. But better test it before this breaks due to
----------------
Don't repeat comments, please. It's the same as below in deduction guides test.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116290/new/
https://reviews.llvm.org/D116290
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
