<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman",serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
span.EmailStyle17
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:#1F497D;}
span.EmailStyle18
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:#1F497D;}
span.EmailStyle19
{mso-style-type:personal-compose;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">Hi Richard-<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">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.<a name="_MailEndCompose"><o:p></o:p></a></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">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.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">Thanks!<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">-Erich<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p> </o:p></span></p>
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><a name="_____replyseparator"></a><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">From:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"> Keane, Erich
<br>
<b>Sent:</b> Friday, December 14, 2018 2:57 PM<br>
<b>To:</b> Richard Smith <richard@metafoo.co.uk><br>
<b>Cc:</b> cfe-commits <cfe-commits@lists.llvm.org><br>
<b>Subject:</b> RE: r349201 - Add extension to always default-initialize nullptr_t.<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">Thanks! I’ll take a look in a bit!<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">-Erich<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">From:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"> Richard Smith [<a href="mailto:richard@metafoo.co.uk">mailto:richard@metafoo.co.uk</a>]
<br>
<b>Sent:</b> Friday, December 14, 2018 2:55 PM<br>
<b>To:</b> Keane, Erich <<a href="mailto:erich.keane@intel.com">erich.keane@intel.com</a>><br>
<b>Cc:</b> cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>><br>
<b>Subject:</b> Re: r349201 - Add extension to always default-initialize nullptr_t.<o:p></o:p></span></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal">Attached. With the functional part of your change reverted and this applied, the modified tests still pass except<o:p></o:p></p>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<div>
<p class="MsoNormal">error: 'note' diagnostics expected but not seen:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> 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<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">error: 'note' diagnostics seen but not expected:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> File /usr/local/google/home/richardsmith/llvm-git-1/src/tools/clang/test/Analysis/nullptr.cpp Line 128: 'p' declared without an initial value<o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">... which seems accurate (but perhaps not useful).<o:p></o:p></p>
</div>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal">On Fri, 14 Dec 2018 at 14:47, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<div>
<p class="MsoNormal">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.<o:p></o:p></p>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal">On Fri, 14 Dec 2018 at 14:42, Keane, Erich via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">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</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"> </span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><a name="m_5031708418523069808_m_2001087958823220"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"> </span></a><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">From:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"> Richard Smith [mailto:<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>]
<br>
<b>Sent:</b> Friday, December 14, 2018 2:41 PM<br>
<b>To:</b> Keane, Erich <<a href="mailto:erich.keane@intel.com" target="_blank">erich.keane@intel.com</a>><br>
<b>Cc:</b> cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>><br>
<b>Subject:</b> Re: r349201 - Add extension to always default-initialize nullptr_t.</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">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.<o:p></o:p></p>
</div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">On Fri, 14 Dec 2018 at 14:25, Erich Keane via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">Author: erichkeane<br>
Date: Fri Dec 14 14:22:29 2018<br>
New Revision: 349201<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=349201&view=rev" target="_blank">
http://llvm.org/viewvc/llvm-project?rev=349201&view=rev</a><br>
Log:<br>
Add extension to always default-initialize nullptr_t.<br>
<br>
Core issue 1013 suggests that having an uninitialied std::nullptr_t be<br>
UB is a bit foolish, since there is only a single valid value. This DR<br>
reports that DR616 fixes it, which does so by making lvalue-to-rvalue<br>
conversions from nullptr_t be equal to nullptr.<br>
<br>
However, just implementing that results in warnings/etc in many places.<br>
In order to fix all situations where nullptr_t would seem uninitialized,<br>
this patch instead (as an otherwise transparent extension) default<br>
initializes uninitialized VarDecls of nullptr_t.<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D53713" target="_blank">
https://reviews.llvm.org/D53713</a><br>
<br>
Change-Id: I84d72a9290054fa55341e8cbdac43c8e7f25b885<br>
<br>
Added:<br>
cfe/trunk/test/SemaCXX/nullptr_t-init.cpp (with props)<br>
Modified:<br>
cfe/trunk/lib/Sema/SemaInit.cpp<br>
cfe/trunk/test/Analysis/nullptr.cpp<br>
cfe/trunk/test/SemaCXX/ast-print-crash.cpp<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaInit.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=349201&r1=349200&r2=349201&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=349201&r1=349200&r2=349201&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaInit.cpp Fri Dec 14 14:22:29 2018<br>
@@ -4881,6 +4881,13 @@ static void TryDefaultInitialization(Sem<br>
return;<br>
}<br>
<br>
+ // As an extension, and to fix Core issue 1013, zero initialize nullptr_t.<br>
+ // Since there is only 1 valid value of nullptr_t, we can just use that.<br>
+ if (DestType->isNullPtrType()) {<br>
+ Sequence.AddZeroInitializationStep(Entity.getType());<br>
+ return;<br>
+ }<br>
+<br>
// - otherwise, no initialization is performed.<br>
<br>
// If a program calls for the default initialization of an object of<br>
<br>
Modified: cfe/trunk/test/Analysis/nullptr.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/nullptr.cpp?rev=349201&r1=349200&r2=349201&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/nullptr.cpp?rev=349201&r1=349200&r2=349201&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/Analysis/nullptr.cpp (original)<br>
+++ cfe/trunk/test/Analysis/nullptr.cpp Fri Dec 14 14:22:29 2018<br>
@@ -125,21 +125,16 @@ struct Type {<br>
};<br>
<br>
void shouldNotCrash() {<br>
- decltype(nullptr) p; // expected-note{{'p' declared without an initial value}}<br>
- if (getSymbol()) // expected-note {{Assuming the condition is false}}<br>
- // expected-note@-1{{Taking false branch}}<br>
- // expected-note@-2{{Assuming the condition is false}}<br>
- // expected-note@-3{{Taking false branch}}<br>
- // expected-note@-4{{Assuming the condition is true}}<br>
- // expected-note@-5{{Taking true branch}}<br>
- invokeF(p); // expected-warning{{1st function call argument is an uninitialized value}}<br>
- // expected-note@-1{{1st function call argument is an uninitialized value}}<br>
+ decltype(nullptr) p; // expected-note{{'p' initialized to a null pointer value}}<br>
if (getSymbol()) // expected-note {{Assuming the condition is false}}<br>
// expected-note@-1{{Taking false branch}}<br>
// expected-note@-2{{Assuming the condition is true}}<br>
// expected-note@-3{{Taking true branch}}<br>
- invokeF(nullptr); // expected-note {{Calling 'invokeF'}}<br>
- // expected-note@-1{{Passing null pointer value via 1st parameter 'x'}}<br>
+ invokeF(p); // expected-note{{Passing null pointer value via 1st parameter 'x'}}<br>
+ // expected-note@-1{{Calling 'invokeF'}}<br>
+ if (getSymbol()) // expected-note {{Assuming the condition is false}}<br>
+ // expected-note@-1{{Taking false branch}}<br>
+ invokeF(nullptr);<br>
if (getSymbol()) { // expected-note {{Assuming the condition is true}}<br>
// expected-note@-1{{Taking true branch}}<br>
X *xx = Type().x; // expected-note {{Null pointer value stored to field 'x'}}<br>
<br>
Modified: cfe/trunk/test/SemaCXX/ast-print-crash.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/ast-print-crash.cpp?rev=349201&r1=349200&r2=349201&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/ast-print-crash.cpp?rev=349201&r1=349200&r2=349201&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/SemaCXX/ast-print-crash.cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/ast-print-crash.cpp Fri Dec 14 14:22:29 2018<br>
@@ -7,6 +7,6 @@<br>
<br>
// CHECK: struct {<br>
// CHECK-NEXT: } dont_crash_on_syntax_error;<br>
-// CHECK-NEXT: decltype(nullptr) p;<br>
+// CHECK-NEXT: decltype(nullptr) p(/*implicit*/(decltype(nullptr))0);<br>
struct {<br>
} dont_crash_on_syntax_error /* missing ; */ decltype(nullptr) p;<br>
<br>
Added: cfe/trunk/test/SemaCXX/nullptr_t-init.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/nullptr_t-init.cpp?rev=349201&view=auto" target="_blank">
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/nullptr_t-init.cpp?rev=349201&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/SemaCXX/nullptr_t-init.cpp (added)<br>
+++ cfe/trunk/test/SemaCXX/nullptr_t-init.cpp Fri Dec 14 14:22:29 2018<br>
@@ -0,0 +1,10 @@<br>
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -ffreestanding -Wuninitialized %s<br>
+// expected-no-diagnostics<br>
+typedef decltype(nullptr) nullptr_t;<br>
+<br>
+// Ensure no 'uninitialized when used here' warnings (Wuninitialized), for <br>
+// nullptr_t always-initialized extension.<br>
+nullptr_t default_init() {<br>
+ nullptr_t a;<br>
+ return a;<br>
+}<br>
<br>
Propchange: cfe/trunk/test/SemaCXX/nullptr_t-init.cpp<br>
------------------------------------------------------------------------------<br>
svn:eol-style = native<br>
<br>
Propchange: cfe/trunk/test/SemaCXX/nullptr_t-init.cpp<br>
------------------------------------------------------------------------------<br>
svn:keywords = "Author Date Id Rev URL"<br>
<br>
Propchange: cfe/trunk/test/SemaCXX/nullptr_t-init.cpp<br>
------------------------------------------------------------------------------<br>
svn:mime-type = text/plain<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><o:p></o:p></p>
</blockquote>
</div>
</div>
</div>
<p class="MsoNormal">_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><o:p></o:p></p>
</blockquote>
</div>
</blockquote>
</div>
</div>
</body>
</html>