Re: C++ PATCH for C++17 selection statements with initializer

[Marek Polacek]

On Sat, Nov 05, 2016 at 10:03:37PM -0400, David Edelsohn wrote:
> The patch adds testcase init-statement6.C, which includes the declaration
> 
> extern void publish (int), raise (int);
> 
> POSIX defines
> 
> int raise (int);
> 
> in  which gets included by the C++ headers for the testcase on AIX.
> 
> This is causes the error message:
> 
> /nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/cpp1z/init-statement6.C:10:28:
> error: ambiguating new declaration of 'void raise(int)'
> ...
> /tmp/GCC/gcc/include-fixed/sys/signal.h:103:12: note: old declaration
> 'int raise(int)'
> 
> Is there a reason for the conflicting / ambiguating declaration?

Oops, no reason at all.  I'm fixing this with:

Tested on x86_64-linux, applying to trunk.

2016-11-15  Marek Polacek  

* g++.dg/cpp1z/init-statement6.C: Rename a function.

diff --git gcc/testsuite/g++.dg/cpp1z/init-statement6.C 
gcc/testsuite/g++.dg/cpp1z/init-statement6.C
index 53b0d31..e8e24b5 100644
--- gcc/testsuite/g++.dg/cpp1z/init-statement6.C
+++ gcc/testsuite/g++.dg/cpp1z/init-statement6.C
@@ -7,14 +7,14 @@
 
 std::map m;
 extern int xread (int *);
-extern void publish (int), raise (int);
+extern void publish (int), xraise (int);
 
 void
 foo ()
 {
   if (auto it = m.find (10); it != m.end ()) { std::string s = it->second; }
   if (char buf[10]; std::fgets(buf, 10, stdin)) { m[0] += buf; }
-  if (int s; int count = xread ()) { publish(count); raise(s); }
+  if (int s; int count = xread ()) { publish(count); xraise(s); }
 
   const char *s;
   if (auto keywords = {"if", "for", "while"};

Marek

Re: C++ PATCH for C++17 selection statements with initializer

[David Edelsohn]

The patch adds testcase init-statement6.C, which includes the declaration

extern void publish (int), raise (int);

POSIX defines

int raise (int);

in  which gets included by the C++ headers for the testcase on AIX.

This is causes the error message:

/nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/cpp1z/init-statement6.C:10:28:
error: ambiguating new declaration of 'void raise(int)'
...
/tmp/GCC/gcc/include-fixed/sys/signal.h:103:12: note: old declaration
'int raise(int)'

Is there a reason for the conflicting / ambiguating declaration?

Thanks, David

Re: C++ PATCH for C++17 selection statements with initializer

[Jason Merrill]

OK.

Re: C++ PATCH for C++17 selection statements with initializer

[Marek Polacek]

On Wed, Oct 05, 2016 at 12:11:40PM -0400, Jason Merrill wrote:
> On Wed, Oct 5, 2016 at 11:29 AM, Marek Polacek  wrote:
> > How about the version I just posted, i.e.
> > ?
> 
> That doesn't address my first comment.

The following uses cp_parser_skip_to_closing_parenthesis_1, and is, I think,
much nicer.

Bootstrapped/regtested on x86_64-linux and ppc64-linux, ok for trunk?

2016-10-05  Marek Polacek  

Implement P0305R1, Selection statements with initializer.
* cp-array-notation.c (create_an_loop): Call finish_init_stmt
instead of finish_for_init_stmt.
* cp-tree.h (finish_for_init_stmt): Rename to finish_init_stmt.
* decl.c (poplevel): Adjust a comment.
* init.c (build_vec_init): Call finish_init_stmt instead of
finish_for_init_stmt.
* name-lookup.c (pushdecl_maybe_friend_1): Adjust a comment.
* name-lookup.h (enum scope_kind): Likewise.
* parser.c (cp_parser_statement): Update commentary.
(cp_parser_init_statement_p): New function.
(cp_parser_selection_statement): Parse the optional init-statement.
(cp_parser_for): Call finish_init_stmt instead of finish_for_init_stmt.
(cp_parser_c_for): Likewise.
(cp_convert_range_for): Call finish_init_stmt instead of 
finish_for_init_stmt.
(cp_parser_range_for_member_function): Update commentary.
(cp_parser_iteration_statement):
(cp_parser_for_init_statement): Rename to cp_parser_init_statement.
* pt.c (tsubst_omp_for_iterator): Update commentary.
(tsubst_expr): Call finish_init_stmt instead of finish_for_init_stmt.
* semantics.c (finish_for_init_stmt): Rename to finish_init_stmt.
Update commentary.

* g++.dg/cpp1z/init-statement1.C: New test.
* g++.dg/cpp1z/init-statement2.C: New test.
* g++.dg/cpp1z/init-statement3.C: New test.
* g++.dg/cpp1z/init-statement4.C: New test.
* g++.dg/cpp1z/init-statement5.C: New test.
* g++.dg/cpp1z/init-statement6.C: New test.
* g++.dg/cpp1z/init-statement7.C: New test.
* g++.dg/cpp1z/init-statement8.C: New test.

diff --git gcc/cp/cp-array-notation.c gcc/cp/cp-array-notation.c
index 4687ced..633ab09 100644
--- gcc/cp/cp-array-notation.c
+++ gcc/cp/cp-array-notation.c
@@ -66,7 +66,7 @@ create_an_loop (tree init, tree cond, tree incr, tree body)
 
   finish_expr_stmt (init);
   for_stmt = begin_for_stmt (NULL_TREE, NULL_TREE);
-  finish_for_init_stmt (for_stmt);
+  finish_init_stmt (for_stmt);
   finish_for_cond (cond, for_stmt, false);
   finish_for_expr (incr, for_stmt);
   finish_expr_stmt (body);
diff --git gcc/cp/cp-tree.h gcc/cp/cp-tree.h
index 3fbe1d9..92e4017 100644
--- gcc/cp/cp-tree.h
+++ gcc/cp/cp-tree.h
@@ -6297,7 +6297,7 @@ extern void finish_do_stmt(tree, 
tree, bool);
 extern tree finish_return_stmt (tree);
 extern tree begin_for_scope(tree *);
 extern tree begin_for_stmt (tree, tree);
-extern void finish_for_init_stmt   (tree);
+extern void finish_init_stmt   (tree);
 extern void finish_for_cond(tree, tree, bool);
 extern void finish_for_expr(tree, tree);
 extern void finish_for_stmt(tree);
diff --git gcc/cp/decl.c gcc/cp/decl.c
index 6646062..6a08d8f 100644
--- gcc/cp/decl.c
+++ gcc/cp/decl.c
@@ -639,9 +639,8 @@ poplevel (int keep, int reverse, int functionbody)
   BLOCK_SUPERCONTEXT (link) = block;
 
   /* We still support the old for-scope rules, whereby the variables
- in a for-init statement were in scope after the for-statement
- ended.  We only use the new rules if flag_new_for_scope is
- nonzero.  */
+ in a init statement were in scope after the for-statement ended.
+ We only use the new rules if flag_new_for_scope is nonzero.  */
   leaving_for_scope
 = current_binding_level->kind == sk_for && flag_new_for_scope == 1;
 
diff --git gcc/cp/init.c gcc/cp/init.c
index 2d5877d..d1c8274 100644
--- gcc/cp/init.c
+++ gcc/cp/init.c
@@ -4052,7 +4052,7 @@ build_vec_init (tree base, tree maxindex, tree init,
   tree to;
 
   for_stmt = begin_for_stmt (NULL_TREE, NULL_TREE);
-  finish_for_init_stmt (for_stmt);
+  finish_init_stmt (for_stmt);
   finish_for_cond (build2 (GT_EXPR, boolean_type_node, iterator,
   build_int_cst (TREE_TYPE (iterator), -1)),
   for_stmt, false);
diff --git gcc/cp/name-lookup.c gcc/cp/name-lookup.c
index 1bce63b..9e84a1b 100644
--- gcc/cp/name-lookup.c
+++ gcc/cp/name-lookup.c
@@ -1156,7 +1156,7 @@ pushdecl_maybe_friend_1 (tree x, bool is_friend)
   }
}
  /* Error if redeclaring a local declared in a
-for-init-statement or in the condition of an if 

Re: C++ PATCH for C++17 selection statements with initializer

[Jason Merrill]

On Wed, Oct 5, 2016 at 11:29 AM, Marek Polacek  wrote:
> On Wed, Oct 05, 2016 at 10:48:19AM -0400, Jason Merrill wrote:
>> On Wed, Oct 5, 2016 at 9:14 AM, Marek Polacek  wrote:
>> > +/* Return true if we're looking at (init; cond), false otherwise.  */
>> > +
>> > +static bool
>> > +cp_parser_init_statement_p (cp_parser *parser)
>> > +{
>> > +  unsigned paren_depth = 0;
>> > +  unsigned brace_depth = 0;
>>
>> Do we really need another one of these token scanning functions?
>> Can't you write this in terms of
>> cp_parser_skip_to_closing_parenthesis?
>>
>> > +   /* Parse the optional init-statement.  */
>> > +   tree decl;
>> > +   cp_lexer_save_tokens (parser->lexer);
>> > +   const bool init_stmt_p = cp_parser_init_statement_p (parser);
>> > +   /* Roll back the tokens we skipped.  */
>> > +   cp_lexer_rollback_tokens (parser->lexer);
>>
>> The save/rollback should be in the the predicate function, not the caller.
>
> How about the version I just posted, i.e.
> ?

That doesn't address my first comment.

Jason

Re: C++ PATCH for C++17 selection statements with initializer

[Marek Polacek]

On Wed, Oct 05, 2016 at 10:48:19AM -0400, Jason Merrill wrote:
> On Wed, Oct 5, 2016 at 9:14 AM, Marek Polacek  wrote:
> > +/* Return true if we're looking at (init; cond), false otherwise.  */
> > +
> > +static bool
> > +cp_parser_init_statement_p (cp_parser *parser)
> > +{
> > +  unsigned paren_depth = 0;
> > +  unsigned brace_depth = 0;
> 
> Do we really need another one of these token scanning functions?
> Can't you write this in terms of
> cp_parser_skip_to_closing_parenthesis?
> 
> > +   /* Parse the optional init-statement.  */
> > +   tree decl;
> > +   cp_lexer_save_tokens (parser->lexer);
> > +   const bool init_stmt_p = cp_parser_init_statement_p (parser);
> > +   /* Roll back the tokens we skipped.  */
> > +   cp_lexer_rollback_tokens (parser->lexer);
> 
> The save/rollback should be in the the predicate function, not the caller.

How about the version I just posted, i.e.
?

Marek

Re: C++ PATCH for C++17 selection statements with initializer

[Marek Polacek]

On Wed, Oct 05, 2016 at 03:31:51PM +0200, Jakub Jelinek wrote:
> On Wed, Oct 05, 2016 at 03:14:25PM +0200, Marek Polacek wrote:
> > This is my attempt to implement P0305R1, Selection statements with 
> > initializer.
> > It allows the users to write
> > 
> >   if (init; cond) // ...
> > 
> > which is equivalent to
> > 
> >   {
> > init
> > if (cond) // ...
> >   }
> 
> Well, it isn't exactly equivalent, because unlike { init; if (cond) /* ... 
> */; }
> there aren't two scopes, but just one.  So I'd say you should have tests

It was basically a quote from the standard.

> that verify that init and cond are indeed in the same scope, e.g. by trying
> something like if (int c = 5; int c = 5) ... and verifying dg-error is
> reported.

This works as expected -- with a "redeclaration" error.  I've added a test.

> > +   case CPP_CLOSE_PAREN:
> > + /* If the next token is a non-nested '(', then we have reached
> > +the end of the if condition.  */
> 
> Looks like typo, shouldn't that be ')' ?

Ah, yes.
 
> Also, do you really need two counters?
> 
> > + if (paren_depth-- == 0)
> > +   return false;
> > + break;
> > +
> > +   case CPP_OPEN_PAREN:
> > + ++paren_depth;
> > + break;
> > +
> > +   case CPP_CLOSE_BRACE:
> > + --brace_depth;
> 
> I mean, shouldn't you also stop before } when seeing if (int a = }; b)
> rather than wrapping around?
 
Ok, one counter seems to work too.  Fixed.

> > +  /* Consume the token.  */
> > +  cp_lexer_consume_token (parser->lexer);
> > +}
> 
> Also, do you really need to consume all the tokens and then rollback, rather
> than just use peek_nth_token with the index increasing in each iteration?

Here I followed cp_parser_compound_literal_p and cp_parser_array_designator_p.
But since I only read token, sure, I can do what you suggest.

Anything else?

Bootstrapped/regtested on x86_64-linux and ppc64-linux, ok for trunk?

2016-10-05  Marek Polacek  

Implement P0305R1, Selection statements with initializer.
* cp-array-notation.c (create_an_loop): Call finish_init_stmt
instead of finish_for_init_stmt.
* cp-tree.h (finish_for_init_stmt): Rename to finish_init_stmt.
* decl.c (poplevel): Adjust a comment.
* init.c (build_vec_init): Call finish_init_stmt instead of
finish_for_init_stmt.
* name-lookup.c (pushdecl_maybe_friend_1): Adjust a comment.
* name-lookup.h (enum scope_kind): Likewise.
* parser.c (cp_parser_statement): Update commentary.
(cp_parser_init_statement_p): New function.
(cp_parser_selection_statement): Parse the optional init-statement.
(cp_parser_for): Call finish_init_stmt instead of finish_for_init_stmt.
(cp_parser_c_for): Likewise.
(cp_convert_range_for): Call finish_init_stmt instead of 
finish_for_init_stmt.
(cp_parser_range_for_member_function): Update commentary.
(cp_parser_iteration_statement):
(cp_parser_for_init_statement): Rename to cp_parser_init_statement.
* pt.c (tsubst_omp_for_iterator): Update commentary.
(tsubst_expr): Call finish_init_stmt instead of finish_for_init_stmt.
* semantics.c (finish_for_init_stmt): Rename to finish_init_stmt.
Update commentary.

* g++.dg/cpp1z/init-statement1.C: New test.
* g++.dg/cpp1z/init-statement2.C: New test.
* g++.dg/cpp1z/init-statement3.C: New test.
* g++.dg/cpp1z/init-statement4.C: New test.
* g++.dg/cpp1z/init-statement5.C: New test.
* g++.dg/cpp1z/init-statement6.C: New test.
* g++.dg/cpp1z/init-statement7.C: New test.
* g++.dg/cpp1z/init-statement8.C: New test.

diff --git gcc/cp/cp-array-notation.c gcc/cp/cp-array-notation.c
index 4687ced..633ab09 100644
--- gcc/cp/cp-array-notation.c
+++ gcc/cp/cp-array-notation.c
@@ -66,7 +66,7 @@ create_an_loop (tree init, tree cond, tree incr, tree body)
 
   finish_expr_stmt (init);
   for_stmt = begin_for_stmt (NULL_TREE, NULL_TREE);
-  finish_for_init_stmt (for_stmt);
+  finish_init_stmt (for_stmt);
   finish_for_cond (cond, for_stmt, false);
   finish_for_expr (incr, for_stmt);
   finish_expr_stmt (body);
diff --git gcc/cp/cp-tree.h gcc/cp/cp-tree.h
index 3fbe1d9..92e4017 100644
--- gcc/cp/cp-tree.h
+++ gcc/cp/cp-tree.h
@@ -6297,7 +6297,7 @@ extern void finish_do_stmt(tree, 
tree, bool);
 extern tree finish_return_stmt (tree);
 extern tree begin_for_scope(tree *);
 extern tree begin_for_stmt (tree, tree);
-extern void finish_for_init_stmt   (tree);
+extern void finish_init_stmt   (tree);
 extern void finish_for_cond(tree, tree, bool);
 extern void finish_for_expr(tree, tree);
 extern void finish_for_stmt(tree);
diff --git gcc/cp/decl.c gcc/cp/decl.c
index 6646062..6a08d8f 100644
--- gcc/cp/decl.c

Re: C++ PATCH for C++17 selection statements with initializer

[Jason Merrill]

On Wed, Oct 5, 2016 at 9:14 AM, Marek Polacek  wrote:
> +/* Return true if we're looking at (init; cond), false otherwise.  */
> +
> +static bool
> +cp_parser_init_statement_p (cp_parser *parser)
> +{
> +  unsigned paren_depth = 0;
> +  unsigned brace_depth = 0;

Do we really need another one of these token scanning functions?
Can't you write this in terms of
cp_parser_skip_to_closing_parenthesis?

> +   /* Parse the optional init-statement.  */
> +   tree decl;
> +   cp_lexer_save_tokens (parser->lexer);
> +   const bool init_stmt_p = cp_parser_init_statement_p (parser);
> +   /* Roll back the tokens we skipped.  */
> +   cp_lexer_rollback_tokens (parser->lexer);

The save/rollback should be in the the predicate function, not the caller.

Jason

Re: C++ PATCH for C++17 selection statements with initializer

[Jakub Jelinek]

On Wed, Oct 05, 2016 at 03:14:25PM +0200, Marek Polacek wrote:
> This is my attempt to implement P0305R1, Selection statements with 
> initializer.
> It allows the users to write
> 
>   if (init; cond) // ...
> 
> which is equivalent to
> 
>   {
> init
> if (cond) // ...
>   }

Well, it isn't exactly equivalent, because unlike { init; if (cond) /* ... */; }
there aren't two scopes, but just one.  So I'd say you should have tests
that verify that init and cond are indeed in the same scope, e.g. by trying
something like if (int c = 5; int c = 5) ... and verifying dg-error is
reported.

> + case CPP_CLOSE_PAREN:
> +   /* If the next token is a non-nested '(', then we have reached
> +  the end of the if condition.  */

Looks like typo, shouldn't that be ')' ?

Also, do you really need two counters?

> +   if (paren_depth-- == 0)
> + return false;
> +   break;
> +
> + case CPP_OPEN_PAREN:
> +   ++paren_depth;
> +   break;
> +
> + case CPP_CLOSE_BRACE:
> +   --brace_depth;

I mean, shouldn't you also stop before } when seeing if (int a = }; b)
rather than wrapping around?

> +  /* Consume the token.  */
> +  cp_lexer_consume_token (parser->lexer);
> +}

Also, do you really need to consume all the tokens and then rollback, rather
than just use peek_nth_token with the index increasing in each iteration?

Jakub

C++ PATCH for C++17 selection statements with initializer

[Marek Polacek]

This is my attempt to implement P0305R1, Selection statements with initializer.
It allows the users to write

  if (init; cond) // ...

which is equivalent to

  {
init
if (cond) // ...
  }

Similarly for if-else, if constexpr, and switch.

The approach I had taken was to tentatively parse "init" and if no semicolon
follows, abort the parse and just parse the condition.  But this didn't work.
An init-statement is either a simple-declaration or an expression-statement,
and parsing either has irreversible side-effects--e.g. adding a new statement
or a new declaration, or duplicating side-effects.  Using firewalls here
doesn't help.  Since an init-statement ends with a semicolon, I decided to
write cp_parser_init_statement_p, which checks if there's a semicolon in the if
condition that is not nested in () or in {} (think of ({}) or lambda
expressions).  It seemed a bit weird but seems to work well.  And then it's
just a simple matter of calling cp_parser_init_statement that parses either a
simple declaration or an expression statement and does the right thing wrt
scopes.

Since "for-init-statement" is no longer in the standard, I renamed all the
occurrences to "init-statement".

There's also this -ffor-scope thing, but I didn't touch it and it's probably
not necessary to.

Bootstrapped/regtested on x86_64-linux and ppc64-linux, ok for trunk? 

2016-10-05  Marek Polacek  

Implement P0305R1, Selection statements with initializer.
* cp-array-notation.c (create_an_loop): Call finish_init_stmt
instead of finish_for_init_stmt.
* cp-tree.h (finish_for_init_stmt): Rename to finish_init_stmt.
* decl.c (poplevel): Adjust a comment.
* init.c (build_vec_init): Call finish_init_stmt instead of
finish_for_init_stmt.
* name-lookup.c (pushdecl_maybe_friend_1): Adjust a comment.
* name-lookup.h (enum scope_kind): Likewise.
* parser.c (cp_parser_statement): Update commentary.
(cp_parser_init_statement_p): New function.
(cp_parser_selection_statement): Parse the optional init-statement.
(cp_parser_for): Call finish_init_stmt instead of finish_for_init_stmt.
(cp_parser_c_for): Likewise.
(cp_convert_range_for): Call finish_init_stmt instead of 
finish_for_init_stmt.
(cp_parser_range_for_member_function): Update commentary.
(cp_parser_iteration_statement):
(cp_parser_for_init_statement): Rename to cp_parser_init_statement.
* pt.c (tsubst_omp_for_iterator): Update commentary.
(tsubst_expr): Call finish_init_stmt instead of finish_for_init_stmt.
* semantics.c (finish_for_init_stmt): Rename to finish_init_stmt.
Update commentary.

* g++.dg/cpp1z/init-statement1.C: New test.
* g++.dg/cpp1z/init-statement2.C: New test.
* g++.dg/cpp1z/init-statement3.C: New test.
* g++.dg/cpp1z/init-statement4.C: New test.
* g++.dg/cpp1z/init-statement5.C: New test.
* g++.dg/cpp1z/init-statement6.C: New test.
* g++.dg/cpp1z/init-statement7.C: New test.

diff --git gcc/cp/cp-array-notation.c gcc/cp/cp-array-notation.c
index 4687ced..633ab09 100644
--- gcc/cp/cp-array-notation.c
+++ gcc/cp/cp-array-notation.c
@@ -66,7 +66,7 @@ create_an_loop (tree init, tree cond, tree incr, tree body)
 
   finish_expr_stmt (init);
   for_stmt = begin_for_stmt (NULL_TREE, NULL_TREE);
-  finish_for_init_stmt (for_stmt);
+  finish_init_stmt (for_stmt);
   finish_for_cond (cond, for_stmt, false);
   finish_for_expr (incr, for_stmt);
   finish_expr_stmt (body);
diff --git gcc/cp/cp-tree.h gcc/cp/cp-tree.h
index 3fbe1d9..92e4017 100644
--- gcc/cp/cp-tree.h
+++ gcc/cp/cp-tree.h
@@ -6297,7 +6297,7 @@ extern void finish_do_stmt(tree, 
tree, bool);
 extern tree finish_return_stmt (tree);
 extern tree begin_for_scope(tree *);
 extern tree begin_for_stmt (tree, tree);
-extern void finish_for_init_stmt   (tree);
+extern void finish_init_stmt   (tree);
 extern void finish_for_cond(tree, tree, bool);
 extern void finish_for_expr(tree, tree);
 extern void finish_for_stmt(tree);
diff --git gcc/cp/decl.c gcc/cp/decl.c
index 6646062..6a08d8f 100644
--- gcc/cp/decl.c
+++ gcc/cp/decl.c
@@ -639,9 +639,8 @@ poplevel (int keep, int reverse, int functionbody)
   BLOCK_SUPERCONTEXT (link) = block;
 
   /* We still support the old for-scope rules, whereby the variables
- in a for-init statement were in scope after the for-statement
- ended.  We only use the new rules if flag_new_for_scope is
- nonzero.  */
+ in a init statement were in scope after the for-statement ended.
+ We only use the new rules if flag_new_for_scope is nonzero.  */
   leaving_for_scope
 = current_binding_level->kind == sk_for && flag_new_for_scope == 1;
 
diff --git