bernhardmgruber created this revision.
bernhardmgruber added reviewers: aaron.ballman, JonasToth, Eugene.Zelenko,
lebedev.ri.
bernhardmgruber added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.
bernhardmgruber updated this revision to Diff 265993.
bernhardmgruber edited the summary of this revision.
bernhardmgruber marked 3 inline comments as done.
bernhardmgruber added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:285-302
+ SourceLocation End =
+ expandIfMacroId(ReturnLoc.getSourceRange().getEnd(), SM);
+ SourceLocation BeginNameF = expandIfMacroId(F.getLocation(), SM);
+
+ // Extend the ReturnTypeRange until the last token before the function
+ // name.
+ std::pair<FileID, unsigned> Loc = SM.getDecomposedLoc(End);
----------------
Is there an easier way to get the token previous to the function name?
================
Comment at:
clang-tools-extra/docs/clang-tidy/checks/modernize-use-trailing-return-type.rst:22
+ virtual float f3() const && = delete; virtual auto f3() const && -> float
= delete;
+========================================
===============================================
----------------
I tried 2 online rst editors and they failed to format the code blocks inside
the tables. Will this work with the clang documentation?
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/modernize-use-trailing-return-type.cpp:550
+
+#if __cplusplus > 201703L /* C++2a and later */
+
----------------
How do you want to handle these tests which require C++20? I have seen other
checks use a separate file for tests which require a different language version.
- fixed flags in RUN declaration in lit test
- added tests for C++20 concepts and requires clause
- added manual tokenization for AutoType return types which are not plain
'auto' to find source range
- 'const auto'
- 'Concept<int> auto'
- 'decltype(auto)'
- improved formatting of documentation
- support for 'decltype(...)'
- added test for bug 44206, which seems to be fixed
https://reviews.llvm.org/D80514
Files:
clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.h
clang-tools-extra/docs/clang-tidy/checks/modernize-use-trailing-return-type.rst
clang-tools-extra/test/clang-tidy/checkers/modernize-use-trailing-return-type.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-trailing-return-type.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-trailing-return-type.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-trailing-return-type.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy -std=c++14,c++17 %s modernize-use-trailing-return-type %t -- -- -fdeclspec -fexceptions
+// RUN: %check_clang_tidy -std=c++14-or-later %s modernize-use-trailing-return-type %t -- -- -fdeclspec -fexceptions
// FIXME: Fix the checker to work in C++2a mode, it is performing a
// use-of-uninitialized-value.
@@ -11,6 +11,8 @@
class string;
+ class ostream;
+
template <typename T>
auto declval() -> T;
}
@@ -215,18 +217,28 @@
// CHECK-FIXES: {{^}}auto e13() -> struct A;{{$}}
//
-// decltype (unsupported if top level expression)
+// deduced return types
//
+const auto ded1();
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto ded1() -> const auto;{{$}}
+const auto& ded2();
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto ded2() -> const auto&;{{$}}
+
+decltype(auto) ded3();
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto ded3() -> decltype(auto);{{$}}
+
+
decltype(1 + 2) dec1() { return 1 + 2; }
// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
-// TODO: source range of DecltypeTypeLoc not yet implemented
-// _HECK-FIXES: {{^}}auto dec1() -> decltype(1 + 2) { return 1 + 2; }{{$}}
+// CHECK-FIXES: {{^}}auto dec1() -> decltype(1 + 2) { return 1 + 2; }{{$}}
template <typename F, typename T>
decltype(std::declval<F>(std::declval<T>)) dec2(F f, T t) { return f(t); }
// CHECK-MESSAGES: :[[@LINE-1]]:44: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
-// TODO: source range of DecltypeTypeLoc not yet implemented
-// _HECK-FIXES: {{^}}auto dec2(F f, T t) -> decltype(std::declval<F>(std::declval<T>)) { return f(t); }{{$}}
+// CHECK-FIXES: {{^}}auto dec2(F f, T t) -> decltype(std::declval<F>(std::declval<T>)) { return f(t); }{{$}}
template <typename T>
typename decltype(std::declval<T>())::value_type dec3();
// CHECK-MESSAGES: :[[@LINE-1]]:50: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
@@ -531,6 +543,71 @@
return {0};
}
+//
+// Concepts
+//
+
+#if __cplusplus > 201703L /* C++2a and later */
+
+namespace std {
+template <typename T, typename U>
+struct is_same { static constexpr auto value = false; };
+
+template <typename T>
+struct is_same<T, T> { static constexpr auto value = true; };
+
+template <typename T>
+concept floating_point = std::is_same<T, float>::value || std::is_same<T, double>::value || std::is_same<T, long double>::value;
+}
+
+std::floating_point auto con1();
+// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto con1() -> std::floating_point auto;{{$}}
+
+std::floating_point auto con1() { return 3.14f; }
+// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto con1() -> std::floating_point auto { return 3.14f; }{{$}}
+
+namespace a {
+template <typename T>
+concept Concept = true;
+
+template <typename T, typename U>
+concept BinaryConcept = true;
+}
+
+a::Concept decltype(auto) con2();
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto con2() -> a::Concept decltype(auto);{{$}}
+
+a::BinaryConcept<int> decltype(auto) con3();
+// CHECK-MESSAGES: :[[@LINE-1]]:38: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto con3() -> a::BinaryConcept<int> decltype(auto);{{$}}
+
+const std::floating_point auto* volatile con4();
+// CHECK-MESSAGES: :[[@LINE-1]]:42: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto con4() -> const std::floating_point auto* volatile;{{$}}
+
+template <typename T>
+int req1(T t) requires std::floating_point<T>;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto req1(T t) -> int requires std::floating_point<T>;{{$}}
+
+template <typename T>
+T req2(T t) requires requires { t + t; };
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+ // CHECK-FIXES: {{^}}auto req2(T t) -> T requires requires { t + t; };{{$}}
+
+#endif
+
+//
+// bug 44206, no rewrite should happen due to collision with parameter name
+//
+
+using std::ostream;
+ostream& operator<<(ostream& ostream, int i);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}ostream& operator<<(ostream& ostream, int i);{{$}}
//
// Samples which do not trigger the check
@@ -544,7 +621,6 @@
template <typename T> auto f(T t) -> int;
auto ff();
-decltype(auto) fff();
void c();
void c(int arg);
Index: clang-tools-extra/docs/clang-tidy/checks/modernize-use-trailing-return-type.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/modernize-use-trailing-return-type.rst
+++ clang-tools-extra/docs/clang-tidy/checks/modernize-use-trailing-return-type.rst
@@ -11,58 +11,46 @@
Example
-------
-.. code-block:: c++
+======================================== ===============================================
+Before After
+======================================== ===============================================
+.. code-block:: c++ .. code-block:: c++
- int f1();
- inline int f2(int arg) noexcept;
- virtual float f3() const && = delete;
-
-transforms to:
-
-.. code-block:: c++
-
- auto f1() -> int;
- inline auto f2(int arg) -> int noexcept;
- virtual auto f3() const && -> float = delete;
+ int f1(); auto f1() -> int;
+ inline int f2(int arg) noexcept; inline auto f2(int arg) -> int noexcept;
+ virtual float f3() const && = delete; virtual auto f3() const && -> float = delete;
+======================================== ===============================================
Known Limitations
-----------------
The following categories of return types cannot be rewritten currently:
+
* function pointers
* member function pointers
* member pointers
-* decltype, when it is the top level expression
Unqualified names in the return type might erroneously refer to different entities after the rewrite.
Preventing such errors requires a full lookup of all unqualified names present in the return type in the scope of the trailing return type location.
This location includes e.g. function parameter names and members of the enclosing class (including all inherited classes).
Such a lookup is currently not implemented.
-Given the following piece of code
-
-.. code-block:: c++
-
- struct Object { long long value; };
- Object f(unsigned Object) { return {Object * 2}; }
- class CC {
- int Object;
- struct Object m();
- };
- Object CC::m() { return {0}; }
-
-a careless rewrite would produce the following output:
-
-.. code-block:: c++
-
- struct Object { long long value; };
- auto f(unsigned Object) -> Object { return {Object * 2}; } // error
- class CC {
- int Object;
- auto m() -> struct Object;
- };
- auto CC::m() -> Object { return {0}; } // error
-
-This code fails to compile because the Object in the context of f refers to the equally named function parameter.
-Similarly, the Object in the context of m refers to the equally named class member.
-The check can currently only detect a clash with a function parameter name.
+For example, a careless rewrite would produce the following output:
+
+======================================== ===============================================
+Before After
+======================================== ===============================================
+.. code-block:: c++ .. code-block:: c++
+
+ struct S { long long value; }; struct S { long long value; };
+ S f(unsigned S) { return {S * 2}; } auto f(unsigned S) -> S { return {S * 2}; } // error
+ class CC { class CC {
+ int S; int S;
+ struct S m(); auto m() -> struct S;
+ }; };
+ S CC::m() { return {0}; } auto CC::m() -> S { return {0}; } // error
+======================================== ===============================================
+
+This code fails to compile because the S in the context of f refers to the equally named function parameter.
+Similarly, the S in the context of m refers to the equally named class member.
+The check can currently only detect and avoid a clash with a function parameter name.
Index: clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.h
+++ clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.h
@@ -49,10 +49,11 @@
const SourceManager &SM,
const LangOptions &LangOpts);
SourceRange findReturnTypeAndCVSourceRange(const FunctionDecl &F,
+ const TypeLoc &ReturnLoc,
const ASTContext &Ctx,
const SourceManager &SM,
const LangOptions &LangOpts);
- bool keepSpecifiers(std::string &ReturnType, std::string &Auto,
+ void keepSpecifiers(std::string &ReturnType, std::string &Auto,
SourceRange ReturnTypeCVRange, const FunctionDecl &F,
const FriendDecl *Fr, const ASTContext &Ctx,
const SourceManager &SM, const LangOptions &LangOpts);
Index: clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
@@ -260,8 +260,8 @@
}
SourceRange UseTrailingReturnTypeCheck::findReturnTypeAndCVSourceRange(
- const FunctionDecl &F, const ASTContext &Ctx, const SourceManager &SM,
- const LangOptions &LangOpts) {
+ const FunctionDecl &F, const TypeLoc &ReturnLoc, const ASTContext &Ctx,
+ const SourceManager &SM, const LangOptions &LangOpts) {
// We start with the range of the return type and expand to neighboring
// qualifiers (const, volatile and restrict).
@@ -273,6 +273,35 @@
return {};
}
+ // If the return type is a constrained 'auto' or 'decltype(auto)', we need to
+ // include the tokens after the concept. Unfortunately, the source range of an
+ // AutoTypeLoc, if it is constrained, does not include the 'auto' or
+ // 'decltype(auto)'. If the return type is a plain 'decltype(...)', the
+ // source range only contains the first 'decltype' token.
+ auto ATL = ReturnLoc.getAs<AutoTypeLoc>();
+ if ((ATL && (ATL.isConstrained() ||
+ ATL.getAutoKeyword() == AutoTypeKeyword::DecltypeAuto)) ||
+ ReturnLoc.getAs<DecltypeTypeLoc>()) {
+ SourceLocation End =
+ expandIfMacroId(ReturnLoc.getSourceRange().getEnd(), SM);
+ SourceLocation BeginNameF = expandIfMacroId(F.getLocation(), SM);
+
+ // Extend the ReturnTypeRange until the last token before the function
+ // name.
+ std::pair<FileID, unsigned> Loc = SM.getDecomposedLoc(End);
+ StringRef File = SM.getBufferData(Loc.first);
+ const char *TokenBegin = File.data() + Loc.second;
+ Lexer Lexer(SM.getLocForStartOfFile(Loc.first), LangOpts, File.begin(),
+ TokenBegin, File.end());
+ Token T;
+ SourceLocation LastTLoc = End;
+ while (!Lexer.LexFromRawLexer(T) &&
+ SM.isBeforeInTranslationUnit(T.getLocation(), BeginNameF)) {
+ LastTLoc = T.getLocation();
+ }
+ ReturnTypeRange.setEnd(LastTLoc);
+ }
+
// If the return type has no local qualifiers, it's source range is accurate.
if (!hasAnyNestedLocalQualifiers(F.getReturnType()))
return ReturnTypeRange;
@@ -316,7 +345,7 @@
return ReturnTypeRange;
}
-bool UseTrailingReturnTypeCheck::keepSpecifiers(
+void UseTrailingReturnTypeCheck::keepSpecifiers(
std::string &ReturnType, std::string &Auto, SourceRange ReturnTypeCVRange,
const FunctionDecl &F, const FriendDecl *Fr, const ASTContext &Ctx,
const SourceManager &SM, const LangOptions &LangOpts) {
@@ -326,14 +355,14 @@
if (!F.isConstexpr() && !F.isInlineSpecified() &&
F.getStorageClass() != SC_Extern && F.getStorageClass() != SC_Static &&
!Fr && !(M && M->isVirtualAsWritten()))
- return true;
+ return;
// Tokenize return type. If it contains macros which contain a mix of
// qualifiers, specifiers and types, give up.
llvm::Optional<SmallVector<ClassifiedToken, 8>> MaybeTokens =
classifyTokensBeforeFunctionName(F, Ctx, SM, LangOpts);
if (!MaybeTokens)
- return false;
+ return;
// Find specifiers, remove them from the return type, add them to 'auto'.
unsigned int ReturnTypeBeginOffset =
@@ -367,13 +396,13 @@
DeletedChars += TLengthWithWS;
}
- return true;
+ return;
}
void UseTrailingReturnTypeCheck::registerMatchers(MatchFinder *Finder) {
- auto F = functionDecl(unless(anyOf(hasTrailingReturn(), returns(voidType()),
- returns(autoType()), cxxConversionDecl(),
- cxxMethodDecl(isImplicit()))))
+ auto F = functionDecl(
+ unless(anyOf(hasTrailingReturn(), returns(voidType()),
+ cxxConversionDecl(), cxxMethodDecl(isImplicit()))))
.bind("Func");
Finder->addMatcher(F, this);
@@ -396,11 +425,17 @@
if (F->getLocation().isInvalid())
return;
+ // Skip functions which return just 'auto'.
+ const auto *AT = F->getDeclaredReturnType()->getAs<AutoType>();
+ if (AT != nullptr && !AT->isConstrained() &&
+ AT->getKeyword() == AutoTypeKeyword::Auto &&
+ !hasAnyNestedLocalQualifiers(F->getDeclaredReturnType()))
+ return;
+
// TODO: implement those
if (F->getDeclaredReturnType()->isFunctionPointerType() ||
F->getDeclaredReturnType()->isMemberFunctionPointerType() ||
- F->getDeclaredReturnType()->isMemberPointerType() ||
- F->getDeclaredReturnType()->getAs<DecltypeType>() != nullptr) {
+ F->getDeclaredReturnType()->isMemberPointerType()) {
diag(F->getLocation(), Message);
return;
}
@@ -434,7 +469,7 @@
// discards user formatting and order of const, volatile, type, whitespace,
// space before & ... .
SourceRange ReturnTypeCVRange =
- findReturnTypeAndCVSourceRange(*F, Ctx, SM, LangOpts);
+ findReturnTypeAndCVSourceRange(*F, FTL.getReturnLoc(), Ctx, SM, LangOpts);
if (ReturnTypeCVRange.isInvalid())
return;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
