Author: alexfh
Date: Fri Dec 23 09:03:12 2016
New Revision: 290434
URL: http://llvm.org/viewvc/llvm-project?rev=290434&view=rev
Log:
[clang-tidy] Flag implicit conversion operators.
Modified:
clang-tools-extra/trunk/clang-tidy/google/ExplicitConstructorCheck.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/google-explicit-constructor.rst
clang-tools-extra/trunk/test/clang-tidy/google-explicit-constructor.cpp
Modified: clang-tools-extra/trunk/clang-tidy/google/ExplicitConstructorCheck.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/ExplicitConstructorCheck.cpp?rev=290434&r1=290433&r2=290434&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/google/ExplicitConstructorCheck.cpp
(original)
+++ clang-tools-extra/trunk/clang-tidy/google/ExplicitConstructorCheck.cpp Fri
Dec 23 09:03:12 2016
@@ -22,9 +22,12 @@ namespace google {
void ExplicitConstructorCheck::registerMatchers(MatchFinder *Finder) {
// Only register the matchers for C++; the functionality currently does not
// provide any benefit to other languages, despite being benign.
- if (getLangOpts().CPlusPlus)
- Finder->addMatcher(
- cxxConstructorDecl(unless(isInstantiated())).bind("ctor"), this);
+ if (!getLangOpts().CPlusPlus)
+ return;
+ Finder->addMatcher(cxxConstructorDecl(unless(isInstantiated())).bind("ctor"),
+ this);
+
Finder->addMatcher(cxxConversionDecl(unless(isExplicit())).bind("conversion"),
+ this);
}
// Looks for the token matching the predicate and returns the range of the
found
@@ -75,6 +78,17 @@ static bool isStdInitializerList(QualTyp
}
void ExplicitConstructorCheck::check(const MatchFinder::MatchResult &Result) {
+ constexpr char WarningMessage[] =
+ "%0 must be marked explicit to avoid unintentional implicit conversions";
+
+ if (const auto *Conversion =
+ Result.Nodes.getNodeAs<CXXConversionDecl>("conversion")) {
+ SourceLocation Loc = Conversion->getLocation();
+ diag(Loc, WarningMessage)
+ << Conversion << FixItHint::CreateInsertion(Loc, "explicit ");
+ return;
+ }
+
const auto *Ctor = Result.Nodes.getNodeAs<CXXConstructorDecl>("ctor");
// Do not be confused: isExplicit means 'explicit' keyword is present,
// isImplicit means that it's a compiler-generated constructor.
@@ -101,10 +115,9 @@ void ExplicitConstructorCheck::check(con
else
ConstructorDescription = "initializer-list";
- DiagnosticBuilder Diag =
- diag(Ctor->getLocation(),
- "%0 constructor should not be declared explicit")
- << ConstructorDescription;
+ auto Diag = diag(Ctor->getLocation(),
+ "%0 constructor should not be declared explicit")
+ << ConstructorDescription;
if (ExplicitTokenRange.isValid()) {
Diag << FixItHint::CreateRemoval(
CharSourceRange::getCharRange(ExplicitTokenRange));
@@ -119,8 +132,7 @@ void ExplicitConstructorCheck::check(con
bool SingleArgument =
Ctor->getNumParams() == 1 && !Ctor->getParamDecl(0)->isParameterPack();
SourceLocation Loc = Ctor->getLocation();
- diag(Loc,
- "%0 must be marked explicit to avoid unintentional implicit
conversions")
+ diag(Loc, WarningMessage)
<< (SingleArgument
? "single-argument constructors"
: "constructors that are callable with a single argument")
Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=290434&r1=290433&r2=290434&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Fri Dec 23 09:03:12 2016
@@ -183,6 +183,10 @@ Improvements to clang-tidy
<http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-string-cstr.html>`_
check
now warns about redundant calls to data() too.
+- The `google-explicit-constructor
+
<http://clang.llvm.org/extra/clang-tidy/checks/google-explicit-constructor.html>`_
check
+ now warns about conversion operators not marked explicit.
+
Fixed bugs:
- `modernize-make-unique
Modified:
clang-tools-extra/trunk/docs/clang-tidy/checks/google-explicit-constructor.rst
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/google-explicit-constructor.rst?rev=290434&r1=290433&r2=290434&view=diff
==============================================================================
---
clang-tools-extra/trunk/docs/clang-tidy/checks/google-explicit-constructor.rst
(original)
+++
clang-tools-extra/trunk/docs/clang-tidy/checks/google-explicit-constructor.rst
Fri Dec 23 09:03:12 2016
@@ -4,6 +4,53 @@ google-explicit-constructor
===========================
-Checks that all single-argument constructors are explicit.
+Checks that constructors callable with a single argument and conversion
+operators are marked explicit to avoid the risk of unintentional implicit
+conversions.
+
+Consider this example:
+
+.. code-block:: c++
+
+ struct S {
+ int x;
+ operator bool() const { return true; }
+ };
+
+ bool f() {
+ S a{1};
+ S b{2};
+ return a == b;
+ }
+
+The function will return ``true``, since the objects are implicitly converted
to
+``bool`` before comparison, which is unlikely to be the intent.
+
+The check will suggest inserting ``explicit`` before the constructor or
+conversion operator declaration. However, copy and move constructors should not
+be explicit, as well as constructors taking a single ``initializer_list``
+argument.
+
+This code:
+
+.. code-block:: c++
+
+ struct S {
+ S(int a);
+ explicit S(const S&);
+ operator bool() const;
+ ...
+
+will become
+
+.. code-block:: c++
+
+ struct S {
+ explicit S(int a);
+ S(const S&);
+ explicit operator bool() const;
+ ...
+
+
See https://google.github.io/styleguide/cppguide.html#Explicit_Constructors
Modified:
clang-tools-extra/trunk/test/clang-tidy/google-explicit-constructor.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/google-explicit-constructor.cpp?rev=290434&r1=290433&r2=290434&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/google-explicit-constructor.cpp
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/google-explicit-constructor.cpp Fri
Dec 23 09:03:12 2016
@@ -38,6 +38,7 @@ struct A {
explicit A(void *x) {}
explicit A(void *x, void *y) {}
+ explicit operator bool() const { return true; }
explicit A(const A& a) {}
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: copy constructor should not be
declared explicit [google-explicit-constructor]
@@ -62,6 +63,10 @@ struct B {
B(const std::initializer_list<unsigned> &list2) {}
B(std::initializer_list<unsigned> &&list3) {}
+ operator bool() const { return true; }
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'operator bool' must be marked
explicit to avoid unintentional implicit conversions
[google-explicit-constructor]
+ // CHECK-FIXES: {{^ }}explicit operator bool() const { return true; }
+
explicit B(::std::initializer_list<double> list4) {}
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: initializer-list constructor
should not be declared explicit [google-explicit-constructor]
// CHECK-FIXES: {{^ }}B(::std::initializer_list<double> list4) {}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
