diff mbox

C++ PATCH for c++/78948 (instantiation from discarded statement)

Message ID CADzB+2mah0B-_rJ6zjhSRX+k+cwo4y7OzzwEL71_giz4fkY7EA@mail.gmail.com
State New
Headers show

Commit Message

Jason Merrill Jan. 8, 2017, 6:34 a.m. UTC
P0292 defines the notion of "discarded statement" which is almost but
not quite the same as "unevaluated operand".  This PR shows a case
where we need to be able to tell that we're in a discarded statement
at a lower level than in the parser, so this patch moves the
information about being in a discarded statement from the parser into
saved_scope.  I've also added a test for a couple of cases that
demonstrate why we can't just use cp_unevaluated_context.

Tested x86_64-pc-linux-gnu, applying to trunk.
commit 4d3de5520010cda3bdb47c8a6532e734f09cfc3e
Author: Jason Merrill <jason@redhat.com>
Date:   Fri Jan 6 09:45:02 2017 -0500

            PR c++/78948 - instantiation from discarded statement
            * parser.h (struct cp_parser): Remove in_discarded_stmt field.
            * cp-tree.h (in_discarded_stmt): Declare it.
            (struct saved_scope): Add discarded_stmt bitfield.
            (in_discarded_stmt): New macro.
            * decl2.c (mark_used): Check it.
            * parser.c (cp_parser_selection_statement): Adjust.
            (cp_parser_jump_statement): Adjust.

Comments

Nathan Sidwell Jan. 9, 2017, 1:49 p.m. UTC | #1
On 01/08/2017 01:34 AM, Jason Merrill wrote:
> P0292 defines the notion of "discarded statement" which is almost but
> not quite the same as "unevaluated operand".  This PR shows a case
> where we need to be able to tell that we're in a discarded statement
> at a lower level than in the parser, so this patch moves the
> information about being in a discarded statement from the parser into
> saved_scope.  I've also added a test for a couple of cases that
> demonstrate why we can't just use cp_unevaluated_context.

> +  if constexpr(sizeof(long long) == sizeof(char*))
> +    ;
> +  else
> +    sizeof_mismatch<sizeof(long long)>();

This is going to behave differently on 32 and 64 bit HW.  Is that 
intentional? (If so, a comment would be nice)

nathan
Kyrill Tkachov Jan. 9, 2017, 1:55 p.m. UTC | #2
On 09/01/17 13:49, Nathan Sidwell wrote:
> On 01/08/2017 01:34 AM, Jason Merrill wrote:
>> P0292 defines the notion of "discarded statement" which is almost but
>> not quite the same as "unevaluated operand".  This PR shows a case
>> where we need to be able to tell that we're in a discarded statement
>> at a lower level than in the parser, so this patch moves the
>> information about being in a discarded statement from the parser into
>> saved_scope.  I've also added a test for a couple of cases that
>> demonstrate why we can't just use cp_unevaluated_context.
>
>> +  if constexpr(sizeof(long long) == sizeof(char*))
>> +    ;
>> +  else
>> +    sizeof_mismatch<sizeof(long long)>();
>
> This is going to behave differently on 32 and 64 bit HW.  Is that intentional? (If so, a comment would be nice)
>

Indeed I see this test failing when testing aarch64 with -mabi=ilp32.

Kyrill

> nathan
>
Jakub Jelinek Jan. 9, 2017, 2:03 p.m. UTC | #3
On Mon, Jan 09, 2017 at 08:49:25AM -0500, Nathan Sidwell wrote:
> On 01/08/2017 01:34 AM, Jason Merrill wrote:
> > P0292 defines the notion of "discarded statement" which is almost but
> > not quite the same as "unevaluated operand".  This PR shows a case
> > where we need to be able to tell that we're in a discarded statement
> > at a lower level than in the parser, so this patch moves the
> > information about being in a discarded statement from the parser into
> > saved_scope.  I've also added a test for a couple of cases that
> > demonstrate why we can't just use cp_unevaluated_context.
> 
> > +  if constexpr(sizeof(long long) == sizeof(char*))
> > +    ;
> > +  else
> > +    sizeof_mismatch<sizeof(long long)>();
> 
> This is going to behave differently on 32 and 64 bit HW.  Is that
> intentional? (If so, a comment would be nice)

Yeah, it fails on i686-linux and other ILP32 and similar targets.

FAIL: g++.dg/cpp1z/constexpr-if10.C   (test for excess errors)

Could we do e.g.
sed -i -e 's/long long/int */g' testsuite/g++.dg/cpp1z/constexpr-if10.C
so that it is something where if constexpr will be always true?

	Jakub
Nathan Sidwell Jan. 9, 2017, 2:10 p.m. UTC | #4
On 01/09/2017 09:03 AM, Jakub Jelinek wrote:

> FAIL: g++.dg/cpp1z/constexpr-if10.C   (test for excess errors)
>
> Could we do e.g.
> sed -i -e 's/long long/int */g' testsuite/g++.dg/cpp1z/constexpr-if10.C
> so that it is something where if constexpr will be always true?

that would seem fine to me

nathan
diff mbox

Patch

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 39f5d79..24de346 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -1281,6 +1281,10 @@  struct GTY(()) saved_scope {
   BOOL_BITFIELD x_processing_explicit_instantiation : 1;
   BOOL_BITFIELD need_pop_function_context : 1;
 
+/* Nonzero if we are parsing the discarded statement of a constexpr
+   if-statement.  */
+  BOOL_BITFIELD discarded_stmt : 1;
+
   int unevaluated_operand;
   int inhibit_evaluation_warnings;
   int noexcept_operand;
@@ -1341,6 +1345,8 @@  extern GTY(()) struct saved_scope *scope_chain;
 #define processing_specialization scope_chain->x_processing_specialization
 #define processing_explicit_instantiation scope_chain->x_processing_explicit_instantiation
 
+#define in_discarded_stmt scope_chain->discarded_stmt
+
 /* RAII sentinel to handle clearing processing_template_decl and restoring
    it when done.  */
 
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index a0375ad..435f51f 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -5112,7 +5112,7 @@  mark_used (tree decl, tsubst_flags_t complain)
     }
 
   /* If we don't need a value, then we don't need to synthesize DECL.  */
-  if (cp_unevaluated_operand != 0)
+  if (cp_unevaluated_operand || in_discarded_stmt)
     return true;
 
   DECL_ODR_USED (decl) = 1;
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 57ae064..e8c0642 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -11147,12 +11147,12 @@  cp_parser_selection_statement (cp_parser* parser, bool *if_p,
 
 	    /* Outside a template, the non-selected branch of a constexpr
 	       if is a 'discarded statement', i.e. unevaluated.  */
-	    bool was_discarded = parser->in_discarded_stmt;
+	    bool was_discarded = in_discarded_stmt;
 	    bool discard_then = (cx && !processing_template_decl
 				 && integer_zerop (condition));
 	    if (discard_then)
 	      {
-		parser->in_discarded_stmt = true;
+		in_discarded_stmt = true;
 		++c_inhibit_evaluation_warnings;
 	      }
 
@@ -11166,7 +11166,7 @@  cp_parser_selection_statement (cp_parser* parser, bool *if_p,
 	    if (discard_then)
 	      {
 		THEN_CLAUSE (statement) = NULL_TREE;
-		parser->in_discarded_stmt = was_discarded;
+		in_discarded_stmt = was_discarded;
 		--c_inhibit_evaluation_warnings;
 	      }
 
@@ -11178,7 +11178,7 @@  cp_parser_selection_statement (cp_parser* parser, bool *if_p,
 				     && integer_nonzerop (condition));
 		if (discard_else)
 		  {
-		    parser->in_discarded_stmt = true;
+		    in_discarded_stmt = true;
 		    ++c_inhibit_evaluation_warnings;
 		  }
 
@@ -11235,7 +11235,7 @@  cp_parser_selection_statement (cp_parser* parser, bool *if_p,
 		if (discard_else)
 		  {
 		    ELSE_CLAUSE (statement) = NULL_TREE;
-		    parser->in_discarded_stmt = was_discarded;
+		    in_discarded_stmt = was_discarded;
 		    --c_inhibit_evaluation_warnings;
 		  }
 	      }
@@ -12143,7 +12143,7 @@  cp_parser_jump_statement (cp_parser* parser)
 	     expression.  */
 	  expr = NULL_TREE;
 	/* Build the return-statement.  */
-	if (current_function_auto_return_pattern && parser->in_discarded_stmt)
+	if (current_function_auto_return_pattern && in_discarded_stmt)
 	  /* Don't deduce from a discarded return statement.  */;
 	else
 	  statement = finish_return_stmt (expr);
diff --git a/gcc/cp/parser.h b/gcc/cp/parser.h
index f242f4c..0994e1e 100644
--- a/gcc/cp/parser.h
+++ b/gcc/cp/parser.h
@@ -336,10 +336,6 @@  struct GTY(()) cp_parser {
      a local class.  */
   bool in_function_body;
 
-  /* TRUE if we are parsing a C++17 discarded statement (the non-taken branch
-     of an if constexpr).  */
-  bool in_discarded_stmt;
-
   /* Nonzero if we're processing a __transaction_atomic or
      __transaction_relaxed statement.  */
   unsigned char in_transaction;
diff --git a/gcc/testsuite/g++.dg/cpp1z/constexpr-if10.C b/gcc/testsuite/g++.dg/cpp1z/constexpr-if10.C
new file mode 100644
index 0000000..64de53f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/constexpr-if10.C
@@ -0,0 +1,16 @@ 
+// PR c++/79848
+// { dg-options -std=c++1z }
+
+template <int T>
+void sizeof_mismatch()
+{
+    static_assert(T == 0, "sizeof mismatch");
+}
+
+int main()
+{
+  if constexpr(sizeof(long long) == sizeof(char*))
+    ;
+  else
+    sizeof_mismatch<sizeof(long long)>();
+}
diff --git a/gcc/testsuite/g++.dg/cpp1z/constexpr-if11.C b/gcc/testsuite/g++.dg/cpp1z/constexpr-if11.C
new file mode 100644
index 0000000..1c6247e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/constexpr-if11.C
@@ -0,0 +1,16 @@ 
+// Test that discarded statements differ from unevaluated operands in some
+// ways.
+// { dg-options -std=c++1z }
+
+struct A { int i; };
+
+int main()
+{
+  if constexpr(true)
+    ;
+  else
+    {
+      []{}();
+      A::i;			// { dg-error "non-static" }
+    }
+}