Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -622,6 +622,19 @@ Improvements to Clang's diagnostics

- Improved the FixIts for unused lambda captures.

- ``-Wpointer-bool-conversion`` will now also warn in the following case

.. code-block:: c

struct B {
B(bool V) {}
};
void test(const B& b);
void test0(B* b) {
test(b); // this will call B::B(bool) and create a new B
}


Improvements to Clang's time-trace
----------------------------------

Expand Down
3 changes: 3 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -4397,6 +4397,9 @@ def ext_ms_impcast_fn_obj : ExtWarn<
"implicit conversion between pointer-to-function and pointer-to-object is a "
"Microsoft extension">, InGroup<MicrosoftCast>;

def warn_imp_constructor_pointer_to_bool : Warning<
"implicit conversion from %0 to %1 calls %q2; did you intend to dereference ?">,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"implicit conversion from %0 to %1 calls %q2; did you intend to dereference ?">,
"implicit conversion from %0 to %1 calls %q2; did you intend to dereference instead?">,

@erichkeane do you think we should have fixits attached to notes? Something like:

implicit conversion from %0 to %1 calls %q2 -- warning on the code
did you intend to dereference? -- with fixit to insert *
did you intend the bool conversion? -- with fixit to insert a (static_)cast or the !! trick

My concern is: if the user intended this behavior, it'd be nice for them to have a way to silence the diagnostic. And if they didn't intend this behavior, giving them a fix-it helps them correct their code.

InGroup<PointerBoolConversion>;
def warn_impcast_pointer_to_bool : Warning<
"address of %select{'%1'|function '%1'|array '%1'|lambda function pointer "
"conversion operator}0 will always evaluate to 'true'">,
Expand Down
22 changes: 22 additions & 0 deletions clang/lib/Sema/SemaChecking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11767,6 +11767,28 @@ static void CheckImplicitArgumentConversions(Sema &S, const CallExpr *TheCall,
SourceLocation CC) {
for (unsigned I = 0, N = TheCall->getNumArgs(); I < N; ++I) {
const Expr *CurrA = TheCall->getArg(I);

if (const auto *MTE = dyn_cast<MaterializeTemporaryExpr>(CurrA))
// We shouldnt skip over any node here as it may be an attempt to silence
// the warning
if (const auto *CCE = dyn_cast<CXXConstructExpr>(MTE->getSubExpr()))
if (CCE->getNumArgs() == 1 &&
CCE->getArg(0)->getType()->isBooleanType() &&
!CCE->getConstructor()->isExplicit()) {
const Expr *Inner = CCE->getArg(0)->IgnoreImpCasts();
if ((Inner->getType()->isAnyPointerType() &&
Inner->getType()->getPointeeType().getUnqualifiedType() ==
CCE->getType().getUnqualifiedType())) {
S.Diag(CCE->getLocation(),
diag::warn_imp_constructor_pointer_to_bool)
<< Inner->getType() << CCE->getType() << CCE->getConstructor()
<< FixItHint::CreateInsertion(Inner->getBeginLoc(), "*");
S.Diag(CCE->getConstructor()->getLocation(),
diag::note_entity_declared_at)
<< CCE->getConstructor();
}
}

if (!IsImplicitBoolFloatConversion(S, CurrA, true))
continue;

Expand Down
33 changes: 33 additions & 0 deletions clang/test/SemaCXX/warn-bool-conversion.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// RUN: %clang_cc1 -fsyntax-only -verify=expected,expected-cxx11 %s
// RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
// RUN: %clang_cc1 -fsyntax-only -verify=expected,expected-cxx11 -std=c++11 %s
// RUN: not %clang_cc1 -fsyntax-only %s -fdiagnostics-parseable-fixits 2>&1 | FileCheck %s

namespace BooleanFalse {
int* j = false;
Expand Down Expand Up @@ -234,3 +235,35 @@ namespace Template {
template void h<d>();
}
#endif // __cplusplus < 201703L

namespace implicit_constructor_bool {

struct B {
bool a;
B(bool V) : a(V) {} // expected-note {{'B' declared here}}
};

void test(const B& b);

void test0(B* b) {
test(b); // expected-warning {{implicit conversion from 'B *' to 'const B' calls 'implicit_constructor_bool::B::B'; did you intend to dereference ?}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:8-[[@LINE-1]]:8}:"*"
test((const B&)b);
test(B(b));
test((bool)b);
test(static_cast<bool>(b));
test(*b);
}

struct C {
bool a;
explicit C(bool V) : a(V) {}
};

void testC(const C& b); // expected-note {{candidate function not viable: no known conversion from 'C *' to 'const C' for 1st argument; dereference the argument with *}}

void testC0(C* b) {
testC(b); // expected-error {{no matching function for call to 'testC'}}
}

}