r349201 - Add extension to always default-initialize nullptr_t.
Keane, Erich via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 17 06:14:32 PST 2019
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 <richard at metafoo.co.uk>
Cc: cfe-commits <cfe-commits at lists.llvm.org>
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:richard at metafoo.co.uk]
Sent: Friday, December 14, 2018 2:55 PM
To: Keane, Erich <erich.keane at intel.com<mailto:erich.keane at intel.com>>
Cc: cfe-commits <cfe-commits at lists.llvm.org<mailto:cfe-commits at 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 <richard at metafoo.co.uk<mailto:richard at 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 <cfe-commits at lists.llvm.org<mailto:cfe-commits at 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:richard at metafoo.co.uk<mailto:richard at metafoo.co.uk>]
Sent: Friday, December 14, 2018 2:41 PM
To: Keane, Erich <erich.keane at intel.com<mailto:erich.keane at intel.com>>
Cc: cfe-commits <cfe-commits at lists.llvm.org<mailto:cfe-commits at 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 <cfe-commits at lists.llvm.org<mailto:cfe-commits at 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&view=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&r1=349200&r2=349201&view=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&r1=349200&r2=349201&view=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 at -1{{Taking false branch}}
- // expected-note at -2{{Assuming the condition is false}}
- // expected-note at -3{{Taking false branch}}
- // expected-note at -4{{Assuming the condition is true}}
- // expected-note at -5{{Taking true branch}}
- invokeF(p); // expected-warning{{1st function call argument is an uninitialized value}}
- // expected-note at -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 at -1{{Taking false branch}}
// expected-note at -2{{Assuming the condition is true}}
// expected-note at -3{{Taking true branch}}
- invokeF(nullptr); // expected-note {{Calling 'invokeF'}}
- // expected-note at -1{{Passing null pointer value via 1st parameter 'x'}}
+ invokeF(p); // expected-note{{Passing null pointer value via 1st parameter 'x'}}
+ // expected-note at -1{{Calling 'invokeF'}}
+ if (getSymbol()) // expected-note {{Assuming the condition is false}}
+ // expected-note at -1{{Taking false branch}}
+ invokeF(nullptr);
if (getSymbol()) { // expected-note {{Assuming the condition is true}}
// expected-note at -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&r1=349200&r2=349201&view=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: decltype(nullptr) p;
+// CHECK-NEXT: decltype(nullptr) p(/*implicit*/(decltype(nullptr))0);
struct {
} dont_crash_on_syntax_error /* missing ; */ decltype(nullptr) p;
Added: cfe/trunk/test/SemaCXX/nullptr_t-init.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/nullptr_t-init.cpp?rev=349201&view=auto
==============================================================================
--- cfe/trunk/test/SemaCXX/nullptr_t-init.cpp (added)
+++ cfe/trunk/test/SemaCXX/nullptr_t-init.cpp Fri Dec 14 14:22:29 2018
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -ffreestanding -Wuninitialized %s
+// expected-no-diagnostics
+typedef decltype(nullptr) nullptr_t;
+
+// Ensure no 'uninitialized when used here' warnings (Wuninitialized), for
+// nullptr_t always-initialized extension.
+nullptr_t default_init() {
+ nullptr_t a;
+ return a;
+}
Propchange: cfe/trunk/test/SemaCXX/nullptr_t-init.cpp
------------------------------------------------------------------------------
svn:eol-style = native
Propchange: cfe/trunk/test/SemaCXX/nullptr_t-init.cpp
------------------------------------------------------------------------------
svn:keywords = "Author Date Id Rev URL"
Propchange: cfe/trunk/test/SemaCXX/nullptr_t-init.cpp
------------------------------------------------------------------------------
svn:mime-type = text/plain
_______________________________________________
cfe-commits mailing list
cfe-commits at lists.llvm.org<mailto:cfe-commits at lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
_______________________________________________
cfe-commits mailing list
cfe-commits at lists.llvm.org<mailto:cfe-commits at lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190117/69583439/attachment-0001.html>
More information about the cfe-commits
mailing list