avogelsgesang updated this revision to Diff 396237. avogelsgesang added a comment.
Fix code formatting Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116290/new/ https://reviews.llvm.org/D116290 Files: clang/docs/ClangFormatStyleOptions.rst clang/docs/ReleaseNotes.rst clang/include/clang/Format/Format.h clang/lib/Format/CMakeLists.txt clang/lib/Format/Format.cpp clang/lib/Format/TemplateArgumentKeywordFixer.cpp clang/lib/Format/TemplateArgumentKeywordFixer.h clang/unittests/Format/CMakeLists.txt clang/unittests/Format/TemplateArgumentKeywordFixerTest.cpp
Index: clang/unittests/Format/TemplateArgumentKeywordFixerTest.cpp =================================================================== --- /dev/null +++ clang/unittests/Format/TemplateArgumentKeywordFixerTest.cpp @@ -0,0 +1,170 @@ +//===- unittest/Format/TemplateArgumentKeywordFixerTest.cpp ---------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "clang/Format/Format.h" + +#include "FormatTestUtils.h" +#include "TestLexer.h" +#include "gtest/gtest.h" + +#include "../../lib/Format/TemplateArgumentKeywordFixer.h" + +using testing::ScopedTrace; + +namespace clang { +namespace format { +namespace { + +class TemplateArgumentKeywordFixerTest : public ::testing::Test { +protected: + std::string format(llvm::StringRef Code, + const std::vector<tooling::Range> &Ranges, + const FormatStyle &Style) { + LLVM_DEBUG(llvm::errs() << "---\n"); + LLVM_DEBUG(llvm::errs() << Code << "\n\n"); + FormattingAttemptStatus Status; + tooling::Replacements Replaces = + reformat(Style, Code, Ranges, "<stdin>", &Status); + EXPECT_TRUE(Status.FormatComplete); + auto Result = applyAllReplacements(Code, Replaces); + EXPECT_TRUE(static_cast<bool>(Result)); + LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n"); + return *Result; + } + + std::string format(llvm::StringRef Code, const FormatStyle &Style) { + return format(Code, + /*Ranges=*/{1, tooling::Range(0, Code.size())}, Style); + } + + void _verifyFormat(const char *File, int Line, llvm::StringRef Expected, + llvm::StringRef Code, const FormatStyle &Style) { + ScopedTrace t(File, Line, ::testing::Message() << Code.str()); + EXPECT_EQ(Expected.str(), format(Expected, Style)) + << "Expected code is not stable"; + EXPECT_EQ(Expected.str(), format(Code, Style)); + EXPECT_EQ(Expected.str(), format(test::messUp(Code), Style)); + } +}; + +#define verifyFormat(...) _verifyFormat(__FILE__, __LINE__, __VA_ARGS__) + +TEST_F(TemplateArgumentKeywordFixerTest, DisabledByDefault) { + FormatStyle Style = getLLVMStyle(); + EXPECT_EQ(Style.TemplateArgumentKeyword, FormatStyle::TAS_Leave); + verifyFormat("template <class X> class A", "template <class X> class A", + Style); + verifyFormat("template <typename X> class A", "template <typename X> class A", + Style); + verifyFormat("template <typename X, class Y> class A", + "template <typename X, class Y> class A", Style); +} + +TEST_F(TemplateArgumentKeywordFixerTest, FormatsAsClass) { + FormatStyle Style = getLLVMStyle(); + Style.TemplateArgumentKeyword = FormatStyle::TAS_Class; + verifyFormat("template <class X> class A", "template <class X> class A", + Style); + verifyFormat("template <class X> class A", "template <typename X> class A", + Style); + verifyFormat("template <class X, class Y> class A;", + "template <typename X, typename Y> class A;", Style); + verifyFormat("template <class X, class Y> class A;", + "template <class X, typename Y> class A;", Style); + verifyFormat("template <template <class X> class Y> class A;", + "template <template <typename X> typename Y> class A;", Style); +} + +TEST_F(TemplateArgumentKeywordFixerTest, ClassLeavesOtherTypenames) { + FormatStyle Style = getLLVMStyle(); + Style.TemplateArgumentKeyword = FormatStyle::TAS_Class; + // `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 + // type + verifyFormat( + "template <class X, class Y = typename X::Nested> class A;", + "template <typename X, typename Y = typename X::Nested> class A;", Style); + // A legitimate `typename` might also occur in a nested list, directly after + // the `,` token It's not sufficient to just check for a comma in front of + // `typename`. + verifyFormat("template <class X, class Y = X<X, typename X::Nested>, class Z " + "= void> class A;", + "template <typename X, typename Y = X<X, typename X::Nested>, " + "typename Z = void> class A;", + Style); + // `typename` might also occur in a function call, not only in a type + verifyFormat("template <class X, int Y = func<typename Y::type>(), class Z = " + "void> class A;", + "template <typename X, int Y = func<typename Y::type>(), " + "typename Z = void> class A;", + Style); + // `typename` might also occur in function call arguments, outside the + // template list + verifyFormat("template <class X, int Y = func(typename X::type{42}), class " + "Z> class A;", + "template <typename X, int Y = func(typename X::type{42}), " + "typename Z> class A;", + Style); +} + +TEST_F(TemplateArgumentKeywordFixerTest, FormatsAsTypename) { + FormatStyle Style = getLLVMStyle(); + Style.TemplateArgumentKeyword = FormatStyle::TAS_Typename; + verifyFormat("template <typename X> class A", "template <class X> class A", + Style); + verifyFormat("template <typename X> class A", "template <typename X> class A", + Style); + verifyFormat("template <typename X, typename Y> class A;", + "template <class X, class Y> class A;", Style); + verifyFormat("template <typename X, typename Y> class A;", + "template <typename X, class Y> class A;", Style); + verifyFormat("template <template <typename X> typename Y> class A;", + "template <template <class X> class Y> class A;", Style); +} + +TEST_F(TemplateArgumentKeywordFixerTest, FormatsAsTypenameCpp14) { + FormatStyle Style = getLLVMStyle(); + Style.Standard = FormatStyle::LS_Cpp14; + Style.TemplateArgumentKeyword = FormatStyle::TAS_Typename; + // In C++14, template-template arguments must not use `typename`! + verifyFormat("template <template <typename X> class Y> class A;", + "template <template <class X> class Y> class A;", Style); + // When `class` is preferred, we replace both `typename`s, also in C++14 + Style.TemplateArgumentKeyword = FormatStyle::TAS_Class; + verifyFormat("template <template <class X> class Y> class A;", + "template <template <typename X> typename Y> class A;", Style); +} + +TEST_F(TemplateArgumentKeywordFixerTest, FormatsUsing) { + FormatStyle Style = getLLVMStyle(); + Style.Standard = FormatStyle::LS_Cpp14; + Style.TemplateArgumentKeyword = FormatStyle::TAS_Typename; + // 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 + // some future change to `clang-format` + verifyFormat("template <typename T> using A = B<T[]>;", + "template <class T> using A = B<T[]>; ", Style); +} + +TEST_F(TemplateArgumentKeywordFixerTest, FormatsDeductionGuides) { + FormatStyle Style = getLLVMStyle(); + Style.Standard = FormatStyle::LS_Cpp14; + Style.TemplateArgumentKeyword = FormatStyle::TAS_Typename; + // We also format template argument lists for deduction guides. + // There are no special semantics re. deduction guides and the normal + // implementation just works. But better test it before this breaks due to + // some future change to `clang-format` + verifyFormat("template <typename T> A(const T &, const T &) -> A<T &>;", + "template <class T> A(const T &, const T &) -> A<T &>;", Style); +} + +} // namespace +} // namespace format +} // namespace clang Index: clang/unittests/Format/CMakeLists.txt =================================================================== --- clang/unittests/Format/CMakeLists.txt +++ clang/unittests/Format/CMakeLists.txt @@ -22,6 +22,7 @@ SortImportsTestJS.cpp SortImportsTestJava.cpp SortIncludesTest.cpp + TemplateArgumentKeywordFixerTest.cpp UsingDeclarationsSorterTest.cpp TokenAnnotatorTest.cpp ) Index: clang/lib/Format/TemplateArgumentKeywordFixer.h =================================================================== --- /dev/null +++ clang/lib/Format/TemplateArgumentKeywordFixer.h @@ -0,0 +1,38 @@ +//===--- TemplateArgumentKeywordFixer.h ------------------------------*- C++ +//-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +/// +/// \file +/// This file declares TemplateArgumentKeywordFixer, a TokenAnalyzer that +// /enforces / consistent usage of `typename`/`class` for template arguments. +/// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_LIB_FORMAT_TEMPLATEARGUMENTKEYWORDFIXER_H +#define LLVM_CLANG_LIB_FORMAT_TEMPLATEARGUMENTKEYWORDFIXER_H + +#include "TokenAnalyzer.h" + +namespace clang { +namespace format { + +class TemplateArgumentKeywordFixer : public TokenAnalyzer { +public: + TemplateArgumentKeywordFixer(const Environment &Env, + const FormatStyle &Style); + + std::pair<tooling::Replacements, unsigned> + analyze(TokenAnnotator &Annotator, + SmallVectorImpl<AnnotatedLine *> &AnnotatedLines, + FormatTokenLexer &Tokens) override; +}; + +} // end namespace format +} // end namespace clang + +#endif Index: clang/lib/Format/TemplateArgumentKeywordFixer.cpp =================================================================== --- /dev/null +++ clang/lib/Format/TemplateArgumentKeywordFixer.cpp @@ -0,0 +1,119 @@ +//===--- TemplateArgumentKeywordFixer.cpp -----------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +/// +/// \file +/// This file implements TemplateArgumentKeywordFixer, a TokenAnalyzer that +/// enforces consistent usage of `typename` or `class` in template arguments. +/// +//===----------------------------------------------------------------------===// + +#include "TemplateArgumentKeywordFixer.h" + +namespace clang { +namespace format { + +TemplateArgumentKeywordFixer::TemplateArgumentKeywordFixer( + const Environment &Env, const FormatStyle &Style) + : TokenAnalyzer(Env, Style) {} + +static void replaceToken(const SourceManager &SourceMgr, + tooling::Replacements &Fixes, const FormatToken *Tok, + StringRef NewText) { + auto Range = CharSourceRange::getCharRange(Tok->getStartOfNonWhitespace(), + Tok->Tok.getEndLoc()); + auto Replacement = tooling::Replacement(SourceMgr, Range, NewText); + auto Err = Fixes.add(Replacement); + if (Err) + llvm::errs() << "Error while replacing template argument keyword: " + << llvm::toString(std::move(Err)) << "\n"; +} + +std::pair<tooling::Replacements, unsigned> +TemplateArgumentKeywordFixer::analyze( + TokenAnnotator &Annotator, SmallVectorImpl<AnnotatedLine *> &AnnotatedLines, + FormatTokenLexer &Tokens) { + const SourceManager &SourceMgr = Env.getSourceManager(); + AffectedRangeMgr.computeAffectedLines(AnnotatedLines); + tooling::Replacements Fixes; + + StringRef ReplacementStr; + tok::TokenKind BadToken; + bool KeepTemplateTemplateKW = false; + switch (Style.TemplateArgumentKeyword) { + case FormatStyle::TAS_Leave: + return {Fixes, 0}; + case FormatStyle::TAS_Typename: + assert(Style.Standard != FormatStyle::LS_Auto); + KeepTemplateTemplateKW = Style.Standard < FormatStyle::LS_Cpp17; + BadToken = tok::kw_class; + ReplacementStr = "typename"; + break; + case FormatStyle::TAS_Class: + BadToken = tok::kw_typename; + ReplacementStr = "class"; + break; + } + + for (size_t I = 0, E = AnnotatedLines.size(); I != E; ++I) { + FormatToken *Tok = AnnotatedLines[I]->First; + + // Find the first `template` keyword on this line + while (Tok && Tok->isNot(tok::kw_template)) + Tok = Tok->Next; + if (!Tok) + continue; + + // Find the corresponding `<`. Might be on the next line + Tok = Tok->Next; + while (!Tok && I < E) + Tok = AnnotatedLines[++I]->First; + if (Tok->isNot(TT_TemplateOpener) || !Tok->MatchingParen) { + // This is unexpected and indicates a syntax error. + // Just skip over this line and don't bother about it. + continue; + } + FormatToken *EndTok = Tok->MatchingParen; + + // Loop over the remaining tokens in the argument list and + // replace every `class`/`typename` keyword + while (Tok != EndTok) { + if (Tok->is(BadToken)) { + if (!KeepTemplateTemplateKW || + !Tok->Previous->ClosesTemplateDeclaration) { + replaceToken(SourceMgr, Fixes, Tok, ReplacementStr); + } + } + + bool StartedDefaultArg = Tok->is(tok::equal); + auto advanceTok = [&]() { + Tok = Tok->Next; + while (!Tok && I < E) + Tok = AnnotatedLines[++I]->First; + assert(Tok && "Unexpected end of token stream before encountering " + "matching parens"); + }; + advanceTok(); + if (StartedDefaultArg) { + // Default arguments might contain legitimate uses of the `typename` + // keyword which must not be rewritten to `class`. Example: + // > `template<class X = typename T::Nested> class A; + // Skip over the complete default clause. + unsigned NestingLevel = Tok->NestingLevel; + while (NestingLevel != Tok->NestingLevel || + !Tok->isOneOf(tok::comma, tok::greater)) { + advanceTok(); + } + } + } + } + + return {Fixes, 0}; +} + +} // namespace format +} // namespace clang Index: clang/lib/Format/Format.cpp =================================================================== --- clang/lib/Format/Format.cpp +++ clang/lib/Format/Format.cpp @@ -21,6 +21,7 @@ #include "NamespaceEndCommentsFixer.h" #include "QualifierAlignmentFixer.h" #include "SortJavaScriptImports.h" +#include "TemplateArgumentKeywordFixer.h" #include "TokenAnalyzer.h" #include "TokenAnnotator.h" #include "UnwrappedLineFormatter.h" @@ -137,6 +138,14 @@ } }; +template <> struct ScalarEnumerationTraits<FormatStyle::TemplateArgumentStyle> { + static void enumeration(IO &IO, FormatStyle::TemplateArgumentStyle &Value) { + IO.enumCase(Value, "Leave", FormatStyle::TAS_Leave); + IO.enumCase(Value, "typename", FormatStyle::TAS_Typename); + IO.enumCase(Value, "class", FormatStyle::TAS_Class); + } +}; + template <> struct ScalarEnumerationTraits<FormatStyle::ShortFunctionStyle> { static void enumeration(IO &IO, FormatStyle::ShortFunctionStyle &Value) { IO.enumCase(Value, "None", FormatStyle::SFS_None); @@ -815,6 +824,7 @@ Style.StatementAttributeLikeMacros); IO.mapOptional("StatementMacros", Style.StatementMacros); IO.mapOptional("TabWidth", Style.TabWidth); + IO.mapOptional("TemplateArgumentKeyword", Style.TemplateArgumentKeyword); IO.mapOptional("TypenameMacros", Style.TypenameMacros); IO.mapOptional("UseCRLF", Style.UseCRLF); IO.mapOptional("UseTab", Style.UseTab); @@ -1225,6 +1235,8 @@ LLVMStyle.SpacesInAngles = FormatStyle::SIAS_Never; LLVMStyle.SpacesInConditionalStatement = false; + LLVMStyle.TemplateArgumentKeyword = FormatStyle::TAS_Leave; + LLVMStyle.PenaltyBreakAssignment = prec::Assignment; LLVMStyle.PenaltyBreakComment = 300; LLVMStyle.PenaltyBreakFirstLessLess = 120; @@ -3038,6 +3050,13 @@ }); } + if (Style.isCpp() && + Style.TemplateArgumentKeyword != FormatStyle::TAS_Leave) { + Passes.emplace_back([&](const Environment &Env) { + return TemplateArgumentKeywordFixer(Env, Expanded).process(); + }); + } + if (Style.Language == FormatStyle::LK_Cpp) { if (Style.FixNamespaceComments) Passes.emplace_back([&](const Environment &Env) { Index: clang/lib/Format/CMakeLists.txt =================================================================== --- clang/lib/Format/CMakeLists.txt +++ clang/lib/Format/CMakeLists.txt @@ -13,6 +13,7 @@ SortJavaScriptImports.cpp TokenAnalyzer.cpp TokenAnnotator.cpp + TemplateArgumentKeywordFixer.cpp UnwrappedLineFormatter.cpp UnwrappedLineParser.cpp UsingDeclarationsSorter.cpp Index: clang/include/clang/Format/Format.h =================================================================== --- clang/include/clang/Format/Format.h +++ clang/include/clang/Format/Format.h @@ -1929,6 +1929,39 @@ /// \version 14 std::vector<std::string> QualifierOrder; + /// Different template argument styles. + enum TemplateArgumentStyle { + /// Don't change `class` to `typename` or vice versa (default). + /// \code + /// template<typename Key, class Value> class Map; + /// template<template<class X> class Y> f() + /// \endcode + TAS_Leave, + /// Use `typename` instead of `class`. + /// \code + /// template<typename Key, typename Value> class Map; + /// template<template<typename X> typename Y> f() + /// \endcode + TAS_Typename, // typename + /// Use `class` instead of `typename`. + /// \code + /// template<class Key, class Value> class Map; + /// template<class<class X> class Y> f(); + /// \endcode + TAS_Class // class + }; + + /// Keyword to use for template arguments (``class`` or ``typename``) + /// \warning + /// Setting ``TemplateArgumentKeyword`` to something other than `Leave`, + /// 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 + /// option. + /// \endwarning + /// \version 14 + TemplateArgumentStyle TemplateArgumentKeyword; + /// Different ways to break inheritance list. enum BreakInheritanceListStyle : unsigned char { /// Break inheritance list before the colon and after the commas. @@ -3826,6 +3859,7 @@ Standard == R.Standard && StatementAttributeLikeMacros == R.StatementAttributeLikeMacros && StatementMacros == R.StatementMacros && TabWidth == R.TabWidth && + TemplateArgumentKeyword == R.TemplateArgumentKeyword && UseTab == R.UseTab && UseCRLF == R.UseCRLF && TypenameMacros == R.TypenameMacros; } Index: clang/docs/ReleaseNotes.rst =================================================================== --- clang/docs/ReleaseNotes.rst +++ clang/docs/ReleaseNotes.rst @@ -294,6 +294,10 @@ `const` `volatile` `static` `inline` `constexpr` `restrict` to be controlled relative to the `type`. +- Option ``TemplateArgumentKeyword`` has been added to enforce consistent + usage of either the ``class`` or the ``typename`` keyword when declaring + template arguments. + - Add a ``Custom`` style to ``SpaceBeforeParens``, to better configure the space before parentheses. The custom options can be set using ``SpaceBeforeParensOptions``. Index: clang/docs/ClangFormatStyleOptions.rst =================================================================== --- clang/docs/ClangFormatStyleOptions.rst +++ clang/docs/ClangFormatStyleOptions.rst @@ -4000,6 +4000,45 @@ **TabWidth** (``Unsigned``) :versionbadge:`clang-format 3.7` The number of columns used for tab stops. +**TemplateArgumentKeyword** (``TemplateArgumentStyle``) :versionbadge:`clang-format 14` + Keyword to use for template arguments (``class`` or ``typename``) + + .. warning:: + + Setting ``TemplateArgumentKeyword`` to something other than `Leave`, + 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 + option. + + Possible values: + + * ``TAS_Leave`` (in configuration: ``Leave``) + Don't change `class` to `typename` or vice versa (default). + + .. code-block:: c++ + + template<typename Key, class Value> class Map; + template<template<class X> class Y> f() + + * ``TAS_Typename`` (in configuration: ``typename``) + Use `typename` instead of `class`. + + .. code-block:: c++ + + template<typename Key, typename Value> class Map; + template<template<typename X> typename Y> f() + + * ``TAS_Class`` (in configuration: ``class``) + Use `class` instead of `typename`. + + .. code-block:: c++ + + template<class Key, class Value> class Map; + template<class<class X> class Y> f(); + + + **TypenameMacros** (``List of Strings``) :versionbadge:`clang-format 9` A vector of macros that should be interpreted as type declarations instead of as function calls.
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
