Add extension to always default-initialize nullptr_t.

[erichkeane]

Summary

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.

[erichkeane]

Woops, apparently a couple of tests fail on this that I somehow missed the 1st time. Looking into it.

[erichkeane]

Update the 2 failing tests. Would love some help looking at the Analysis fix, it seems to no longer have to 'assume' stuff due to the this now being defined behavior.

Additionally, the other test is just an AST change, but seems to be exactly what I'm suggesting as an extension here.

[erichkeane]

Is there any feedback here? Am I just completely incorrect in how I tried to fix this?

[aaron.ballman]

Adding some reviewers for the static analyzer questions raised.

I think this is a reasonable approach, but are there situations where we should diagnose this as an extension? I can't think of one, but I figured the question should still be asked.

[NoQ]

I approve this for Static Analyzer. As long as this is the correct thing to do in the compiler, Static Analyzer should ideally behave similarly to the compiler. Exposing portability bugs that would show up on other compilers is generally less important than behaving similarly to Clang itself.

[rsmith]

I'm concerned that this will hide real bugs by suppressing the bogus warnings in (only) the simple cases. There is no "read a value from memory" operation on type nullptr_t (just like for, say, class types), and any warning that believes there is is incorrect. For example, given

alignas(nullptr_t) char buffer[sizeof(nullptr_t)];
nullptr_t *p = (nullptr_t*)buffer;
nullptr_t q = *p;

... there is no uninitialized use, but this patch will do nothing to make static analysis (etc) aware of that.

I think the real problem here is that our AST representation is wrong. We use CK_LValueToRValue to model a read from memory, but we also use it in examples like the above where there is no read from memory.

test/Analysis/nullptr.cpp

136

This bug is already fixed in trunk.

[erichkeane]

As stated in CFE commits (in response to Richard's comments):

`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

[rsmith]

test/Analysis/nullptr.cpp

136

Never mind, the fix was reverted in r346065.

lib/Sema/SemaInit.cpp

static void TryDefaultInitialization(Sema &S,
   //       constructor for T is called (and the initialization is ill-formed if
   //       T has no accessible default constructor);
   if (DestType->isRecordType() && S.getLangOpts().CPlusPlus) {
     TryConstructorInitialization(S, Entity, Kind, None, DestType,
                                  Entity.getType(), Sequence);
     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
   //   a const-qualified type T, T shall be a class type with a user-provided
   //   default constructor.
   if (DestType.isConstQualified() && S.getLangOpts().CPlusPlus) {
     if (!maybeRecoverWithZeroInitialization(S, Sequence, Entity))
       Sequence.SetFailed(InitializationSequence::FK_DefaultInitOfConst);

test/Analysis/nullptr.cpp

x->f(); // expected-warning{{Called C++ object pointer is null}}
           // expected-note@-1{{Called C++ object pointer is null}}
 }
 
 struct Type {
   decltype(nullptr) x;
 };
 
 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'}}
                       // expected-note@-1{{'xx' initialized to a null pointer value}}
     xx->f(); // expected-warning{{Called C++ object pointer is null}}
             // expected-note@-1{{Called C++ object pointer is null}}
   }
 }

test/SemaCXX/ast-print-crash.cpp

 // RUN: not %clang_cc1 -triple %ms_abi_triple -ast-print %s -std=gnu++11 \
 // RUN:     | FileCheck %s
 
 // The test compiles a file with a syntax error which used to cause a crash with
 // -ast-print. Compilation fails due to the syntax error, but compiler should
 // not crash and print out whatever it manager to parse.
 
 // 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;

test/SemaCXX/nullptr_t-init.cpp

+// 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;
+}