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 <erich.ke...@intel.com> Cc: cfe-commits <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 <rich...@metafoo.co.uk<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 <cfe-commits@lists.llvm.org<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 <erich.ke...@intel.com<mailto:erich.ke...@intel.com>> Cc: cfe-commits <cfe-commits@lists.llvm.org<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 <cfe-commits@lists.llvm.org<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&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@-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&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@lists.llvm.org<mailto:cfe-commits@lists.llvm.org> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
