| Message ID | 55D22A8F.8060701@oracle.com |
|---|---|
| State | New |
| Headers | show |
On 08/17/2015 02:40 PM, Paolo Carlini wrote: > the bug is very clear: in C++11 we reject the testcase because > null_ptr_cst_p returns 'true' for 'false', whereas [conv.ptr] is > carefully worded in terms of integer literals not boolean literals. The > obvious fix, using == INTEGER_TYPE instead of CP_INTEGRAL_TYPE_P, > appears to work well. OK. I wonder if we can also drop the STRIP_NOPs on the C++11 path. Jason
Hi, On 08/17/2015 08:50 PM, Jason Merrill wrote: > On 08/17/2015 02:40 PM, Paolo Carlini wrote: >> the bug is very clear: in C++11 we reject the testcase because >> null_ptr_cst_p returns 'true' for 'false', whereas [conv.ptr] is >> carefully worded in terms of integer literals not boolean literals. The >> obvious fix, using == INTEGER_TYPE instead of CP_INTEGRAL_TYPE_P, >> appears to work well. > OK. I wonder if we can also drop the STRIP_NOPs on the C++11 path. You are not alone ;) No, not trivially, without we ICE on pr51313.C. Thanks, Paolo.
On 08/17/2015 02:52 PM, Paolo Carlini wrote: > On 08/17/2015 08:50 PM, Jason Merrill wrote: >> OK. I wonder if we can also drop the STRIP_NOPs on the C++11 path. > You are not alone ;) No, not trivially, without we ICE on pr51313.C. Hmm, that testcase is ill-formed. We ought to reject it, though not ICE. Jason
Hi, On 08/17/2015 08:59 PM, Jason Merrill wrote: > On 08/17/2015 02:52 PM, Paolo Carlini wrote: >> On 08/17/2015 08:50 PM, Jason Merrill wrote: >>> OK. I wonder if we can also drop the STRIP_NOPs on the C++11 path. >> You are not alone ;) No, not trivially, without we ICE on pr51313.C. > > Hmm, that testcase is ill-formed. We ought to reject it, though not ICE. Yes, now I see. At the moment however, I have no idea where that NOP_EXPR is coming from and whether it would be safe to assume in null_ptr_cst_p that one can occur only due to a bug elsewhere... Note that integer_zerop calls STRIP_NOPS right at the beginning anyway and the ICE comes from TREE_OVERFLOW. Anyway, about the substance of pr51313.C, is it Ok with you if I add an xfailed dg-error to it and investigate it separately? Thanks! Paolo.
On 08/17/2015 03:20 PM, Paolo Carlini wrote: > On 08/17/2015 08:59 PM, Jason Merrill wrote: >> On 08/17/2015 02:52 PM, Paolo Carlini wrote: >>> On 08/17/2015 08:50 PM, Jason Merrill wrote: >>>> OK. I wonder if we can also drop the STRIP_NOPs on the C++11 path. >>> You are not alone ;) No, not trivially, without we ICE on pr51313.C. >> >> Hmm, that testcase is ill-formed. We ought to reject it, though not ICE. > Yes, now I see. At the moment however, I have no idea where that > NOP_EXPR is coming from and whether it would be safe to assume in > null_ptr_cst_p that one can occur only due to a bug elsewhere... I imagine it's coming from built-in folding of isdigit, which is not a bug. > Note > that integer_zerop calls STRIP_NOPS right at the beginning anyway and > the ICE comes from TREE_OVERFLOW. So I guess null_ptr_cst_p should check for INTEGER_CST before calling integer_zerop. > Anyway, about the substance of pr51313.C, is it Ok with you if I add an > xfailed dg-error to it and investigate it separately? Please also reopen 51313 if you do that. Jason
Index: cp/call.c =================================================================== --- cp/call.c (revision 226939) +++ cp/call.c (working copy) @@ -524,22 +524,34 @@ struct z_candidate { bool null_ptr_cst_p (tree t) { + tree type = TREE_TYPE (t); + /* [conv.ptr] A null pointer constant is an integral constant expression (_expr.const_) rvalue of integer type that evaluates to zero or an rvalue of type std::nullptr_t. */ - if (NULLPTR_TYPE_P (TREE_TYPE (t))) + if (NULLPTR_TYPE_P (type)) return true; - if (CP_INTEGRAL_TYPE_P (TREE_TYPE (t))) + + if (cxx_dialect >= cxx11) { /* Core issue 903 says only literal 0 is a null pointer constant. */ - if (cxx_dialect < cxx11) - t = fold_non_dependent_expr (t); + if (TREE_CODE (type) == INTEGER_TYPE) + { + STRIP_NOPS (t); + if (integer_zerop (t) && !TREE_OVERFLOW (t)) + return true; + } + } + else if (CP_INTEGRAL_TYPE_P (type)) + { + t = fold_non_dependent_expr (t); STRIP_NOPS (t); if (integer_zerop (t) && !TREE_OVERFLOW (t)) return true; } + return false; } Index: testsuite/g++.dg/cpp0x/nullptr34.C =================================================================== --- testsuite/g++.dg/cpp0x/nullptr34.C (revision 0) +++ testsuite/g++.dg/cpp0x/nullptr34.C (working copy) @@ -0,0 +1,17 @@ +// PR c++/67216 +// { dg-do compile { target c++11 } } + +struct s { + s( long ) {} +}; + +struct t { + t( void * ) {} +}; + +void foo(s) {} +void foo(t) {} + +int main() { + foo(false); +} Index: testsuite/g++.dg/warn/Wconversion2.C =================================================================== --- testsuite/g++.dg/warn/Wconversion2.C (revision 226939) +++ testsuite/g++.dg/warn/Wconversion2.C (working copy) @@ -1,3 +1,4 @@ // { dg-options "-Wconversion-null" } void foo(const char *); -void bar() { foo(false); } // { dg-warning "pointer type for argument" } +void bar() { foo(false); } // { dg-warning "pointer type for argument" "" { target { ! c++11 } } } +// { dg-error "cannot convert" "" { target c++11 } 3 } Index: testsuite/g++.dg/warn/Wnull-conversion-1.C =================================================================== --- testsuite/g++.dg/warn/Wnull-conversion-1.C (revision 226939) +++ testsuite/g++.dg/warn/Wnull-conversion-1.C (working copy) @@ -6,10 +6,13 @@ void func1(int* ptr); void func2() { - int* t = false; // { dg-warning "converting 'false' to pointer" } + int* t = false; // { dg-warning "converting 'false' to pointer" "" { target { ! c++11 } } } +// { dg-error "cannot convert" "" { target c++11 } 9 } int* p; - p = false; // { dg-warning "converting 'false' to pointer" } + p = false; // { dg-warning "converting 'false' to pointer" "" { target { ! c++11 } } } +// { dg-error "cannot convert" "" { target c++11 } 12 } int* r = sizeof(char) / 2; // { dg-error "invalid conversion from" "" { target c++11 } } - func1(false); // { dg-warning "converting 'false' to pointer" } + func1(false); // { dg-warning "converting 'false' to pointer" "" { target { ! c++11 } } } +// { dg-error "cannot convert" "" { target c++11 } 15 } int i = NULL; // { dg-warning "converting to non-pointer" } } Index: testsuite/g++.old-deja/g++.other/null3.C =================================================================== --- testsuite/g++.old-deja/g++.other/null3.C (revision 226939) +++ testsuite/g++.old-deja/g++.other/null3.C (working copy) @@ -2,5 +2,6 @@ void x() { - int* p = 1==0; // { dg-warning "converting 'false' to pointer" } + int* p = 1==0; // { dg-warning "converting 'false' to pointer" "" { target { ! c++11 } } } +// { dg-error "cannot convert" "" { target c++11 } 5 } }