[cfe-commits] r161501 - in /cfe/trunk: include/clang/AST/Expr.h include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td lib/AST/Expr.cpp lib/Sema/SemaExpr.cpp lib/Sema/SemaOverload.cpp test/SemaCXX/constant-expression-c
David Blaikie
dblaikie at gmail.com
Fri Aug 17 15:43:28 PDT 2012
On Fri, Aug 17, 2012 at 3:33 PM, Nico Weber <thakis at chromium.org> wrote:
> Should this really be on by default? On chrome, this triggers a single
> time (linux-only):
>
> ../../third_party/tcmalloc/chromium/src/stack_trace_table.cc:138:16:
> warning: expression which evaluates to zero treated as a null pointer
> constant of type 'void *' [-Wnon-literal-null-conversion]
> out[idx++] = static_cast<uintptr_t>(0);
> ^~~~~~~~~~~~~~~~~~~~~~~~~
>
> out is declared as `void** out = new void*[out_len];`. The warning
> isn't wrong, but it looks rather pedantic to me. Should this be only
> in -Wall (or maybe even in -pedantic)?
Might be a fair candidate for -Wall, though it did find some
reasonable stuff in google. 18 cases overall with some fairly
interesting ones (see b/6954211 for the ones that've been committed so
far, or cl/32692314 for some of the remaining ones.
The worst offenders are integer constants with value 0 that aren't at
all intended to be pointers. (most easily occurred in function calls
where the caller thought the argument was of one type but it's
actually of a pointer type)
I have some more once this warning opens up to cover comparisons,
conditional operands, and return statements - there's a lot of
confusing "cstr == '\0'" code where the user probably meant to deref
the lhs but didn't.
>
> On Wed, Aug 8, 2012 at 10:33 AM, David Blaikie <dblaikie at gmail.com> wrote:
>> Author: dblaikie
>> Date: Wed Aug 8 12:33:31 2012
>> New Revision: 161501
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=161501&view=rev
>> Log:
>> Implement warning for integral null pointer constants other than the literal 0.
>>
>> This is effectively a warning for code that violates core issue 903 & thus will
>> become standard error in the future, hopefully. It catches strange null
>> pointers such as: '\0', 1 - 1, const int null = 0; etc...
>>
>> There's currently a flaw in this warning (& the warning for 'false' as a null
>> pointer literal as well) where it doesn't trigger on comparisons (ptr == '\0'
>> for example). Fix to come in a future patch.
>>
>> Also, due to this only being a warning, not an error, it triggers quite
>> frequently on gtest code which tests expressions for null-pointer-ness in a
>> SFINAE context (so it wouldn't be a problem if this was an error as in an
>> actual implementation of core issue 903). To workaround this for now, the
>> diagnostic does not fire in unevaluated contexts.
>>
>> Review by Sean Silva and Richard Smith.
>>
>> Modified:
>> cfe/trunk/include/clang/AST/Expr.h
>> cfe/trunk/include/clang/Basic/DiagnosticGroups.td
>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> cfe/trunk/lib/AST/Expr.cpp
>> cfe/trunk/lib/Sema/SemaExpr.cpp
>> cfe/trunk/lib/Sema/SemaOverload.cpp
>> cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp
>> cfe/trunk/test/SemaCXX/lambda-expressions.cpp
>> cfe/trunk/test/SemaTemplate/explicit-instantiation.cpp
>> cfe/trunk/test/SemaTemplate/instantiate-member-class.cpp
>>
>> Modified: cfe/trunk/include/clang/AST/Expr.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=161501&r1=161500&r2=161501&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/AST/Expr.h (original)
>> +++ cfe/trunk/include/clang/AST/Expr.h Wed Aug 8 12:33:31 2012
>> @@ -541,8 +541,15 @@
>> /// \brief Expression is not a Null pointer constant.
>> NPCK_NotNull = 0,
>>
>> - /// \brief Expression is a Null pointer constant built from a zero integer.
>> - NPCK_ZeroInteger,
>> + /// \brief Expression is a Null pointer constant built from a zero integer
>> + /// expression that is not a simple, possibly parenthesized, zero literal.
>> + /// C++ Core Issue 903 will classify these expressions as "not pointers"
>> + /// once it is adopted.
>> + /// http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#903
>> + NPCK_ZeroExpression,
>> +
>> + /// \brief Expression is a Null pointer constant built from a literal zero.
>> + NPCK_ZeroLiteral,
>>
>> /// \brief Expression is a C++0X nullptr.
>> NPCK_CXX0X_nullptr,
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=161501&r1=161500&r2=161501&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Wed Aug 8 12:33:31 2012
>> @@ -33,6 +33,7 @@
>> def SignConversion : DiagGroup<"sign-conversion">;
>> def BoolConversion : DiagGroup<"bool-conversion">;
>> def IntConversion : DiagGroup<"int-conversion">;
>> +def NonLiteralNullConversion : DiagGroup<"non-literal-null-conversion">;
>> def NullConversion : DiagGroup<"null-conversion">;
>> def BuiltinRequiresHeader : DiagGroup<"builtin-requires-header">;
>> def CXXCompat: DiagGroup<"c++-compat">;
>> @@ -314,7 +315,8 @@
>> StringConversion,
>> SignConversion,
>> BoolConversion,
>> - NullConversion,
>> + NullConversion, // NULL->non-pointer
>> + NonLiteralNullConversion, // (1-1)->pointer (etc)
>> IntConversion]>,
>> DiagCategory<"Value Conversion Issue">;
>>
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=161501&r1=161500&r2=161501&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Aug 8 12:33:31 2012
>> @@ -1896,6 +1896,9 @@
>> def warn_impcast_bool_to_null_pointer : Warning<
>> "initialization of pointer of type %0 to null from a constant boolean "
>> "expression">, InGroup<BoolConversion>;
>> +def warn_non_literal_null_pointer : Warning<
>> + "expression which evaluates to zero treated as a null pointer constant of "
>> + "type %0">, InGroup<NonLiteralNullConversion>;
>> def warn_impcast_null_pointer_to_integer : Warning<
>> "implicit conversion of NULL constant to %0">,
>> InGroup<NullConversion>;
>>
>> Modified: cfe/trunk/lib/AST/Expr.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=161501&r1=161500&r2=161501&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/AST/Expr.cpp (original)
>> +++ cfe/trunk/lib/AST/Expr.cpp Wed Aug 8 12:33:31 2012
>> @@ -2903,7 +2903,7 @@
>> llvm_unreachable("Unexpected value dependent expression!");
>> case NPC_ValueDependentIsNull:
>> if (isTypeDependent() || getType()->isIntegralType(Ctx))
>> - return NPCK_ZeroInteger;
>> + return NPCK_ZeroExpression;
>> else
>> return NPCK_NotNull;
>>
>> @@ -2977,7 +2977,12 @@
>> return NPCK_NotNull;
>> }
>>
>> - return (EvaluateKnownConstInt(Ctx) == 0) ? NPCK_ZeroInteger : NPCK_NotNull;
>> + if (EvaluateKnownConstInt(Ctx) != 0)
>> + return NPCK_NotNull;
>> +
>> + if (isa<IntegerLiteral>(this))
>> + return NPCK_ZeroLiteral;
>> + return NPCK_ZeroExpression;
>> }
>>
>> /// \brief If this expression is an l-value for an Objective C
>>
>> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=161501&r1=161500&r2=161501&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Aug 8 12:33:31 2012
>> @@ -4632,7 +4632,10 @@
>> if (NullKind == Expr::NPCK_NotNull)
>> return false;
>>
>> - if (NullKind == Expr::NPCK_ZeroInteger) {
>> + if (NullKind == Expr::NPCK_ZeroExpression)
>> + return false;
>> +
>> + if (NullKind == Expr::NPCK_ZeroLiteral) {
>> // In this case, check to make sure that we got here from a "NULL"
>> // string in the source code.
>> NullExpr = NullExpr->IgnoreParenImpCasts();
>>
>> Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=161501&r1=161500&r2=161501&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaOverload.cpp Wed Aug 8 12:33:31 2012
>> @@ -2562,13 +2562,17 @@
>>
>> Kind = CK_BitCast;
>>
>> - if (!IsCStyleOrFunctionalCast &&
>> - Context.hasSameUnqualifiedType(From->getType(), Context.BoolTy) &&
>> - From->isNullPointerConstant(Context, Expr::NPC_ValueDependentIsNotNull))
>> - DiagRuntimeBehavior(From->getExprLoc(), From,
>> - PDiag(diag::warn_impcast_bool_to_null_pointer)
>> - << ToType << From->getSourceRange());
>> -
>> + if (!IsCStyleOrFunctionalCast && !FromType->isAnyPointerType() &&
>> + From->isNullPointerConstant(Context, Expr::NPC_ValueDependentIsNotNull) ==
>> + Expr::NPCK_ZeroExpression) {
>> + if (Context.hasSameUnqualifiedType(From->getType(), Context.BoolTy))
>> + DiagRuntimeBehavior(From->getExprLoc(), From,
>> + PDiag(diag::warn_impcast_bool_to_null_pointer)
>> + << ToType << From->getSourceRange());
>> + else if (!isUnevaluatedContext())
>> + Diag(From->getExprLoc(), diag::warn_non_literal_null_pointer)
>> + << ToType << From->getSourceRange();
>> + }
>> if (const PointerType *ToPtrType = ToType->getAs<PointerType>()) {
>> if (const PointerType *FromPtrType = FromType->getAs<PointerType>()) {
>> QualType FromPointeeType = FromPtrType->getPointeeType(),
>>
>> Modified: cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp?rev=161501&r1=161500&r2=161501&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp (original)
>> +++ cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp Wed Aug 8 12:33:31 2012
>> @@ -710,10 +710,16 @@
>> static_assert((Base2*)(Derived*)(Base*)pb1 == pok2, "");
>> static_assert((Derived*)(Base*)pb1 == (Derived*)pok2, "");
>>
>> -constexpr Base *nullB = 42 - 6 * 7;
>> +constexpr Base *nullB = 42 - 6 * 7; // expected-warning {{expression which evaluates to zero treated as a null pointer constant of type 'Class::Base *const'}}
>> static_assert((Bottom*)nullB == 0, "");
>> static_assert((Derived*)nullB == 0, "");
>> static_assert((void*)(Bottom*)nullB == (void*)(Derived*)nullB, "");
>> +Base * nullB2 = '\0'; // expected-warning {{expression which evaluates to zero treated as a null pointer constant of type 'Class::Base *'}}
>> +Base * nullB3 = (0);
>> +// We suppress the warning in unevaluated contexts to workaround some gtest
>> +// behavior. Once this becomes an error this isn't a problem anymore.
>> +static_assert(nullB == (1 - 1), "");
>> +
>>
>> namespace ConversionOperators {
>>
>>
>> Modified: cfe/trunk/test/SemaCXX/lambda-expressions.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/lambda-expressions.cpp?rev=161501&r1=161500&r2=161501&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/SemaCXX/lambda-expressions.cpp (original)
>> +++ cfe/trunk/test/SemaCXX/lambda-expressions.cpp Wed Aug 8 12:33:31 2012
>> @@ -117,16 +117,16 @@
>>
>> const int m = 0;
>> [=] {
>> - int &k = f(m); // a null pointer constant
>> + int &k = f(m); // expected-warning{{expression which evaluates to zero treated as a null pointer constant of type 'int *'}}
>> } ();
>>
>> [=] () -> bool {
>> - int &k = f(m); // a null pointer constant
>> + int &k = f(m); // expected-warning{{expression which evaluates to zero treated as a null pointer constant of type 'int *'}}
>> return &m == 0;
>> } ();
>>
>> [m] {
>> - int &k = f(m); // a null pointer constant
>> + int &k = f(m); // expected-warning{{expression which evaluates to zero treated as a null pointer constant of type 'int *'}}
>> } ();
>> }
>> }
>>
>> Modified: cfe/trunk/test/SemaTemplate/explicit-instantiation.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/explicit-instantiation.cpp?rev=161501&r1=161500&r2=161501&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/SemaTemplate/explicit-instantiation.cpp (original)
>> +++ cfe/trunk/test/SemaTemplate/explicit-instantiation.cpp Wed Aug 8 12:33:31 2012
>> @@ -14,7 +14,7 @@
>> T f0(T x) {
>> return x + 1; // expected-error{{invalid operands}}
>> }
>> - T* f0(T*, T*) { return T(); }
>> + T* f0(T*, T*) { return T(); } // expected-warning{{expression which evaluates to zero treated as a null pointer constant of type 'int *'}}
>>
>> template<typename U>
>> T f0(T, U) { return T(); }
>> @@ -32,7 +32,7 @@
>> template NotDefaultConstructible X0<NotDefaultConstructible>::value; // expected-note{{instantiation}}
>>
>> template int X0<int>::f0(int);
>> -template int* X0<int>::f0(int*, int*);
>> +template int* X0<int>::f0(int*, int*); // expected-note{{in instantiation of member function 'X0<int>::f0' requested here}}
>> template int X0<int>::f0(int, float);
>>
>> template int X0<int>::f0(int) const; // expected-error{{does not refer}}
>>
>> Modified: cfe/trunk/test/SemaTemplate/instantiate-member-class.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/instantiate-member-class.cpp?rev=161501&r1=161500&r2=161501&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/SemaTemplate/instantiate-member-class.cpp (original)
>> +++ cfe/trunk/test/SemaTemplate/instantiate-member-class.cpp Wed Aug 8 12:33:31 2012
>> @@ -124,19 +124,20 @@
>> {
>> struct B
>> {
>> - struct C { C() { int *ptr = I; } }; // expected-error{{cannot initialize a variable of type 'int *' with an rvalue of type 'int'}}
>> + struct C { C() { int *ptr = I; } }; // expected-error{{cannot initialize a variable of type 'int *' with an rvalue of type 'int'}} \
>> + expected-warning{{expression which evaluates to zero treated as a null pointer constant of type 'int *'}}
>> };
>> };
>>
>> template<int N> void foo()
>> {
>> - class A<N>::B::C X; // expected-note{{in instantiation of member function}}
>> + class A<N>::B::C X; // expected-note 2 {{in instantiation of member function}}
>> int A<N+1>::B::C::*member = 0;
>> }
>>
>> void bar()
>> {
>> - foo<0>();
>> + foo<0>(); // expected-note{{in instantiation of function template}}
>> foo<1>(); // expected-note{{in instantiation of function template}}
>> }
>> }
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list