RE: r349201 - Add extension to always default-initialize nullptr_t.
Hi Richard-
I finally got a chance to take a look at this patch and run it through the
nullptr_t tests that we have, and it seems to solve the problem I was
attempting. It would be my preference to commit this.
I currently have my existing tests (from the previous patch) and am happy with
the messages. What work to you believe needs to happen in order to get this
committed? I note that you didn’t have tests in it and I’m not sure what you
were attempting to test initially, but if you can tell me what you were
attempting to solve, I could perhaps come up with a handful of tests and commit
this.
Thanks!
-Erich
From: Keane, Erich
Sent: Friday, December 14, 2018 2:57 PM
To: Richard Smith
Cc: cfe-commits
Subject: RE: r349201 - Add extension to always default-initialize nullptr_t.
Thanks! I’ll take a look in a bit!
-Erich
From: Richard Smith [mailto:rich...@metafoo.co.uk]
Sent: Friday, December 14, 2018 2:55 PM
To: Keane, Erich mailto:erich.ke...@intel.com>>
Cc: cfe-commits mailto:cfe-commits@lists.llvm.org>>
Subject: Re: r349201 - Add extension to always default-initialize nullptr_t.
Attached. With the functional part of your change reverted and this applied,
the modified tests still pass except
error: 'note' diagnostics expected but not seen:
File
/usr/local/google/home/richardsmith/llvm-git-1/src/tools/clang/test/Analysis/nullptr.cpp
Line 128: 'p' initialized to a null pointer value
error: 'note' diagnostics seen but not expected:
File
/usr/local/google/home/richardsmith/llvm-git-1/src/tools/clang/test/Analysis/nullptr.cpp
Line 128: 'p' declared without an initial value
... which seems accurate (but perhaps not useful).
On Fri, 14 Dec 2018 at 14:47, Richard Smith
mailto:rich...@metafoo.co.uk>> wrote:
I have a patch I put together a while back as an attempt to fix a different
bug, that creates a new CastKind for this operation (didn't work out there, but
it might do so here). I'll dig it out and mail it to you in case it's a useful
starting point.
On Fri, 14 Dec 2018 at 14:42, Keane, Erich via cfe-commits
mailto:cfe-commits@lists.llvm.org>> wrote:
Alright, no problem. I’ll push a revert in a few minutes and give it another
try. Perhaps I can suppress the creation of a CK_LValueToRValue in the case
where it’s a read from nullptr_t
From: Richard Smith [mailto:rich...@metafoo.co.uk<mailto:rich...@metafoo.co.uk>]
Sent: Friday, December 14, 2018 2:41 PM
To: Keane, Erich mailto:erich.ke...@intel.com>>
Cc: cfe-commits mailto:cfe-commits@lists.llvm.org>>
Subject: Re: r349201 - Add extension to always default-initialize nullptr_t.
Sorry, I was late with my review comment. I think this is the wrong way to
approach this problem. This does not "fix all situations where nullptr_t would
seem uninitialized", and it makes our AST representation lose source fidelity.
On Fri, 14 Dec 2018 at 14:25, Erich Keane via cfe-commits
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: erichkeane
Date: Fri Dec 14 14:22:29 2018
New Revision: 349201
URL: http://llvm.org/viewvc/llvm-project?rev=349201=rev
Log:
Add extension to always default-initialize nullptr_t.
Core issue 1013 suggests that having an uninitialied std::nullptr_t be
UB is a bit foolish, since there is only a single valid value. This DR
reports that DR616 fixes it, which does so by making lvalue-to-rvalue
conversions from nullptr_t be equal to nullptr.
However, just implementing that results in warnings/etc in many places.
In order to fix all situations where nullptr_t would seem uninitialized,
this patch instead (as an otherwise transparent extension) default
initializes uninitialized VarDecls of nullptr_t.
Differential Revision: https://reviews.llvm.org/D53713
Change-Id: I84d72a9290054fa55341e8cbdac43c8e7f25b885
Added:
cfe/trunk/test/SemaCXX/nullptr_t-init.cpp (with props)
Modified:
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/test/Analysis/nullptr.cpp
cfe/trunk/test/SemaCXX/ast-print-crash.cpp
Modified: cfe/trunk/lib/Sema/SemaInit.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=349201=349200=349201=diff
==
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Fri Dec 14 14:22:29 2018
@@ -4881,6 +4881,13 @@ static void TryDefaultInitialization(Sem
return;
}
+ // As an extension, and to fix Core issue 1013, zero initialize nullptr_t.
+ // Since there is only 1 valid value of nullptr_t, we can just use that.
+ if (DestType->isNullPtrType()) {
+Sequence.AddZeroInitializationStep(Entity.getType());
+return;
+ }
+
// - otherwise, no initialization is performed.
// If a program calls for the default initialization of an object of
Modified: cfe/trunk/test/Analysis/nullptr.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/
RE: r349201 - Add extension to always default-initialize nullptr_t.
Thanks! I’ll take a look in a bit!
-Erich
From: Richard Smith [mailto:rich...@metafoo.co.uk]
Sent: Friday, December 14, 2018 2:55 PM
To: Keane, Erich
Cc: cfe-commits
Subject: Re: r349201 - Add extension to always default-initialize nullptr_t.
Attached. With the functional part of your change reverted and this applied,
the modified tests still pass except
error: 'note' diagnostics expected but not seen:
File
/usr/local/google/home/richardsmith/llvm-git-1/src/tools/clang/test/Analysis/nullptr.cpp
Line 128: 'p' initialized to a null pointer value
error: 'note' diagnostics seen but not expected:
File
/usr/local/google/home/richardsmith/llvm-git-1/src/tools/clang/test/Analysis/nullptr.cpp
Line 128: 'p' declared without an initial value
... which seems accurate (but perhaps not useful).
On Fri, 14 Dec 2018 at 14:47, Richard Smith
mailto:rich...@metafoo.co.uk>> wrote:
I have a patch I put together a while back as an attempt to fix a different
bug, that creates a new CastKind for this operation (didn't work out there, but
it might do so here). I'll dig it out and mail it to you in case it's a useful
starting point.
On Fri, 14 Dec 2018 at 14:42, Keane, Erich via cfe-commits
mailto:cfe-commits@lists.llvm.org>> wrote:
Alright, no problem. I’ll push a revert in a few minutes and give it another
try. Perhaps I can suppress the creation of a CK_LValueToRValue in the case
where it’s a read from nullptr_t
From: Richard Smith [mailto:rich...@metafoo.co.uk<mailto:rich...@metafoo.co.uk>]
Sent: Friday, December 14, 2018 2:41 PM
To: Keane, Erich mailto:erich.ke...@intel.com>>
Cc: cfe-commits mailto:cfe-commits@lists.llvm.org>>
Subject: Re: r349201 - Add extension to always default-initialize nullptr_t.
Sorry, I was late with my review comment. I think this is the wrong way to
approach this problem. This does not "fix all situations where nullptr_t would
seem uninitialized", and it makes our AST representation lose source fidelity.
On Fri, 14 Dec 2018 at 14:25, Erich Keane via cfe-commits
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: erichkeane
Date: Fri Dec 14 14:22:29 2018
New Revision: 349201
URL: http://llvm.org/viewvc/llvm-project?rev=349201=rev
Log:
Add extension to always default-initialize nullptr_t.
Core issue 1013 suggests that having an uninitialied std::nullptr_t be
UB is a bit foolish, since there is only a single valid value. This DR
reports that DR616 fixes it, which does so by making lvalue-to-rvalue
conversions from nullptr_t be equal to nullptr.
However, just implementing that results in warnings/etc in many places.
In order to fix all situations where nullptr_t would seem uninitialized,
this patch instead (as an otherwise transparent extension) default
initializes uninitialized VarDecls of nullptr_t.
Differential Revision: https://reviews.llvm.org/D53713
Change-Id: I84d72a9290054fa55341e8cbdac43c8e7f25b885
Added:
cfe/trunk/test/SemaCXX/nullptr_t-init.cpp (with props)
Modified:
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/test/Analysis/nullptr.cpp
cfe/trunk/test/SemaCXX/ast-print-crash.cpp
Modified: cfe/trunk/lib/Sema/SemaInit.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=349201=349200=349201=diff
==
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Fri Dec 14 14:22:29 2018
@@ -4881,6 +4881,13 @@ static void TryDefaultInitialization(Sem
return;
}
+ // As an extension, and to fix Core issue 1013, zero initialize nullptr_t.
+ // Since there is only 1 valid value of nullptr_t, we can just use that.
+ if (DestType->isNullPtrType()) {
+Sequence.AddZeroInitializationStep(Entity.getType());
+return;
+ }
+
// - otherwise, no initialization is performed.
// If a program calls for the default initialization of an object of
Modified: cfe/trunk/test/Analysis/nullptr.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/nullptr.cpp?rev=349201=349200=349201=diff
==
--- cfe/trunk/test/Analysis/nullptr.cpp (original)
+++ cfe/trunk/test/Analysis/nullptr.cpp Fri Dec 14 14:22:29 2018
@@ -125,21 +125,16 @@ struct Type {
};
void shouldNotCrash() {
- decltype(nullptr) p; // expected-note{{'p' declared without an initial
value}}
- if (getSymbol()) // expected-note {{Assuming the condition is false}}
- // expected-note@-1{{Taking false branch}}
- // expected-note@-2{{Assuming the condition is false}}
- // expected-note@-3{{Taking false branch}}
- // expected-note@-4{{Assuming the condition is true}}
- // expected-note@-5{{Taking true branch}}
-invokeF(p); // expected-warning{{1st function call argument is an
uninitialized value}}
-
Re: r349201 - Add extension to always default-initialize nullptr_t.
Attached. With the functional part of your change reverted and this
applied, the modified tests still pass except
error: 'note' diagnostics expected but not seen:
File
/usr/local/google/home/richardsmith/llvm-git-1/src/tools/clang/test/Analysis/nullptr.cpp
Line 128: 'p' initialized to a null pointer value
error: 'note' diagnostics seen but not expected:
File
/usr/local/google/home/richardsmith/llvm-git-1/src/tools/clang/test/Analysis/nullptr.cpp
Line 128: 'p' declared without an initial value
... which seems accurate (but perhaps not useful).
On Fri, 14 Dec 2018 at 14:47, Richard Smith wrote:
> I have a patch I put together a while back as an attempt to fix a
> different bug, that creates a new CastKind for this operation (didn't work
> out there, but it might do so here). I'll dig it out and mail it to you in
> case it's a useful starting point.
>
> On Fri, 14 Dec 2018 at 14:42, Keane, Erich via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Alright, no problem. I’ll push a revert in a few minutes and give it
>> another try. Perhaps I can suppress the creation of a CK_LValueToRValue in
>> the case where it’s a read from nullptr_t
>>
>>
>>
>>
>>
>> *From:* Richard Smith [mailto:rich...@metafoo.co.uk]
>> *Sent:* Friday, December 14, 2018 2:41 PM
>> *To:* Keane, Erich
>> *Cc:* cfe-commits
>> *Subject:* Re: r349201 - Add extension to always default-initialize
>> nullptr_t.
>>
>>
>>
>> Sorry, I was late with my review comment. I think this is the wrong way
>> to approach this problem. This does not "fix all situations where nullptr_t
>> would seem uninitialized", and it makes our AST representation lose source
>> fidelity.
>>
>>
>>
>> On Fri, 14 Dec 2018 at 14:25, Erich Keane via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>> Author: erichkeane
>> Date: Fri Dec 14 14:22:29 2018
>> New Revision: 349201
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=349201=rev
>> Log:
>> Add extension to always default-initialize nullptr_t.
>>
>> Core issue 1013 suggests that having an uninitialied std::nullptr_t be
>> UB is a bit foolish, since there is only a single valid value. This DR
>> reports that DR616 fixes it, which does so by making lvalue-to-rvalue
>> conversions from nullptr_t be equal to nullptr.
>>
>> However, just implementing that results in warnings/etc in many places.
>> In order to fix all situations where nullptr_t would seem uninitialized,
>> this patch instead (as an otherwise transparent extension) default
>> initializes uninitialized VarDecls of nullptr_t.
>>
>> Differential Revision: https://reviews.llvm.org/D53713
>>
>> Change-Id: I84d72a9290054fa55341e8cbdac43c8e7f25b885
>>
>> Added:
>> cfe/trunk/test/SemaCXX/nullptr_t-init.cpp (with props)
>> Modified:
>> cfe/trunk/lib/Sema/SemaInit.cpp
>> cfe/trunk/test/Analysis/nullptr.cpp
>> cfe/trunk/test/SemaCXX/ast-print-crash.cpp
>>
>> Modified: cfe/trunk/lib/Sema/SemaInit.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=349201=349200=349201=diff
>>
>> ==
>> --- cfe/trunk/lib/Sema/SemaInit.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaInit.cpp Fri Dec 14 14:22:29 2018
>> @@ -4881,6 +4881,13 @@ static void TryDefaultInitialization(Sem
>> return;
>>}
>>
>> + // As an extension, and to fix Core issue 1013, zero initialize
>> nullptr_t.
>> + // Since there is only 1 valid value of nullptr_t, we can just use
>> that.
>> + if (DestType->isNullPtrType()) {
>> +Sequence.AddZeroInitializationStep(Entity.getType());
>> +return;
>> + }
>> +
>>// - otherwise, no initialization is performed.
>>
>>// If a program calls for the default initialization of an object of
>>
>> Modified: cfe/trunk/test/Analysis/nullptr.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/nullptr.cpp?rev=349201=349200=349201=diff
>>
>> ==
>> --- cfe/trunk/test/Analysis/nullptr.cpp (original)
>> +++ cfe/trunk/test/Analysis/nullptr.cpp Fri Dec 14 14:22:29 2018
>> @@ -125,21 +125,16 @@ struct Type {
>> };
>>
>> void shouldNotCrash() {
>> - decltype(nullptr) p; // expected-note{{'p' declared without an initial
>> value}}
>> - if (getSymbol()) // expected-note {{Assuming the condition is false}}
Re: r349201 - Add extension to always default-initialize nullptr_t.
I have a patch I put together a while back as an attempt to fix a different
bug, that creates a new CastKind for this operation (didn't work out there,
but it might do so here). I'll dig it out and mail it to you in case it's a
useful starting point.
On Fri, 14 Dec 2018 at 14:42, Keane, Erich via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Alright, no problem. I’ll push a revert in a few minutes and give it
> another try. Perhaps I can suppress the creation of a CK_LValueToRValue in
> the case where it’s a read from nullptr_t
>
>
>
>
>
> *From:* Richard Smith [mailto:rich...@metafoo.co.uk]
> *Sent:* Friday, December 14, 2018 2:41 PM
> *To:* Keane, Erich
> *Cc:* cfe-commits
> *Subject:* Re: r349201 - Add extension to always default-initialize
> nullptr_t.
>
>
>
> Sorry, I was late with my review comment. I think this is the wrong way to
> approach this problem. This does not "fix all situations where nullptr_t
> would seem uninitialized", and it makes our AST representation lose source
> fidelity.
>
>
>
> On Fri, 14 Dec 2018 at 14:25, Erich Keane via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> Author: erichkeane
> Date: Fri Dec 14 14:22:29 2018
> New Revision: 349201
>
> URL: http://llvm.org/viewvc/llvm-project?rev=349201=rev
> Log:
> Add extension to always default-initialize nullptr_t.
>
> Core issue 1013 suggests that having an uninitialied std::nullptr_t be
> UB is a bit foolish, since there is only a single valid value. This DR
> reports that DR616 fixes it, which does so by making lvalue-to-rvalue
> conversions from nullptr_t be equal to nullptr.
>
> However, just implementing that results in warnings/etc in many places.
> In order to fix all situations where nullptr_t would seem uninitialized,
> this patch instead (as an otherwise transparent extension) default
> initializes uninitialized VarDecls of nullptr_t.
>
> Differential Revision: https://reviews.llvm.org/D53713
>
> Change-Id: I84d72a9290054fa55341e8cbdac43c8e7f25b885
>
> Added:
> cfe/trunk/test/SemaCXX/nullptr_t-init.cpp (with props)
> Modified:
> cfe/trunk/lib/Sema/SemaInit.cpp
> cfe/trunk/test/Analysis/nullptr.cpp
> cfe/trunk/test/SemaCXX/ast-print-crash.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaInit.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=349201=349200=349201=diff
>
> ==
> --- cfe/trunk/lib/Sema/SemaInit.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaInit.cpp Fri Dec 14 14:22:29 2018
> @@ -4881,6 +4881,13 @@ static void TryDefaultInitialization(Sem
> return;
>}
>
> + // As an extension, and to fix Core issue 1013, zero initialize
> nullptr_t.
> + // Since there is only 1 valid value of nullptr_t, we can just use that.
> + if (DestType->isNullPtrType()) {
> +Sequence.AddZeroInitializationStep(Entity.getType());
> +return;
> + }
> +
>// - otherwise, no initialization is performed.
>
>// If a program calls for the default initialization of an object of
>
> Modified: cfe/trunk/test/Analysis/nullptr.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/nullptr.cpp?rev=349201=349200=349201=diff
>
> ==
> --- cfe/trunk/test/Analysis/nullptr.cpp (original)
> +++ cfe/trunk/test/Analysis/nullptr.cpp Fri Dec 14 14:22:29 2018
> @@ -125,21 +125,16 @@ struct Type {
> };
>
> void shouldNotCrash() {
> - decltype(nullptr) p; // expected-note{{'p' declared without an initial
> value}}
> - if (getSymbol()) // expected-note {{Assuming the condition is false}}
> - // expected-note@-1{{Taking false branch}}
> - // expected-note@-2{{Assuming the condition is false}}
> - // expected-note@-3{{Taking false branch}}
> - // expected-note@-4{{Assuming the condition is true}}
> - // expected-note@-5{{Taking true branch}}
> -invokeF(p); // expected-warning{{1st function call argument is an
> uninitialized value}}
> -// expected-note@-1{{1st function call argument is an
> uninitialized value}}
> + decltype(nullptr) p; // expected-note{{'p' initialized to a null
> pointer value}}
>if (getSymbol()) // expected-note {{Assuming the condition is false}}
> // expected-note@-1{{Taking false branch}}
> // expected-note@-2{{Assuming the condition is true}}
> // expected-note@-3{{Taking true branch}}
> -invokeF(nullptr); // expected-note {{Calling 'invokeF'}}
> -
RE: r349201 - Add extension to always default-initialize nullptr_t.
Reverted in r349206.
From: Richard Smith [mailto:rich...@metafoo.co.uk]
Sent: Friday, December 14, 2018 2:41 PM
To: Keane, Erich
Cc: cfe-commits
Subject: Re: r349201 - Add extension to always default-initialize nullptr_t.
Sorry, I was late with my review comment. I think this is the wrong way to
approach this problem. This does not "fix all situations where nullptr_t would
seem uninitialized", and it makes our AST representation lose source fidelity.
On Fri, 14 Dec 2018 at 14:25, Erich Keane via cfe-commits
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: erichkeane
Date: Fri Dec 14 14:22:29 2018
New Revision: 349201
URL: http://llvm.org/viewvc/llvm-project?rev=349201=rev
Log:
Add extension to always default-initialize nullptr_t.
Core issue 1013 suggests that having an uninitialied std::nullptr_t be
UB is a bit foolish, since there is only a single valid value. This DR
reports that DR616 fixes it, which does so by making lvalue-to-rvalue
conversions from nullptr_t be equal to nullptr.
However, just implementing that results in warnings/etc in many places.
In order to fix all situations where nullptr_t would seem uninitialized,
this patch instead (as an otherwise transparent extension) default
initializes uninitialized VarDecls of nullptr_t.
Differential Revision: https://reviews.llvm.org/D53713
Change-Id: I84d72a9290054fa55341e8cbdac43c8e7f25b885
Added:
cfe/trunk/test/SemaCXX/nullptr_t-init.cpp (with props)
Modified:
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/test/Analysis/nullptr.cpp
cfe/trunk/test/SemaCXX/ast-print-crash.cpp
Modified: cfe/trunk/lib/Sema/SemaInit.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=349201=349200=349201=diff
==
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Fri Dec 14 14:22:29 2018
@@ -4881,6 +4881,13 @@ static void TryDefaultInitialization(Sem
return;
}
+ // As an extension, and to fix Core issue 1013, zero initialize nullptr_t.
+ // Since there is only 1 valid value of nullptr_t, we can just use that.
+ if (DestType->isNullPtrType()) {
+Sequence.AddZeroInitializationStep(Entity.getType());
+return;
+ }
+
// - otherwise, no initialization is performed.
// If a program calls for the default initialization of an object of
Modified: cfe/trunk/test/Analysis/nullptr.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/nullptr.cpp?rev=349201=349200=349201=diff
==
--- cfe/trunk/test/Analysis/nullptr.cpp (original)
+++ cfe/trunk/test/Analysis/nullptr.cpp Fri Dec 14 14:22:29 2018
@@ -125,21 +125,16 @@ struct Type {
};
void shouldNotCrash() {
- decltype(nullptr) p; // expected-note{{'p' declared without an initial
value}}
- if (getSymbol()) // expected-note {{Assuming the condition is false}}
- // expected-note@-1{{Taking false branch}}
- // expected-note@-2{{Assuming the condition is false}}
- // expected-note@-3{{Taking false branch}}
- // expected-note@-4{{Assuming the condition is true}}
- // expected-note@-5{{Taking true branch}}
-invokeF(p); // expected-warning{{1st function call argument is an
uninitialized value}}
-// expected-note@-1{{1st function call argument is an
uninitialized value}}
+ decltype(nullptr) p; // expected-note{{'p' initialized to a null pointer
value}}
if (getSymbol()) // expected-note {{Assuming the condition is false}}
// expected-note@-1{{Taking false branch}}
// expected-note@-2{{Assuming the condition is true}}
// expected-note@-3{{Taking true branch}}
-invokeF(nullptr); // expected-note {{Calling 'invokeF'}}
- // expected-note@-1{{Passing null pointer value via 1st
parameter 'x'}}
+invokeF(p); // expected-note{{Passing null pointer value via 1st parameter
'x'}}
+// expected-note@-1{{Calling 'invokeF'}}
+ if (getSymbol()) // expected-note {{Assuming the condition is false}}
+ // expected-note@-1{{Taking false branch}}
+invokeF(nullptr);
if (getSymbol()) { // expected-note {{Assuming the condition is true}}
// expected-note@-1{{Taking true branch}}
X *xx = Type().x; // expected-note {{Null pointer value stored to field
'x'}}
Modified: cfe/trunk/test/SemaCXX/ast-print-crash.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/ast-print-crash.cpp?rev=349201=349200=349201=diff
==
--- cfe/trunk/test/SemaCXX/ast-print-crash.cpp (original)
+++ cfe/trunk/test/SemaCXX/ast-print-crash.cpp Fri Dec 14 14:22:29 2018
@@ -7,6 +7,6 @@
// CHECK: stru
RE: r349201 - Add extension to always default-initialize nullptr_t.
Alright, no problem. I’ll push a revert in a few minutes and give it another
try. Perhaps I can suppress the creation of a CK_LValueToRValue in the case
where it’s a read from nullptr_t
From: Richard Smith [mailto:rich...@metafoo.co.uk]
Sent: Friday, December 14, 2018 2:41 PM
To: Keane, Erich
Cc: cfe-commits
Subject: Re: r349201 - Add extension to always default-initialize nullptr_t.
Sorry, I was late with my review comment. I think this is the wrong way to
approach this problem. This does not "fix all situations where nullptr_t would
seem uninitialized", and it makes our AST representation lose source fidelity.
On Fri, 14 Dec 2018 at 14:25, Erich Keane via cfe-commits
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: erichkeane
Date: Fri Dec 14 14:22:29 2018
New Revision: 349201
URL: http://llvm.org/viewvc/llvm-project?rev=349201=rev
Log:
Add extension to always default-initialize nullptr_t.
Core issue 1013 suggests that having an uninitialied std::nullptr_t be
UB is a bit foolish, since there is only a single valid value. This DR
reports that DR616 fixes it, which does so by making lvalue-to-rvalue
conversions from nullptr_t be equal to nullptr.
However, just implementing that results in warnings/etc in many places.
In order to fix all situations where nullptr_t would seem uninitialized,
this patch instead (as an otherwise transparent extension) default
initializes uninitialized VarDecls of nullptr_t.
Differential Revision: https://reviews.llvm.org/D53713
Change-Id: I84d72a9290054fa55341e8cbdac43c8e7f25b885
Added:
cfe/trunk/test/SemaCXX/nullptr_t-init.cpp (with props)
Modified:
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/test/Analysis/nullptr.cpp
cfe/trunk/test/SemaCXX/ast-print-crash.cpp
Modified: cfe/trunk/lib/Sema/SemaInit.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=349201=349200=349201=diff
==
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Fri Dec 14 14:22:29 2018
@@ -4881,6 +4881,13 @@ static void TryDefaultInitialization(Sem
return;
}
+ // As an extension, and to fix Core issue 1013, zero initialize nullptr_t.
+ // Since there is only 1 valid value of nullptr_t, we can just use that.
+ if (DestType->isNullPtrType()) {
+Sequence.AddZeroInitializationStep(Entity.getType());
+return;
+ }
+
// - otherwise, no initialization is performed.
// If a program calls for the default initialization of an object of
Modified: cfe/trunk/test/Analysis/nullptr.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/nullptr.cpp?rev=349201=349200=349201=diff
==
--- cfe/trunk/test/Analysis/nullptr.cpp (original)
+++ cfe/trunk/test/Analysis/nullptr.cpp Fri Dec 14 14:22:29 2018
@@ -125,21 +125,16 @@ struct Type {
};
void shouldNotCrash() {
- decltype(nullptr) p; // expected-note{{'p' declared without an initial
value}}
- if (getSymbol()) // expected-note {{Assuming the condition is false}}
- // expected-note@-1{{Taking false branch}}
- // expected-note@-2{{Assuming the condition is false}}
- // expected-note@-3{{Taking false branch}}
- // expected-note@-4{{Assuming the condition is true}}
- // expected-note@-5{{Taking true branch}}
-invokeF(p); // expected-warning{{1st function call argument is an
uninitialized value}}
-// expected-note@-1{{1st function call argument is an
uninitialized value}}
+ decltype(nullptr) p; // expected-note{{'p' initialized to a null pointer
value}}
if (getSymbol()) // expected-note {{Assuming the condition is false}}
// expected-note@-1{{Taking false branch}}
// expected-note@-2{{Assuming the condition is true}}
// expected-note@-3{{Taking true branch}}
-invokeF(nullptr); // expected-note {{Calling 'invokeF'}}
- // expected-note@-1{{Passing null pointer value via 1st
parameter 'x'}}
+invokeF(p); // expected-note{{Passing null pointer value via 1st parameter
'x'}}
+// expected-note@-1{{Calling 'invokeF'}}
+ if (getSymbol()) // expected-note {{Assuming the condition is false}}
+ // expected-note@-1{{Taking false branch}}
+invokeF(nullptr);
if (getSymbol()) { // expected-note {{Assuming the condition is true}}
// expected-note@-1{{Taking true branch}}
X *xx = Type().x; // expected-note {{Null pointer value stored to field
'x'}}
Modified: cfe/trunk/test/SemaCXX/ast-print-crash.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/ast-print-crash.cpp?rev=349201=349200=349201=diff
==
--- cfe
Re: r349201 - Add extension to always default-initialize nullptr_t.
Sorry, I was late with my review comment. I think this is the wrong way to
approach this problem. This does not "fix all situations where nullptr_t
would seem uninitialized", and it makes our AST representation lose source
fidelity.
On Fri, 14 Dec 2018 at 14:25, Erich Keane via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Author: erichkeane
> Date: Fri Dec 14 14:22:29 2018
> New Revision: 349201
>
> URL: http://llvm.org/viewvc/llvm-project?rev=349201=rev
> Log:
> Add extension to always default-initialize nullptr_t.
>
> Core issue 1013 suggests that having an uninitialied std::nullptr_t be
> UB is a bit foolish, since there is only a single valid value. This DR
> reports that DR616 fixes it, which does so by making lvalue-to-rvalue
> conversions from nullptr_t be equal to nullptr.
>
> However, just implementing that results in warnings/etc in many places.
> In order to fix all situations where nullptr_t would seem uninitialized,
> this patch instead (as an otherwise transparent extension) default
> initializes uninitialized VarDecls of nullptr_t.
>
> Differential Revision: https://reviews.llvm.org/D53713
>
> Change-Id: I84d72a9290054fa55341e8cbdac43c8e7f25b885
>
> Added:
> cfe/trunk/test/SemaCXX/nullptr_t-init.cpp (with props)
> Modified:
> cfe/trunk/lib/Sema/SemaInit.cpp
> cfe/trunk/test/Analysis/nullptr.cpp
> cfe/trunk/test/SemaCXX/ast-print-crash.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaInit.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=349201=349200=349201=diff
>
> ==
> --- cfe/trunk/lib/Sema/SemaInit.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaInit.cpp Fri Dec 14 14:22:29 2018
> @@ -4881,6 +4881,13 @@ static void TryDefaultInitialization(Sem
> return;
>}
>
> + // As an extension, and to fix Core issue 1013, zero initialize
> nullptr_t.
> + // Since there is only 1 valid value of nullptr_t, we can just use that.
> + if (DestType->isNullPtrType()) {
> +Sequence.AddZeroInitializationStep(Entity.getType());
> +return;
> + }
> +
>// - otherwise, no initialization is performed.
>
>// If a program calls for the default initialization of an object of
>
> Modified: cfe/trunk/test/Analysis/nullptr.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/nullptr.cpp?rev=349201=349200=349201=diff
>
> ==
> --- cfe/trunk/test/Analysis/nullptr.cpp (original)
> +++ cfe/trunk/test/Analysis/nullptr.cpp Fri Dec 14 14:22:29 2018
> @@ -125,21 +125,16 @@ struct Type {
> };
>
> void shouldNotCrash() {
> - decltype(nullptr) p; // expected-note{{'p' declared without an initial
> value}}
> - if (getSymbol()) // expected-note {{Assuming the condition is false}}
> - // expected-note@-1{{Taking false branch}}
> - // expected-note@-2{{Assuming the condition is false}}
> - // expected-note@-3{{Taking false branch}}
> - // expected-note@-4{{Assuming the condition is true}}
> - // expected-note@-5{{Taking true branch}}
> -invokeF(p); // expected-warning{{1st function call argument is an
> uninitialized value}}
> -// expected-note@-1{{1st function call argument is an
> uninitialized value}}
> + decltype(nullptr) p; // expected-note{{'p' initialized to a null
> pointer value}}
>if (getSymbol()) // expected-note {{Assuming the condition is false}}
> // expected-note@-1{{Taking false branch}}
> // expected-note@-2{{Assuming the condition is true}}
> // expected-note@-3{{Taking true branch}}
> -invokeF(nullptr); // expected-note {{Calling 'invokeF'}}
> - // expected-note@-1{{Passing null pointer value
> via 1st parameter 'x'}}
> +invokeF(p); // expected-note{{Passing null pointer value via 1st
> parameter 'x'}}
> +// expected-note@-1{{Calling 'invokeF'}}
> + if (getSymbol()) // expected-note {{Assuming the condition is false}}
> + // expected-note@-1{{Taking false branch}}
> +invokeF(nullptr);
>if (getSymbol()) { // expected-note {{Assuming the condition is true}}
>// expected-note@-1{{Taking true branch}}
> X *xx = Type().x; // expected-note {{Null pointer value stored to
> field 'x'}}
>
> Modified: cfe/trunk/test/SemaCXX/ast-print-crash.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/ast-print-crash.cpp?rev=349201=349200=349201=diff
>
> ==
> --- cfe/trunk/test/SemaCXX/ast-print-crash.cpp (original)
> +++ cfe/trunk/test/SemaCXX/ast-print-crash.cpp Fri Dec 14 14:22:29 2018
> @@ -7,6 +7,6 @@
>
> // CHECK: struct {
> // CHECK-NEXT: } dont_crash_on_syntax_error;
> -// CHECK-NEXT:
