[C++] PR 83921 ("[7/8 Regression] GCC rejects constexpr initialization of empty aggregate")

Hi,

I'm finishing testing on x86_64-linux the below - which anyway seems 
very unlikely to cause regressions because we aren't really stress 
testing the relevant checks in potential_constant_expression_1 much, if 
at all (surely stmtexpr19.C tests static).

Anyway, the issue is the following. In 239268 aka "Implement C++17 
constexpr lambda" Jason added some checks to 
potential_constant_expression_1 covering static, thread_local and 
uninitialized var declaration in constexpr function context. Then 
extended to constexpr context more generally in 249382 aka "constexpr 
and static var in statement-expression", with ext/stmtexpr19.C covering 
the static case. Now, it looks like the check for uninitialized vars in 
constexpr functions context is more correctly carried out by 
check_for_uninitialized_const_var instead, because the simple check in 
potential_constant_expression_1 as-is causes the regression pointed out 
by this bug. Thus the fix below which just restricts the check in 
potential_constant_expression_1, and the testcases, one for this bug 
proper, plus one, very similar to stmtexpr19.C, double checking that we 
are still diagnosing in the statement-expression context. I also 
verified under the debugger how for constexpr-83921.C we are actually 
running check_for_uninitialized_const_var on 'f' - which obviously passes.

Thanks, Paolo.

////////////////////////
/cp
2018-01-18  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/83921
	* constexpr.c (potential_constant_expression_1): Do not handle
	uninitialized vars in constexpr function here.

/testsuite
2018-01-18  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/83921
	* g++.dg/cpp1y/constexpr-83921.C: New.
	* g++.dg/ext/stmtexpr20.C: Likewise.

On Thu, Jan 18, 2018 at 5:53 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi,
>
> I'm finishing testing on x86_64-linux the below - which anyway seems very
> unlikely to cause regressions because we aren't really stress testing the
> relevant checks in potential_constant_expression_1 much, if at all (surely
> stmtexpr19.C tests static).
>
> Anyway, the issue is the following. In 239268 aka "Implement C++17 constexpr
> lambda" Jason added some checks to potential_constant_expression_1 covering
> static, thread_local and uninitialized var declaration in constexpr function
> context. Then extended to constexpr context more generally in 249382 aka
> "constexpr and static var in statement-expression", with ext/stmtexpr19.C
> covering the static case. Now, it looks like the check for uninitialized
> vars in constexpr functions context is more correctly carried out by
> check_for_uninitialized_const_var instead, because the simple check in
> potential_constant_expression_1 as-is causes the regression pointed out by
> this bug. Thus the fix below which just restricts the check in
> potential_constant_expression_1, and the testcases, one for this bug proper,
> plus one, very similar to stmtexpr19.C, double checking that we are still
> diagnosing in the statement-expression context. I also verified under the
> debugger how for constexpr-83921.C we are actually running
> check_for_uninitialized_const_var on 'f' - which obviously passes.

Seems like this code should either be removed because it's covered by
check_for_uninitialized_const_var, or it should be fixed to check
default_init_uninitialized_part.

So this code is still needed for stmtexpr19.C?  Why doesn't
check_for_... handle that case?

Jason

Hi,

On 19/01/2018 23:28, Jason Merrill wrote:
> On Thu, Jan 18, 2018 at 5:53 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
>> Hi,
>>
>> I'm finishing testing on x86_64-linux the below - which anyway seems very
>> unlikely to cause regressions because we aren't really stress testing the
>> relevant checks in potential_constant_expression_1 much, if at all (surely
>> stmtexpr19.C tests static).
>>
>> Anyway, the issue is the following. In 239268 aka "Implement C++17 constexpr
>> lambda" Jason added some checks to potential_constant_expression_1 covering
>> static, thread_local and uninitialized var declaration in constexpr function
>> context. Then extended to constexpr context more generally in 249382 aka
>> "constexpr and static var in statement-expression", with ext/stmtexpr19.C
>> covering the static case. Now, it looks like the check for uninitialized
>> vars in constexpr functions context is more correctly carried out by
>> check_for_uninitialized_const_var instead, because the simple check in
>> potential_constant_expression_1 as-is causes the regression pointed out by
>> this bug. Thus the fix below which just restricts the check in
>> potential_constant_expression_1, and the testcases, one for this bug proper,
>> plus one, very similar to stmtexpr19.C, double checking that we are still
>> diagnosing in the statement-expression context. I also verified under the
>> debugger how for constexpr-83921.C we are actually running
>> check_for_uninitialized_const_var on 'f' - which obviously passes.
> Seems like this code should either be removed because it's covered by
> check_for_uninitialized_const_var, or it should be fixed to check
> default_init_uninitialized_part.
>
> So this code is still needed for stmtexpr19.C?  Why doesn't
> check_for_... handle that case?
Well, stmtexpr19.C exists to exercise 'static', which definitely 
check_for_uninitialized_const_var doesn't cover. The same would be true 
for a testcase exercising 'thread_local'. As regards my new 
stmtexpr20.C, check_for_uninitialized_const_var can't diagnose anything 
for a very simple reason:

   if (VAR_P (decl)
       && TREE_CODE (type) != REFERENCE_TYPE
       && (CP_TYPE_CONST_P (type) || var_in_constexpr_fn (decl))
       && !DECL_INITIAL (decl))

thus doesn't do anything for non-const vars outside a constexpr 
function. On the other hand, !DECL_NONTRIVIALLY_INITIALIZED_P is true 
for such vars in constexpr context which actually belong to a statement 
expression (always?). Therefore It seems to me that a way to more 
cleanly solve the bug would be moving something like || 
!DECL_NONTRIVIALLY_INITIALIZED_P to the the above check in 
check_for_uninitialized_const_var, and therefore remove completely the 
uninitialized case from potential_constant_expression_1, but I'm note 
sure it would always work because at the moment isn't entirely clear to 
me how DECL_NONTRIVIALLY_INITIALIZED_P reflects the statement-expression 
context bit (that explain the conservative slant of my patch) What do 
you think?

Paolo.

Hi again,

On 19/01/2018 23:55, Paolo Carlini wrote:
> ...Therefore It seems to me that a way to more cleanly solve the bug 
> would be moving something like || !DECL_NONTRIVIALLY_INITIALIZED_P to 
> the the above check in check_for_uninitialized_const_var, and 
> therefore remove completely the uninitialized case from 
> potential_constant_expression_1, ...
Of course this doesn't work. check_for_uninitialized_const would really 
need to know that the VAR_DECL appears in a statement expression which 
is initializing a constexpr variable, nothing to do with 
DECL_NONTRIVIALLY_INITIALIZED_P. I'll give the issue more thought over 
the we, but removing completely the check from 
potential_constant_expression_1 seems very tough to me, assuming of 
course we really care about diagnosing the uninitialized inner in 
stmtexpr20.C, which is my invention, isn't in the testsuite yet ;)

Paolo.

Hi,

On 20/01/2018 02:59, Paolo Carlini wrote:
> Hi again,
>
> On 19/01/2018 23:55, Paolo Carlini wrote:
>> ...Therefore It seems to me that a way to more cleanly solve the bug 
>> would be moving something like || !DECL_NONTRIVIALLY_INITIALIZED_P to 
>> the the above check in check_for_uninitialized_const_var, and 
>> therefore remove completely the uninitialized case from 
>> potential_constant_expression_1, ...
> Of course this doesn't work. check_for_uninitialized_const would 
> really need to know that the VAR_DECL appears in a statement 
> expression which is initializing a constexpr variable, nothing to do 
> with DECL_NONTRIVIALLY_INITIALIZED_P. I'll give the issue more thought 
> over the we, but removing completely the check from 
> potential_constant_expression_1 seems very tough to me, assuming of 
> course we really care about diagnosing the uninitialized inner in 
> stmtexpr20.C, which is my invention, isn't in the testsuite yet ;)
Thus the below, carefully tested over the we, would be a change 
completely removing the problematic error_at call, plus some testcases 
checking that we would still do the right thing in a few cases (bug 
submitter added constexpr-83921-2.C). The updated stmtexpr20.C shows 
that we would reject anyway a statement expression using an 
uninitialized inner, simply because the whole initializer would still be 
flagged as non const. ICC does the same. CLANG diagnoses the 
uninitialized inner but then actually accepts something using 'static 
const int inner = 1' whereas we don't anyway, we (and ICC) don't accept 
anything similar to stmtexpr20.C. So, I would say this kind of change 
may actually make sense...

Thanks,
Paolo.

//////////////////////.
Index: cp/constexpr.c
===================================================================
--- cp/constexpr.c	(revision 256939)
+++ cp/constexpr.c	(working copy)
@@ -5707,13 +5707,6 @@ potential_constant_expression_1 (tree t, bool want
 			  "%<thread_local%> in %<constexpr%> context", tmp);
 	      return false;
 	    }
-	  else if (!DECL_NONTRIVIALLY_INITIALIZED_P (tmp))
-	    {
-	      if (flags & tf_error)
-		error_at (DECL_SOURCE_LOCATION (tmp), "uninitialized "
-			  "variable %qD in %<constexpr%> context", tmp);
-	      return false;
-	    }
 	}
       return RECUR (tmp, want_rval);
 
Index: testsuite/g++.dg/cpp1y/constexpr-83921-1.C
===================================================================
--- testsuite/g++.dg/cpp1y/constexpr-83921-1.C	(nonexistent)
+++ testsuite/g++.dg/cpp1y/constexpr-83921-1.C	(working copy)
@@ -0,0 +1,5 @@
+// PR c++/83921
+// { dg-do compile { target c++14 } }
+
+struct Foo { };
+constexpr void test() { Foo f; }
Index: testsuite/g++.dg/cpp1y/constexpr-83921-2.C
===================================================================
--- testsuite/g++.dg/cpp1y/constexpr-83921-2.C	(nonexistent)
+++ testsuite/g++.dg/cpp1y/constexpr-83921-2.C	(working copy)
@@ -0,0 +1,5 @@
+// PR c++/83921
+// { dg-do compile { target c++14 } }
+
+struct Foo { Foo() = default; };
+constexpr void test() { Foo f; }
Index: testsuite/g++.dg/cpp1y/constexpr-83921-3.C
===================================================================
--- testsuite/g++.dg/cpp1y/constexpr-83921-3.C	(nonexistent)
+++ testsuite/g++.dg/cpp1y/constexpr-83921-3.C	(working copy)
@@ -0,0 +1,5 @@
+// PR c++/83921
+// { dg-do compile { target c++14 } }
+
+struct Foo { int m; };
+constexpr void test() { Foo f; }  // { dg-error "uninitialized" }
Index: testsuite/g++.dg/ext/stmtexpr20.C
===================================================================
--- testsuite/g++.dg/ext/stmtexpr20.C	(nonexistent)
+++ testsuite/g++.dg/ext/stmtexpr20.C	(working copy)
@@ -0,0 +1,13 @@
+// PR c++/83921
+// { dg-options "" }
+// { dg-do compile { target c++11 } }
+
+struct test { const int *addr; };
+
+const test* setup()
+{
+  static constexpr test atest =
+    { ({ int inner; &inner; }) };  // { dg-error "not a constant" }
+
+  return &atest;
+}

On Mon, Jan 22, 2018 at 5:08 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> On 20/01/2018 02:59, Paolo Carlini wrote:
>>
>> Hi again,
>>
>> On 19/01/2018 23:55, Paolo Carlini wrote:
>>>
>>> ...Therefore It seems to me that a way to more cleanly solve the bug
>>> would be moving something like || !DECL_NONTRIVIALLY_INITIALIZED_P to the
>>> the above check in check_for_uninitialized_const_var, and therefore remove
>>> completely the uninitialized case from potential_constant_expression_1, ...
>>
>> Of course this doesn't work. check_for_uninitialized_const would really
>> need to know that the VAR_DECL appears in a statement expression which is
>> initializing a constexpr variable, nothing to do with
>> DECL_NONTRIVIALLY_INITIALIZED_P.

Ah, that makes sense.  And the static/thread_local checks are
similarly duplicates of checks in start_decl.  It might be nice to
factor those three checks out into another function called in both
places, but perhaps that's not necessary given how small they are.

> Thus the below, carefully tested over the we, would be a change completely
> removing the problematic error_at call, plus some testcases checking that we
> would still do the right thing in a few cases (bug submitter added
> constexpr-83921-2.C). The updated stmtexpr20.C shows that we would reject
> anyway a statement expression using an uninitialized inner, simply because
> the whole initializer would still be flagged as non const.

Do we still diagnose it without the use of the variable?

If not, how about adjusting check_for_uninitialized_const_var to
support calling it from potential_constant_expression_1?  I suppose
we'd need to add a couple of parameters, one complain parm and another
to indicate that the variable is in a constexpr context, and then
return something.

Jason

Hi,

On 22/01/2018 16:52, Jason Merrill wrote:
>> Thus the below, carefully tested over the we, would be a change completely
>> removing the problematic error_at call, plus some testcases checking that we
>> would still do the right thing in a few cases (bug submitter added
>> constexpr-83921-2.C). The updated stmtexpr20.C shows that we would reject
>> anyway a statement expression using an uninitialized inner, simply because
>> the whole initializer would still be flagged as non const.
> Do we still diagnose it without the use of the variable?
Ah, I missed that case. The updated stmtexpr20.C and the additional 
stmtexpr21.C reflect that.
> If not, how about adjusting check_for_uninitialized_const_var to
> support calling it from potential_constant_expression_1?  I suppose
> we'd need to add a couple of parameters, one complain parm and another
> to indicate that the variable is in a constexpr context, and then
> return something.
Ok. The below passes the C++ testsuite and I'm finishing testing it. 
Therefore, as you already hinted to, we can now say that what was 
*really* missing from potential_constant_expression_1 was the use of 
default_init_uninitialized_part, which does all the non-trivial work 
besides the later !DECL_NONTRIVIALLY_INITIALIZED_P check. 
check_for_uninitialized_const_var also provides the informs, which were 
completely missing.

Thanks!
Paolo.

////////////////////
Index: cp/constexpr.c
===================================================================
--- cp/constexpr.c	(revision 256961)
+++ cp/constexpr.c	(working copy)
@@ -5707,13 +5707,9 @@ potential_constant_expression_1 (tree t, bool want
 			  "%<thread_local%> in %<constexpr%> context", tmp);
 	      return false;
 	    }
-	  else if (!DECL_NONTRIVIALLY_INITIALIZED_P (tmp))
-	    {
-	      if (flags & tf_error)
-		error_at (DECL_SOURCE_LOCATION (tmp), "uninitialized "
-			  "variable %qD in %<constexpr%> context", tmp);
-	      return false;
-	    }
+	  else if (!check_for_uninitialized_const_var
+		   (tmp, /*constexpr_context_p=*/true, flags))
+	    return false;
 	}
       return RECUR (tmp, want_rval);
 
Index: cp/cp-tree.h
===================================================================
--- cp/cp-tree.h	(revision 256961)
+++ cp/cp-tree.h	(working copy)
@@ -6221,6 +6221,7 @@ extern tree finish_case_label			(location_t, tree,
 extern tree cxx_maybe_build_cleanup		(tree, tsubst_flags_t);
 extern bool check_array_designated_initializer  (constructor_elt *,
 						 unsigned HOST_WIDE_INT);
+extern bool check_for_uninitialized_const_var   (tree, bool, tsubst_flags_t);
 
 /* in decl2.c */
 extern void record_mangling			(tree, bool);
Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 256961)
+++ cp/decl.c	(working copy)
@@ -72,7 +72,6 @@ static int check_static_variable_definition (tree,
 static void record_unknown_type (tree, const char *);
 static tree builtin_function_1 (tree, tree, bool);
 static int member_function_or_else (tree, tree, enum overload_flags);
-static void check_for_uninitialized_const_var (tree);
 static tree local_variable_p_walkfn (tree *, int *, void *);
 static const char *tag_name (enum tag_types);
 static tree lookup_and_check_tag (enum tag_types, tree, tag_scope, bool);
@@ -5543,10 +5542,14 @@ maybe_commonize_var (tree decl)
     }
 }
 
-/* Issue an error message if DECL is an uninitialized const variable.  */
+/* Issue an error message if DECL is an uninitialized const variable.
+   CONSTEXPR_CONTEXT_P is true when the function is called in a constexpr
+   context from potential_constant_expression.  Returns true if all is well,
+   false otherwise.  */
 
-static void
-check_for_uninitialized_const_var (tree decl)
+bool
+check_for_uninitialized_const_var (tree decl, bool constexpr_context_p,
+				   tsubst_flags_t complain)
 {
   tree type = strip_array_types (TREE_TYPE (decl));
 
@@ -5555,26 +5558,36 @@ maybe_commonize_var (tree decl)
      7.1.6 */
   if (VAR_P (decl)
       && TREE_CODE (type) != REFERENCE_TYPE
-      && (CP_TYPE_CONST_P (type) || var_in_constexpr_fn (decl))
+      && (CP_TYPE_CONST_P (type)
+	  || (!constexpr_context_p && var_in_constexpr_fn (decl))
+	  || (constexpr_context_p && !DECL_NONTRIVIALLY_INITIALIZED_P (decl)))
       && !DECL_INITIAL (decl))
     {
       tree field = default_init_uninitialized_part (type);
       if (!field)
-	return;
+	return true;
 
       if (CP_TYPE_CONST_P (type))
-	permerror (DECL_SOURCE_LOCATION (decl),
-		   "uninitialized const %qD", decl);
-      else
 	{
-	  if (!is_instantiation_of_constexpr (current_function_decl))
+	  if (complain & tf_error)
+	    permerror (DECL_SOURCE_LOCATION (decl),
+		       "uninitialized const %qD", decl);
+	}
+      else if (!constexpr_context_p)
+	{
+	  if (!is_instantiation_of_constexpr (current_function_decl)
+	      && (complain & tf_error))
 	    error_at (DECL_SOURCE_LOCATION (decl),
 		      "uninitialized variable %qD in %<constexpr%> function",
 		      decl);
 	  cp_function_chain->invalid_constexpr = true;
 	}
+      else if (complain & tf_error)
+	error_at (DECL_SOURCE_LOCATION (decl),
+		  "uninitialized variable %qD in %<constexpr%> context",
+		  decl);
 
-      if (CLASS_TYPE_P (type))
+      if (CLASS_TYPE_P (type) && (complain & tf_error))
 	{
 	  tree defaulted_ctor;
 
@@ -5589,7 +5602,11 @@ maybe_commonize_var (tree decl)
 		  "and the implicitly-defined constructor does not "
 		  "initialize %q#D", field);
 	}
+
+      return false;
     }
+
+  return true;
 }
 
 /* Structure holding the current initializer being processed by reshape_init.
@@ -6250,7 +6267,8 @@ check_initializer (tree decl, tree init, int flags
 	  flags |= LOOKUP_ALREADY_DIGESTED;
 	}
       else if (!init)
-	check_for_uninitialized_const_var (decl);
+	check_for_uninitialized_const_var (decl, /*constexpr_context_p =*/false,
+					   tf_warning_or_error);
       /* Do not reshape constructors of vectors (they don't need to be
 	 reshaped.  */
       else if (BRACE_ENCLOSED_INITIALIZER_P (init))
@@ -6377,7 +6395,8 @@ check_initializer (tree decl, tree init, int flags
 	diagnose_uninitialized_cst_or_ref_member (core_type, /*using_new=*/false,
 						  /*complain=*/true);
 
-      check_for_uninitialized_const_var (decl);
+      check_for_uninitialized_const_var (decl, /*constexpr_context_p=*/false,
+					 tf_warning_or_error);
     }
 
   if (init && init != error_mark_node)
Index: testsuite/g++.dg/cpp1y/constexpr-83921-1.C
===================================================================
--- testsuite/g++.dg/cpp1y/constexpr-83921-1.C	(nonexistent)
+++ testsuite/g++.dg/cpp1y/constexpr-83921-1.C	(working copy)
@@ -0,0 +1,5 @@
+// PR c++/83921
+// { dg-do compile { target c++14 } }
+
+struct Foo { };
+constexpr void test() { Foo f; }
Index: testsuite/g++.dg/cpp1y/constexpr-83921-2.C
===================================================================
--- testsuite/g++.dg/cpp1y/constexpr-83921-2.C	(nonexistent)
+++ testsuite/g++.dg/cpp1y/constexpr-83921-2.C	(working copy)
@@ -0,0 +1,5 @@
+// PR c++/83921
+// { dg-do compile { target c++14 } }
+
+struct Foo { Foo() = default; };
+constexpr void test() { Foo f; }
Index: testsuite/g++.dg/cpp1y/constexpr-83921-3.C
===================================================================
--- testsuite/g++.dg/cpp1y/constexpr-83921-3.C	(nonexistent)
+++ testsuite/g++.dg/cpp1y/constexpr-83921-3.C	(working copy)
@@ -0,0 +1,5 @@
+// PR c++/83921
+// { dg-do compile { target c++14 } }
+
+struct Foo { int m; };
+constexpr void test() { Foo f; }  // { dg-error "uninitialized" }
Index: testsuite/g++.dg/ext/stmtexpr20.C
===================================================================
--- testsuite/g++.dg/ext/stmtexpr20.C	(nonexistent)
+++ testsuite/g++.dg/ext/stmtexpr20.C	(working copy)
@@ -0,0 +1,13 @@
+// PR c++/83921
+// { dg-options "" }
+// { dg-do compile { target c++11 } }
+
+struct test { const int *addr; };
+
+const test* setup()
+{
+  static constexpr test atest =
+    { ({ int inner = 1; (const int*)(0); }) };
+
+  return &atest;
+}
Index: testsuite/g++.dg/ext/stmtexpr21.C
===================================================================
--- testsuite/g++.dg/ext/stmtexpr21.C	(nonexistent)
+++ testsuite/g++.dg/ext/stmtexpr21.C	(working copy)
@@ -0,0 +1,13 @@
+// PR c++/83921
+// { dg-options "" }
+// { dg-do compile { target c++11 } }
+
+struct test { const int *addr; };
+
+const test* setup()
+{
+  static constexpr test atest =
+    { ({ int inner; (const int*)(0); }) };  // { dg-error "uninitialized" }
+
+  return &atest;
+}

Hi again,

On 22/01/2018 22:50, Paolo Carlini wrote:
> Ok. The below passes the C++ testsuite and I'm finishing testing it. 
> Therefore, as you already hinted to, we can now say that what was 
> *really* missing from potential_constant_expression_1 was the use of 
> default_init_uninitialized_part, which does all the non-trivial work 
> besides the later !DECL_NONTRIVIALLY_INITIALIZED_P check. 
> check_for_uninitialized_const_var also provides the informs, which 
> were completely missing.
Grrr. Testing the library revealed immediately the failure of 
18_support/byte/ops.cc, because in constexpr_context_p == true, thus 
from potential_constant_expression_1, the case CP_TYPE_CONST_P triggers. 
I guess we really want to keep the existing constexpr_context_p == false 
cases separate. I'm therefore restarting testing with the below.

Paolo.

///////////////
Index: cp/constexpr.c
===================================================================
--- cp/constexpr.c	(revision 256961)
+++ cp/constexpr.c	(working copy)
@@ -5707,13 +5707,9 @@ potential_constant_expression_1 (tree t, bool want
 			  "%<thread_local%> in %<constexpr%> context", tmp);
 	      return false;
 	    }
-	  else if (!DECL_NONTRIVIALLY_INITIALIZED_P (tmp))
-	    {
-	      if (flags & tf_error)
-		error_at (DECL_SOURCE_LOCATION (tmp), "uninitialized "
-			  "variable %qD in %<constexpr%> context", tmp);
-	      return false;
-	    }
+	  else if (!check_for_uninitialized_const_var
+		   (tmp, /*constexpr_context_p=*/true, flags))
+	    return false;
 	}
       return RECUR (tmp, want_rval);
 
Index: cp/cp-tree.h
===================================================================
--- cp/cp-tree.h	(revision 256961)
+++ cp/cp-tree.h	(working copy)
@@ -6221,6 +6221,7 @@ extern tree finish_case_label			(location_t, tree,
 extern tree cxx_maybe_build_cleanup		(tree, tsubst_flags_t);
 extern bool check_array_designated_initializer  (constructor_elt *,
 						 unsigned HOST_WIDE_INT);
+extern bool check_for_uninitialized_const_var   (tree, bool, tsubst_flags_t);
 
 /* in decl2.c */
 extern void record_mangling			(tree, bool);
Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 256961)
+++ cp/decl.c	(working copy)
@@ -72,7 +72,6 @@ static int check_static_variable_definition (tree,
 static void record_unknown_type (tree, const char *);
 static tree builtin_function_1 (tree, tree, bool);
 static int member_function_or_else (tree, tree, enum overload_flags);
-static void check_for_uninitialized_const_var (tree);
 static tree local_variable_p_walkfn (tree *, int *, void *);
 static const char *tag_name (enum tag_types);
 static tree lookup_and_check_tag (enum tag_types, tree, tag_scope, bool);
@@ -5543,10 +5542,14 @@ maybe_commonize_var (tree decl)
     }
 }
 
-/* Issue an error message if DECL is an uninitialized const variable.  */
+/* Issue an error message if DECL is an uninitialized const variable.
+   CONSTEXPR_CONTEXT_P is true when the function is called in a constexpr
+   context from potential_constant_expression.  Returns true if all is well,
+   false otherwise.  */
 
-static void
-check_for_uninitialized_const_var (tree decl)
+bool
+check_for_uninitialized_const_var (tree decl, bool constexpr_context_p,
+				   tsubst_flags_t complain)
 {
   tree type = strip_array_types (TREE_TYPE (decl));
 
@@ -5555,26 +5558,39 @@ maybe_commonize_var (tree decl)
      7.1.6 */
   if (VAR_P (decl)
       && TREE_CODE (type) != REFERENCE_TYPE
-      && (CP_TYPE_CONST_P (type) || var_in_constexpr_fn (decl))
+      && ((!constexpr_context_p
+	   && (CP_TYPE_CONST_P (type) || var_in_constexpr_fn (decl)))
+	  || (constexpr_context_p && !DECL_NONTRIVIALLY_INITIALIZED_P (decl)))
       && !DECL_INITIAL (decl))
     {
       tree field = default_init_uninitialized_part (type);
       if (!field)
-	return;
+	return true;
 
-      if (CP_TYPE_CONST_P (type))
-	permerror (DECL_SOURCE_LOCATION (decl),
-		   "uninitialized const %qD", decl);
-      else
+      if (!constexpr_context_p)
 	{
-	  if (!is_instantiation_of_constexpr (current_function_decl))
-	    error_at (DECL_SOURCE_LOCATION (decl),
-		      "uninitialized variable %qD in %<constexpr%> function",
-		      decl);
-	  cp_function_chain->invalid_constexpr = true;
+	  if (CP_TYPE_CONST_P (type))
+	    {
+	      if (complain & tf_error)
+		permerror (DECL_SOURCE_LOCATION (decl),
+			   "uninitialized const %qD", decl);
+	    }
+	  else
+	    {
+	      if (!is_instantiation_of_constexpr (current_function_decl)
+		  && (complain & tf_error))
+		error_at (DECL_SOURCE_LOCATION (decl),
+			  "uninitialized variable %qD in %<constexpr%> function",
+			  decl);
+	      cp_function_chain->invalid_constexpr = true;
+	    }
 	}
+      else if (complain & tf_error)
+	error_at (DECL_SOURCE_LOCATION (decl),
+		  "uninitialized variable %qD in %<constexpr%> context",
+		  decl);
 
-      if (CLASS_TYPE_P (type))
+      if (CLASS_TYPE_P (type) && (complain & tf_error))
 	{
 	  tree defaulted_ctor;
 
@@ -5589,7 +5605,11 @@ maybe_commonize_var (tree decl)
 		  "and the implicitly-defined constructor does not "
 		  "initialize %q#D", field);
 	}
+
+      return false;
     }
+
+  return true;
 }
 
 /* Structure holding the current initializer being processed by reshape_init.
@@ -6250,7 +6270,8 @@ check_initializer (tree decl, tree init, int flags
 	  flags |= LOOKUP_ALREADY_DIGESTED;
 	}
       else if (!init)
-	check_for_uninitialized_const_var (decl);
+	check_for_uninitialized_const_var (decl, /*constexpr_context_p =*/false,
+					   tf_warning_or_error);
       /* Do not reshape constructors of vectors (they don't need to be
 	 reshaped.  */
       else if (BRACE_ENCLOSED_INITIALIZER_P (init))
@@ -6377,7 +6398,8 @@ check_initializer (tree decl, tree init, int flags
 	diagnose_uninitialized_cst_or_ref_member (core_type, /*using_new=*/false,
 						  /*complain=*/true);
 
-      check_for_uninitialized_const_var (decl);
+      check_for_uninitialized_const_var (decl, /*constexpr_context_p=*/false,
+					 tf_warning_or_error);
     }
 
   if (init && init != error_mark_node)
Index: testsuite/g++.dg/cpp1y/constexpr-83921-1.C
===================================================================
--- testsuite/g++.dg/cpp1y/constexpr-83921-1.C	(nonexistent)
+++ testsuite/g++.dg/cpp1y/constexpr-83921-1.C	(working copy)
@@ -0,0 +1,5 @@
+// PR c++/83921
+// { dg-do compile { target c++14 } }
+
+struct Foo { };
+constexpr void test() { Foo f; }
Index: testsuite/g++.dg/cpp1y/constexpr-83921-2.C
===================================================================
--- testsuite/g++.dg/cpp1y/constexpr-83921-2.C	(nonexistent)
+++ testsuite/g++.dg/cpp1y/constexpr-83921-2.C	(working copy)
@@ -0,0 +1,5 @@
+// PR c++/83921
+// { dg-do compile { target c++14 } }
+
+struct Foo { Foo() = default; };
+constexpr void test() { Foo f; }
Index: testsuite/g++.dg/cpp1y/constexpr-83921-3.C
===================================================================
--- testsuite/g++.dg/cpp1y/constexpr-83921-3.C	(nonexistent)
+++ testsuite/g++.dg/cpp1y/constexpr-83921-3.C	(working copy)
@@ -0,0 +1,5 @@
+// PR c++/83921
+// { dg-do compile { target c++14 } }
+
+struct Foo { int m; };
+constexpr void test() { Foo f; }  // { dg-error "uninitialized" }
Index: testsuite/g++.dg/ext/stmtexpr20.C
===================================================================
--- testsuite/g++.dg/ext/stmtexpr20.C	(nonexistent)
+++ testsuite/g++.dg/ext/stmtexpr20.C	(working copy)
@@ -0,0 +1,13 @@
+// PR c++/83921
+// { dg-options "" }
+// { dg-do compile { target c++11 } }
+
+struct test { const int *addr; };
+
+const test* setup()
+{
+  static constexpr test atest =
+    { ({ int inner = 1; (const int*)(0); }) };
+
+  return &atest;
+}
Index: testsuite/g++.dg/ext/stmtexpr21.C
===================================================================
--- testsuite/g++.dg/ext/stmtexpr21.C	(nonexistent)
+++ testsuite/g++.dg/ext/stmtexpr21.C	(working copy)
@@ -0,0 +1,13 @@
+// PR c++/83921
+// { dg-options "" }
+// { dg-do compile { target c++11 } }
+
+struct test { const int *addr; };
+
+const test* setup()
+{
+  static constexpr test atest =
+    { ({ int inner; (const int*)(0); }) };  // { dg-error "uninitialized" }
+
+  return &atest;
+}

.. testing completed OK.

Just in case it wasn't obvious in my previous messages, when we are 
calling check_for_uninitialized_const_var from 
potential_constant_expression_1 we can't use CP_TYPE_CONST_P as gate for 
emitting diagnostic  - as just discovered - neither we can use 
var_in_constexpr_fn, which would cause many regressions for variables in 
constexpr functions, I remember cpp1y/pr63996.C for example. 
Conservatively, !DECL_NONTRIVIALLY_INITIALIZED_P supplemented by 
default_init_uninitialized_part appears to work fine.

Thanks,
Paolo.

On 01/22/2018 05:13 PM, Paolo Carlini wrote:
> Hi again,
> 
> On 22/01/2018 22:50, Paolo Carlini wrote:
>> Ok. The below passes the C++ testsuite and I'm finishing testing it. 
>> Therefore, as you already hinted to, we can now say that what was 
>> *really* missing from potential_constant_expression_1 was the use of 
>> default_init_uninitialized_part, which does all the non-trivial work 
>> besides the later !DECL_NONTRIVIALLY_INITIALIZED_P check. 
>> check_for_uninitialized_const_var also provides the informs, which 
>> were completely missing.
> Grrr. Testing the library revealed immediately the failure of 
> 18_support/byte/ops.cc, because in constexpr_context_p == true, thus 
> from potential_constant_expression_1, the case CP_TYPE_CONST_P triggers. 
> I guess we really want to keep the existing constexpr_context_p == false 
> cases separate. I'm therefore restarting testing with the below.

> +      && ((!constexpr_context_p
> +	   && (CP_TYPE_CONST_P (type) || var_in_constexpr_fn (decl)))
> +	  || (constexpr_context_p && !DECL_NONTRIVIALLY_INITIALIZED_P (decl)))
>        && !DECL_INITIAL (decl))

I think I'd replace the DECL_INITIAL check with 
DECL_NONTRIVIALLY_INITIALIZED_P, which seems more precise.  So we ought 
to be ok with the simpler

&& (constexpr_context_p
     || CP_TYPE_CONST_P (type) || var_in_constexpr_fn (decl))
&& !DECL_NONTRIVIALLY_INITIALIZED_P (decl))

Jason

Hi,

On 23/01/2018 18:38, Jason Merrill wrote:
> On 01/22/2018 05:13 PM, Paolo Carlini wrote:
>> Hi again,
>>
>> On 22/01/2018 22:50, Paolo Carlini wrote:
>>> Ok. The below passes the C++ testsuite and I'm finishing testing it. 
>>> Therefore, as you already hinted to, we can now say that what was 
>>> *really* missing from potential_constant_expression_1 was the use of 
>>> default_init_uninitialized_part, which does all the non-trivial work 
>>> besides the later !DECL_NONTRIVIALLY_INITIALIZED_P check. 
>>> check_for_uninitialized_const_var also provides the informs, which 
>>> were completely missing.
>> Grrr. Testing the library revealed immediately the failure of 
>> 18_support/byte/ops.cc, because in constexpr_context_p == true, thus 
>> from potential_constant_expression_1, the case CP_TYPE_CONST_P 
>> triggers. I guess we really want to keep the existing 
>> constexpr_context_p == false cases separate. I'm therefore restarting 
>> testing with the below.
>
>> +      && ((!constexpr_context_p
>> +       && (CP_TYPE_CONST_P (type) || var_in_constexpr_fn (decl)))
>> +      || (constexpr_context_p && !DECL_NONTRIVIALLY_INITIALIZED_P 
>> (decl)))
>>        && !DECL_INITIAL (decl))
>
> I think I'd replace the DECL_INITIAL check with 
> DECL_NONTRIVIALLY_INITIALIZED_P, which seems more precise.  So we 
> ought to be ok with the simpler
>
> && (constexpr_context_p
>     || CP_TYPE_CONST_P (type) || var_in_constexpr_fn (decl))
> && !DECL_NONTRIVIALLY_INITIALIZED_P (decl))
Oh nice, thanks Jason.

I'm therefore finishing regtesting the below, it's currently half way 
through the library testsuite. Ok if it passes?

Thanks again,
Paolo.

////////////////////////
Index: cp/constexpr.c
===================================================================
--- cp/constexpr.c	(revision 256991)
+++ cp/constexpr.c	(working copy)
@@ -5707,13 +5707,9 @@ potential_constant_expression_1 (tree t, bool want
 			  "%<thread_local%> in %<constexpr%> context", tmp);
 	      return false;
 	    }
-	  else if (!DECL_NONTRIVIALLY_INITIALIZED_P (tmp))
-	    {
-	      if (flags & tf_error)
-		error_at (DECL_SOURCE_LOCATION (tmp), "uninitialized "
-			  "variable %qD in %<constexpr%> context", tmp);
-	      return false;
-	    }
+	  else if (!check_for_uninitialized_const_var
+		   (tmp, /*constexpr_context_p=*/true, flags))
+	    return false;
 	}
       return RECUR (tmp, want_rval);
 
Index: cp/cp-tree.h
===================================================================
--- cp/cp-tree.h	(revision 256991)
+++ cp/cp-tree.h	(working copy)
@@ -6221,6 +6221,7 @@ extern tree finish_case_label			(location_t, tree,
 extern tree cxx_maybe_build_cleanup		(tree, tsubst_flags_t);
 extern bool check_array_designated_initializer  (constructor_elt *,
 						 unsigned HOST_WIDE_INT);
+extern bool check_for_uninitialized_const_var   (tree, bool, tsubst_flags_t);
 
 /* in decl2.c */
 extern void record_mangling			(tree, bool);
Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 256991)
+++ cp/decl.c	(working copy)
@@ -72,7 +72,6 @@ static int check_static_variable_definition (tree,
 static void record_unknown_type (tree, const char *);
 static tree builtin_function_1 (tree, tree, bool);
 static int member_function_or_else (tree, tree, enum overload_flags);
-static void check_for_uninitialized_const_var (tree);
 static tree local_variable_p_walkfn (tree *, int *, void *);
 static const char *tag_name (enum tag_types);
 static tree lookup_and_check_tag (enum tag_types, tree, tag_scope, bool);
@@ -5545,10 +5544,14 @@ maybe_commonize_var (tree decl)
     }
 }
 
-/* Issue an error message if DECL is an uninitialized const variable.  */
+/* Issue an error message if DECL is an uninitialized const variable.
+   CONSTEXPR_CONTEXT_P is true when the function is called in a constexpr
+   context from potential_constant_expression.  Returns true if all is well,
+   false otherwise.  */
 
-static void
-check_for_uninitialized_const_var (tree decl)
+bool
+check_for_uninitialized_const_var (tree decl, bool constexpr_context_p,
+				   tsubst_flags_t complain)
 {
   tree type = strip_array_types (TREE_TYPE (decl));
 
@@ -5557,26 +5560,38 @@ maybe_commonize_var (tree decl)
      7.1.6 */
   if (VAR_P (decl)
       && TREE_CODE (type) != REFERENCE_TYPE
-      && (CP_TYPE_CONST_P (type) || var_in_constexpr_fn (decl))
-      && !DECL_INITIAL (decl))
+      && (constexpr_context_p
+	  || CP_TYPE_CONST_P (type) || var_in_constexpr_fn (decl))
+      && !DECL_NONTRIVIALLY_INITIALIZED_P (decl))
     {
       tree field = default_init_uninitialized_part (type);
       if (!field)
-	return;
+	return true;
 
-      if (CP_TYPE_CONST_P (type))
-	permerror (DECL_SOURCE_LOCATION (decl),
-		   "uninitialized const %qD", decl);
-      else
+      if (!constexpr_context_p)
 	{
-	  if (!is_instantiation_of_constexpr (current_function_decl))
-	    error_at (DECL_SOURCE_LOCATION (decl),
-		      "uninitialized variable %qD in %<constexpr%> function",
-		      decl);
-	  cp_function_chain->invalid_constexpr = true;
+	  if (CP_TYPE_CONST_P (type))
+	    {
+	      if (complain & tf_error)
+		permerror (DECL_SOURCE_LOCATION (decl),
+			   "uninitialized const %qD", decl);
+	    }
+	  else
+	    {
+	      if (!is_instantiation_of_constexpr (current_function_decl)
+		  && (complain & tf_error))
+		error_at (DECL_SOURCE_LOCATION (decl),
+			  "uninitialized variable %qD in %<constexpr%> "
+			  "function", decl);
+	      cp_function_chain->invalid_constexpr = true;
+	    }
 	}
+      else if (complain & tf_error)
+	error_at (DECL_SOURCE_LOCATION (decl),
+		  "uninitialized variable %qD in %<constexpr%> context",
+		  decl);
 
-      if (CLASS_TYPE_P (type))
+      if (CLASS_TYPE_P (type) && (complain & tf_error))
 	{
 	  tree defaulted_ctor;
 
@@ -5591,7 +5606,11 @@ maybe_commonize_var (tree decl)
 		  "and the implicitly-defined constructor does not "
 		  "initialize %q#D", field);
 	}
+
+      return false;
     }
+
+  return true;
 }
 
 /* Structure holding the current initializer being processed by reshape_init.
@@ -6252,7 +6271,8 @@ check_initializer (tree decl, tree init, int flags
 	  flags |= LOOKUP_ALREADY_DIGESTED;
 	}
       else if (!init)
-	check_for_uninitialized_const_var (decl);
+	check_for_uninitialized_const_var (decl, /*constexpr_context_p =*/false,
+					   tf_warning_or_error);
       /* Do not reshape constructors of vectors (they don't need to be
 	 reshaped.  */
       else if (BRACE_ENCLOSED_INITIALIZER_P (init))
@@ -6379,7 +6399,8 @@ check_initializer (tree decl, tree init, int flags
 	diagnose_uninitialized_cst_or_ref_member (core_type, /*using_new=*/false,
 						  /*complain=*/true);
 
-      check_for_uninitialized_const_var (decl);
+      check_for_uninitialized_const_var (decl, /*constexpr_context_p=*/false,
+					 tf_warning_or_error);
     }
 
   if (init && init != error_mark_node)
Index: testsuite/g++.dg/cpp1y/constexpr-83921-1.C
===================================================================
--- testsuite/g++.dg/cpp1y/constexpr-83921-1.C	(nonexistent)
+++ testsuite/g++.dg/cpp1y/constexpr-83921-1.C	(working copy)
@@ -0,0 +1,5 @@
+// PR c++/83921
+// { dg-do compile { target c++14 } }
+
+struct Foo { };
+constexpr void test() { Foo f; }
Index: testsuite/g++.dg/cpp1y/constexpr-83921-2.C
===================================================================
--- testsuite/g++.dg/cpp1y/constexpr-83921-2.C	(nonexistent)
+++ testsuite/g++.dg/cpp1y/constexpr-83921-2.C	(working copy)
@@ -0,0 +1,5 @@
+// PR c++/83921
+// { dg-do compile { target c++14 } }
+
+struct Foo { Foo() = default; };
+constexpr void test() { Foo f; }
Index: testsuite/g++.dg/cpp1y/constexpr-83921-3.C
===================================================================
--- testsuite/g++.dg/cpp1y/constexpr-83921-3.C	(nonexistent)
+++ testsuite/g++.dg/cpp1y/constexpr-83921-3.C	(working copy)
@@ -0,0 +1,5 @@
+// PR c++/83921
+// { dg-do compile { target c++14 } }
+
+struct Foo { int m; };
+constexpr void test() { Foo f; }  // { dg-error "uninitialized" }
Index: testsuite/g++.dg/ext/stmtexpr20.C
===================================================================
--- testsuite/g++.dg/ext/stmtexpr20.C	(nonexistent)
+++ testsuite/g++.dg/ext/stmtexpr20.C	(working copy)
@@ -0,0 +1,13 @@
+// PR c++/83921
+// { dg-options "" }
+// { dg-do compile { target c++11 } }
+
+struct test { const int *addr; };
+
+const test* setup()
+{
+  static constexpr test atest =
+    { ({ int inner = 1; (const int*)(0); }) };
+
+  return &atest;
+}
Index: testsuite/g++.dg/ext/stmtexpr21.C
===================================================================
--- testsuite/g++.dg/ext/stmtexpr21.C	(nonexistent)
+++ testsuite/g++.dg/ext/stmtexpr21.C	(working copy)
@@ -0,0 +1,13 @@
+// PR c++/83921
+// { dg-options "" }
+// { dg-do compile { target c++11 } }
+
+struct test { const int *addr; };
+
+const test* setup()
+{
+  static constexpr test atest =
+    { ({ int inner; (const int*)(0); }) };  // { dg-error "uninitialized" }
+
+  return &atest;
+}

OK, thanks.

On Tue, Jan 23, 2018 at 2:37 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi,
>
> On 23/01/2018 18:38, Jason Merrill wrote:
>>
>> On 01/22/2018 05:13 PM, Paolo Carlini wrote:
>>>
>>> Hi again,
>>>
>>> On 22/01/2018 22:50, Paolo Carlini wrote:
>>>>
>>>> Ok. The below passes the C++ testsuite and I'm finishing testing it.
>>>> Therefore, as you already hinted to, we can now say that what was *really*
>>>> missing from potential_constant_expression_1 was the use of
>>>> default_init_uninitialized_part, which does all the non-trivial work besides
>>>> the later !DECL_NONTRIVIALLY_INITIALIZED_P check.
>>>> check_for_uninitialized_const_var also provides the informs, which were
>>>> completely missing.
>>>
>>> Grrr. Testing the library revealed immediately the failure of
>>> 18_support/byte/ops.cc, because in constexpr_context_p == true, thus from
>>> potential_constant_expression_1, the case CP_TYPE_CONST_P triggers. I guess
>>> we really want to keep the existing constexpr_context_p == false cases
>>> separate. I'm therefore restarting testing with the below.
>>
>>
>>> +      && ((!constexpr_context_p
>>> +       && (CP_TYPE_CONST_P (type) || var_in_constexpr_fn (decl)))
>>> +      || (constexpr_context_p && !DECL_NONTRIVIALLY_INITIALIZED_P
>>> (decl)))
>>>        && !DECL_INITIAL (decl))
>>
>>
>> I think I'd replace the DECL_INITIAL check with
>> DECL_NONTRIVIALLY_INITIALIZED_P, which seems more precise.  So we ought to
>> be ok with the simpler
>>
>> && (constexpr_context_p
>>     || CP_TYPE_CONST_P (type) || var_in_constexpr_fn (decl))
>> && !DECL_NONTRIVIALLY_INITIALIZED_P (decl))
>
> Oh nice, thanks Jason.
>
> I'm therefore finishing regtesting the below, it's currently half way
> through the library testsuite. Ok if it passes?
>
> Thanks again,
> Paolo.
>
> ////////////////////////