Re: C++ PATCH to implement P1094R2, Nested inline namespaces

Re: C++ PATCH to implement P1094R2, Nested inline namespaces

[Ville Voutilainen]

[-- Attachment #1: Type: text/plain, Size: 1002 bytes --]

On Tue, Nov 20, 2018 at 04:59:46PM -0500, Jason Merrill wrote:
> On 11/19/18 5:12 PM, Marek Polacek wrote:
>> > +  /* Don't forget that the innermost namespace might have been
>> > +     marked as inline.  */
>> > +  is_inline |= nested_inline_p;
>> This looks wrong: an inline namespace does not make its nested namespaces
>> inline as well.

>A nested namespace definition cannot be inline.  This is supposed to handle
>cases such as
>namespace A::B::inline C { ... }
>because after 'C' we don't see :: so it breaks and we call push_namespace
>outside the for loop.  So I still don't see a bug; do you have a test that
>I got wrong?

The way I read the question is "what does

namespace A::inline B::C::D {...}

do?".

C and D are not inline. For what it's worth, I had an earlier very incomplete
stab at it, haven't looked how complete it really was; I know that it didn't
handle diagnostics as well as yours, and I have no recollection
of whether it handles the cases like the above. See attached.

[-- Attachment #2: P1094.diff --]
[-- Type: text/x-patch, Size: 2084 bytes --]

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 82b8ef8..cc5427a 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -12729,6 +12729,7 @@ cp_parser_declaration (cp_parser* parser)
                || (token2.type == CPP_OPEN_SQUARE
                    && cp_lexer_peek_nth_token (parser->lexer, 3)->type
                    == CPP_OPEN_SQUARE)
+	       || token2.keyword == RID_INLINE
 	       /* An unnamed namespace definition.  */
 	       || token2.type == CPP_OPEN_BRACE
 	       || token2.keyword == RID_ATTRIBUTE))
@@ -18533,10 +18534,24 @@ cp_parser_namespace_definition (cp_parser* parser)
   /* Parse any specified attributes before the identifier.  */
   tree attribs = cp_parser_attributes_opt (parser);
 
+  bool is_maybe_nested_inline = is_inline;
+
   for (;;)
     {
       identifier = NULL_TREE;
       
+      if (cxx_dialect > cxx17)
+	{
+	  is_maybe_nested_inline =
+	    cp_lexer_next_token_is_keyword (parser->lexer, RID_INLINE);
+
+	  if (is_maybe_nested_inline)
+	    {
+	      maybe_warn_cpp0x (CPP0X_INLINE_NAMESPACES);
+	      cp_lexer_consume_token (parser->lexer);
+	    }
+	}
+
       if (cp_lexer_next_token_is (parser->lexer, CPP_NAME))
 	{
 	  identifier = cp_parser_identifier (parser);
@@ -18555,7 +18570,8 @@ cp_parser_namespace_definition (cp_parser* parser)
 
       /* Nested namespace names can create new namespaces (unlike
 	 other qualified-ids).  */
-      if (int count = identifier ? push_namespace (identifier) : 0)
+      if (int count = identifier ?
+	  push_namespace (identifier, is_maybe_nested_inline) : 0)
 	nested_definition_count += count;
       else
 	cp_parser_error (parser, "nested namespace name required");
@@ -18573,7 +18589,8 @@ cp_parser_namespace_definition (cp_parser* parser)
 	      "a nested namespace definition cannot be inline");
 
   /* Start the namespace.  */
-  nested_definition_count += push_namespace (identifier, is_inline);
+  nested_definition_count +=
+    push_namespace (identifier, is_maybe_nested_inline);
 
   bool has_visibility = handle_namespace_attrs (current_namespace, attribs);
 

Re: C++ PATCH to implement P1094R2, Nested inline namespaces

[Marek Polacek]

On Thu, Nov 22, 2018 at 10:04:26AM +0200, Ville Voutilainen wrote:
> On Tue, Nov 20, 2018 at 04:59:46PM -0500, Jason Merrill wrote:
> > On 11/19/18 5:12 PM, Marek Polacek wrote:
> >> > +  /* Don't forget that the innermost namespace might have been
> >> > +     marked as inline.  */
> >> > +  is_inline |= nested_inline_p;
> >> This looks wrong: an inline namespace does not make its nested namespaces
> >> inline as well.
> 
> >A nested namespace definition cannot be inline.  This is supposed to handle
> >cases such as
> >namespace A::B::inline C { ... }
> >because after 'C' we don't see :: so it breaks and we call push_namespace
> >outside the for loop.  So I still don't see a bug; do you have a test that
> >I got wrong?
> 
> The way I read the question is "what does
> 
> namespace A::inline B::C::D {...}
> 
> do?".

Thanks.  This case is still handled correctly; I just checked the
push_namespace calls in gdb and just the one for B is with make_inline=true.

Arguably I should've added tests testing that some of the namespaces *aren't*
inline.

> C and D are not inline. For what it's worth, I had an earlier very incomplete
> stab at it, haven't looked how complete it really was; I know that it didn't
> handle diagnostics as well as yours, and I have no recollection
> of whether it handles the cases like the above. See attached.

I wasn't aware you had worked on this.  Perhaps we should track the progress of
C++20 features in Bugzilla (to keep track of who's working on what).

Marek

Re: C++ PATCH to implement P1094R2, Nested inline namespaces

[Ville Voutilainen]

On Fri, 23 Nov 2018 at 23:12, Marek Polacek <polacek@redhat.com> wrote:
> > >A nested namespace definition cannot be inline.  This is supposed to handle
> > >cases such as
> > >namespace A::B::inline C { ... }
> > >because after 'C' we don't see :: so it breaks and we call push_namespace
> > >outside the for loop.  So I still don't see a bug; do you have a test that
> > >I got wrong?
> >
> > The way I read the question is "what does
> >
> > namespace A::inline B::C::D {...}
> >
> > do?".
>
> Thanks.  This case is still handled correctly; I just checked the
> push_namespace calls in gdb and just the one for B is with make_inline=true.

I think that renders Jason's concern about it moot, then. :)

> > C and D are not inline. For what it's worth, I had an earlier very incomplete
> > stab at it, haven't looked how complete it really was; I know that it didn't
> > handle diagnostics as well as yours, and I have no recollection
> > of whether it handles the cases like the above. See attached.
>
> I wasn't aware you had worked on this.  Perhaps we should track the progress of
> C++20 features in Bugzilla (to keep track of who's working on what).

Maybe, but in this case it's no big deal, I cooked up a barebones
implementation because
I happened to somewhat like the proposal, so I did a quick stab to be
able to say that
a prototype exists, had anyone asked.

Re: C++ PATCH to implement P1094R2, Nested inline namespaces

[Jason Merrill]

On 11/23/18 4:12 PM, Marek Polacek wrote:
> I wasn't aware you had worked on this.  Perhaps we should track the progress of
> C++20 features in Bugzilla (to keep track of who's working on what).

Yes, I think so.  I created a couple of PRs, for contracts and cmpxchg 
with padding bits, but more are needed.

Jason

Re: C++ PATCH to implement P1094R2, Nested inline namespaces

[Jason Merrill]

On Wed, Nov 28, 2018 at 11:57 AM Marek Polacek <polacek@redhat.com> wrote:
>
> On Tue, Nov 27, 2018 at 04:01:46PM -0500, Jason Merrill wrote:
> > > > >          if (cp_lexer_next_token_is (parser->lexer, CPP_NAME))
> > > > >         {
> > > > >           identifier = cp_parser_identifier (parser);
> > > > > @@ -18904,7 +18916,12 @@ cp_parser_namespace_definition (cp_parser* parser)
> > > > >         }
> > > > >          if (cp_lexer_next_token_is_not (parser->lexer, CPP_SCOPE))
> > > > > -       break;
> > > > > +       {
> > > > > +         /* Don't forget that the innermost namespace might have been
> > > > > +            marked as inline.  */
> > > > > +         is_inline |= nested_inline_p;
> > > >
> > > > This looks wrong: an inline namespace does not make its nested namespaces
> > > > inline as well.
> >
> > > A nested namespace definition cannot be inline.  This is supposed to handle
> > > cases such as
> > > namespace A::B::inline C { ... }
> > > because after 'C' we don't see :: so it breaks and we call push_namespace
> > > outside the for loop.
> >
> > Ah, yes, I see.  I was confused by the use of '|='; what is that for? Why
> > not use '='?  For that matter, why not modify is_inline in the loop instead
> > of a new nested_inline_p variable?
>
> |= is there to handle correctly this case:
> inline namespace N { ... }
> where we have to make sure that the call to push_namespace outside the for (;;)
> is with is_inline=true.  Using '=' would overwrite is_inline to false, because
> there are no nested inlines.  For the same reason I needed a second bool: to
> not lose the information about the topmost inline.  But since the second
> push_namespace call also needs to handle the innermost namespace, it can't use
> topmost_inline_p.

Aha.  Please mention that in the comment.  OK with that change.

Jason

Re: C++ PATCH to implement P1094R2, Nested inline namespaces

[Marek Polacek]

On Tue, Nov 27, 2018 at 04:01:46PM -0500, Jason Merrill wrote:
> > > >          if (cp_lexer_next_token_is (parser->lexer, CPP_NAME))
> > > >    	{
> > > >    	  identifier = cp_parser_identifier (parser);
> > > > @@ -18904,7 +18916,12 @@ cp_parser_namespace_definition (cp_parser* parser)
> > > >    	}
> > > >          if (cp_lexer_next_token_is_not (parser->lexer, CPP_SCOPE))
> > > > -	break;
> > > > +	{
> > > > +	  /* Don't forget that the innermost namespace might have been
> > > > +	     marked as inline.  */
> > > > +	  is_inline |= nested_inline_p;
> > > 
> > > This looks wrong: an inline namespace does not make its nested namespaces
> > > inline as well.
> 
> > A nested namespace definition cannot be inline.  This is supposed to handle
> > cases such as
> > namespace A::B::inline C { ... }
> > because after 'C' we don't see :: so it breaks and we call push_namespace
> > outside the for loop.
> 
> Ah, yes, I see.  I was confused by the use of '|='; what is that for? Why
> not use '='?  For that matter, why not modify is_inline in the loop instead
> of a new nested_inline_p variable?

|= is there to handle correctly this case:
inline namespace N { ... }
where we have to make sure that the call to push_namespace outside the for (;;)
is with is_inline=true.  Using '=' would overwrite is_inline to false, because
there are no nested inlines.  For the same reason I needed a second bool: to
not lose the information about the topmost inline.  But since the second
push_namespace call also needs to handle the innermost namespace, it can't use
topmost_inline_p.

Marek

Re: C++ PATCH to implement P1094R2, Nested inline namespaces

[Jason Merrill]

On 11/21/18 8:46 PM, Marek Polacek wrote:
> On Tue, Nov 20, 2018 at 04:59:46PM -0500, Jason Merrill wrote:
>> On 11/19/18 5:12 PM, Marek Polacek wrote:
>>> On Mon, Nov 19, 2018 at 10:33:17PM +0100, Jakub Jelinek wrote:
>>>> On Mon, Nov 19, 2018 at 04:21:19PM -0500, Marek Polacek wrote:
>>>>> 2018-11-19  Marek Polacek  <polacek@redhat.com>
>>>>>
>>>>> 	Implement P1094R2, Nested inline namespaces.
>>>>> 	* g++.dg/cpp2a/nested-inline-ns1.C: New test.
>>>>> 	* g++.dg/cpp2a/nested-inline-ns2.C: New test.
>>>>> 	* g++.dg/cpp2a/nested-inline-ns3.C: New test.
>>>>
>>>> Just a small testsuite comment.
>>>>
>>>>> --- /dev/null
>>>>> +++ gcc/testsuite/g++.dg/cpp2a/nested-inline-ns1.C
>>>>> @@ -0,0 +1,26 @@
>>>>> +// P1094R2
>>>>> +// { dg-do compile { target c++2a } }
>>>>
>>>> Especially because 2a testing isn't included by default, but also
>>>> to make sure it works right even with -std=c++17, wouldn't it be better to
>>>> drop the nested-inline-ns3.C test, make this test c++17 or
>>>> even better always enabled, add dg-options "-Wpedantic" and
>>>> just add dg-warning with c++17_down and c++14_down what should be
>>>> warned on the 3 lines (with .-1 for c++14_down)?
>>>>
>>>> Or if you want add some further testcases that will test how
>>>> c++17 etc. will dg-error on those with -pedantic-errors etc.
>>>
>>> Sure, I've made it { target c++11 } and dropped the third test:
>>>
>>> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>>>
>>> 2018-11-19  Marek Polacek  <polacek@redhat.com>
>>>
>>> 	Implement P1094R2, Nested inline namespaces.
>>> 	* parser.c (cp_parser_namespace_definition): Parse the optional inline
>>> 	keyword in a nested-namespace-definition.  Adjust push_namespace call.
>>> 	Formatting fix.
>>>
>>> 	* g++.dg/cpp2a/nested-inline-ns1.C: New test.
>>> 	* g++.dg/cpp2a/nested-inline-ns2.C: New test.
>>>
>>> diff --git gcc/cp/parser.c gcc/cp/parser.c
>>> index 292cce15676..f39e9d753d2 100644
>>> --- gcc/cp/parser.c
>>> +++ gcc/cp/parser.c
>>> @@ -18872,6 +18872,7 @@ cp_parser_namespace_definition (cp_parser* parser)
>>>      cp_ensure_no_oacc_routine (parser);
>>>      bool is_inline = cp_lexer_next_token_is_keyword (parser->lexer, RID_INLINE);
>>> +  const bool topmost_inline_p = is_inline;
>>>      if (is_inline)
>>>        {
>>> @@ -18890,6 +18891,17 @@ cp_parser_namespace_definition (cp_parser* parser)
>>>        {
>>>          identifier = NULL_TREE;
>>> +      bool nested_inline_p = cp_lexer_next_token_is_keyword (parser->lexer,
>>> +							     RID_INLINE);
>>> +      if (nested_inline_p && nested_definition_count != 0)
>>> +	{
>>> +	  if (cxx_dialect < cxx2a)
>>> +	    pedwarn (cp_lexer_peek_token (parser->lexer)->location,
>>> +		     OPT_Wpedantic, "nested inline namespace definitions only "
>>> +		     "available with -std=c++2a or -std=gnu++2a");
>>> +	  cp_lexer_consume_token (parser->lexer);
>>> +	}
>>
>> This looks like we won't get any diagnostic in lower conformance modes if
>> there are multiple namespace scopes before the inline keyword.
> 
> If you mean something like
> namespace A::B:C::inline D { }
> then we do get a diagnostic.  nested-inline-ns1.C tests that.  Or do you
> mean something else?

No, this is fine then.

>>>          if (cp_lexer_next_token_is (parser->lexer, CPP_NAME))
>>>    	{
>>>    	  identifier = cp_parser_identifier (parser);
>>> @@ -18904,7 +18916,12 @@ cp_parser_namespace_definition (cp_parser* parser)
>>>    	}
>>>          if (cp_lexer_next_token_is_not (parser->lexer, CPP_SCOPE))
>>> -	break;
>>> +	{
>>> +	  /* Don't forget that the innermost namespace might have been
>>> +	     marked as inline.  */
>>> +	  is_inline |= nested_inline_p;
>>
>> This looks wrong: an inline namespace does not make its nested namespaces
>> inline as well.

> A nested namespace definition cannot be inline.  This is supposed to handle
> cases such as
> namespace A::B::inline C { ... }
> because after 'C' we don't see :: so it breaks and we call push_namespace
> outside the for loop.

Ah, yes, I see.  I was confused by the use of '|='; what is that for? 
Why not use '='?  For that matter, why not modify is_inline in the loop 
instead of a new nested_inline_p variable?

Jason

Re: C++ PATCH to implement P1094R2, Nested inline namespaces

[Marek Polacek]

On Tue, Nov 20, 2018 at 04:59:46PM -0500, Jason Merrill wrote:
> On 11/19/18 5:12 PM, Marek Polacek wrote:
> > On Mon, Nov 19, 2018 at 10:33:17PM +0100, Jakub Jelinek wrote:
> > > On Mon, Nov 19, 2018 at 04:21:19PM -0500, Marek Polacek wrote:
> > > > 2018-11-19  Marek Polacek  <polacek@redhat.com>
> > > > 
> > > > 	Implement P1094R2, Nested inline namespaces.
> > > > 	* g++.dg/cpp2a/nested-inline-ns1.C: New test.
> > > > 	* g++.dg/cpp2a/nested-inline-ns2.C: New test.
> > > > 	* g++.dg/cpp2a/nested-inline-ns3.C: New test.
> > > 
> > > Just a small testsuite comment.
> > > 
> > > > --- /dev/null
> > > > +++ gcc/testsuite/g++.dg/cpp2a/nested-inline-ns1.C
> > > > @@ -0,0 +1,26 @@
> > > > +// P1094R2
> > > > +// { dg-do compile { target c++2a } }
> > > 
> > > Especially because 2a testing isn't included by default, but also
> > > to make sure it works right even with -std=c++17, wouldn't it be better to
> > > drop the nested-inline-ns3.C test, make this test c++17 or
> > > even better always enabled, add dg-options "-Wpedantic" and
> > > just add dg-warning with c++17_down and c++14_down what should be
> > > warned on the 3 lines (with .-1 for c++14_down)?
> > > 
> > > Or if you want add some further testcases that will test how
> > > c++17 etc. will dg-error on those with -pedantic-errors etc.
> > 
> > Sure, I've made it { target c++11 } and dropped the third test:
> > 
> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> > 
> > 2018-11-19  Marek Polacek  <polacek@redhat.com>
> > 
> > 	Implement P1094R2, Nested inline namespaces.
> > 	* parser.c (cp_parser_namespace_definition): Parse the optional inline
> > 	keyword in a nested-namespace-definition.  Adjust push_namespace call.
> > 	Formatting fix.
> > 
> > 	* g++.dg/cpp2a/nested-inline-ns1.C: New test.
> > 	* g++.dg/cpp2a/nested-inline-ns2.C: New test.
> > 
> > diff --git gcc/cp/parser.c gcc/cp/parser.c
> > index 292cce15676..f39e9d753d2 100644
> > --- gcc/cp/parser.c
> > +++ gcc/cp/parser.c
> > @@ -18872,6 +18872,7 @@ cp_parser_namespace_definition (cp_parser* parser)
> >     cp_ensure_no_oacc_routine (parser);
> >     bool is_inline = cp_lexer_next_token_is_keyword (parser->lexer, RID_INLINE);
> > +  const bool topmost_inline_p = is_inline;
> >     if (is_inline)
> >       {
> > @@ -18890,6 +18891,17 @@ cp_parser_namespace_definition (cp_parser* parser)
> >       {
> >         identifier = NULL_TREE;
> > +      bool nested_inline_p = cp_lexer_next_token_is_keyword (parser->lexer,
> > +							     RID_INLINE);
> > +      if (nested_inline_p && nested_definition_count != 0)
> > +	{
> > +	  if (cxx_dialect < cxx2a)
> > +	    pedwarn (cp_lexer_peek_token (parser->lexer)->location,
> > +		     OPT_Wpedantic, "nested inline namespace definitions only "
> > +		     "available with -std=c++2a or -std=gnu++2a");
> > +	  cp_lexer_consume_token (parser->lexer);
> > +	}
> 
> This looks like we won't get any diagnostic in lower conformance modes if
> there are multiple namespace scopes before the inline keyword.

If you mean something like
namespace A::B:C::inline D { }
then we do get a diagnostic.  nested-inline-ns1.C tests that.  Or do you
mean something else?
 
> >         if (cp_lexer_next_token_is (parser->lexer, CPP_NAME))
> >   	{
> >   	  identifier = cp_parser_identifier (parser);
> > @@ -18904,7 +18916,12 @@ cp_parser_namespace_definition (cp_parser* parser)
> >   	}
> >         if (cp_lexer_next_token_is_not (parser->lexer, CPP_SCOPE))
> > -	break;
> > +	{
> > +	  /* Don't forget that the innermost namespace might have been
> > +	     marked as inline.  */
> > +	  is_inline |= nested_inline_p;
> 
> This looks wrong: an inline namespace does not make its nested namespaces
> inline as well.

A nested namespace definition cannot be inline.  This is supposed to handle
cases such as
namespace A::B::inline C { ... }
because after 'C' we don't see :: so it breaks and we call push_namespace
outside the for loop.  So I still don't see a bug; do you have a test that
I got wrong?

Marek

Re: C++ PATCH to implement P1094R2, Nested inline namespaces

[Jason Merrill]

On 11/19/18 5:12 PM, Marek Polacek wrote:
> On Mon, Nov 19, 2018 at 10:33:17PM +0100, Jakub Jelinek wrote:
>> On Mon, Nov 19, 2018 at 04:21:19PM -0500, Marek Polacek wrote:
>>> 2018-11-19  Marek Polacek  <polacek@redhat.com>
>>>
>>> 	Implement P1094R2, Nested inline namespaces.
>>> 	* g++.dg/cpp2a/nested-inline-ns1.C: New test.
>>> 	* g++.dg/cpp2a/nested-inline-ns2.C: New test.
>>> 	* g++.dg/cpp2a/nested-inline-ns3.C: New test.
>>
>> Just a small testsuite comment.
>>
>>> --- /dev/null
>>> +++ gcc/testsuite/g++.dg/cpp2a/nested-inline-ns1.C
>>> @@ -0,0 +1,26 @@
>>> +// P1094R2
>>> +// { dg-do compile { target c++2a } }
>>
>> Especially because 2a testing isn't included by default, but also
>> to make sure it works right even with -std=c++17, wouldn't it be better to
>> drop the nested-inline-ns3.C test, make this test c++17 or
>> even better always enabled, add dg-options "-Wpedantic" and
>> just add dg-warning with c++17_down and c++14_down what should be
>> warned on the 3 lines (with .-1 for c++14_down)?
>>
>> Or if you want add some further testcases that will test how
>> c++17 etc. will dg-error on those with -pedantic-errors etc.
> 
> Sure, I've made it { target c++11 } and dropped the third test:
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
> 
> 2018-11-19  Marek Polacek  <polacek@redhat.com>
> 
> 	Implement P1094R2, Nested inline namespaces.
> 	* parser.c (cp_parser_namespace_definition): Parse the optional inline
> 	keyword in a nested-namespace-definition.  Adjust push_namespace call.
> 	Formatting fix.
> 
> 	* g++.dg/cpp2a/nested-inline-ns1.C: New test.
> 	* g++.dg/cpp2a/nested-inline-ns2.C: New test.
> 
> diff --git gcc/cp/parser.c gcc/cp/parser.c
> index 292cce15676..f39e9d753d2 100644
> --- gcc/cp/parser.c
> +++ gcc/cp/parser.c
> @@ -18872,6 +18872,7 @@ cp_parser_namespace_definition (cp_parser* parser)
>     cp_ensure_no_oacc_routine (parser);
>   
>     bool is_inline = cp_lexer_next_token_is_keyword (parser->lexer, RID_INLINE);
> +  const bool topmost_inline_p = is_inline;
>   
>     if (is_inline)
>       {
> @@ -18890,6 +18891,17 @@ cp_parser_namespace_definition (cp_parser* parser)
>       {
>         identifier = NULL_TREE;
>         
> +      bool nested_inline_p = cp_lexer_next_token_is_keyword (parser->lexer,
> +							     RID_INLINE);
> +      if (nested_inline_p && nested_definition_count != 0)
> +	{
> +	  if (cxx_dialect < cxx2a)
> +	    pedwarn (cp_lexer_peek_token (parser->lexer)->location,
> +		     OPT_Wpedantic, "nested inline namespace definitions only "
> +		     "available with -std=c++2a or -std=gnu++2a");
> +	  cp_lexer_consume_token (parser->lexer);
> +	}

This looks like we won't get any diagnostic in lower conformance modes 
if there are multiple namespace scopes before the inline keyword.

>         if (cp_lexer_next_token_is (parser->lexer, CPP_NAME))
>   	{
>   	  identifier = cp_parser_identifier (parser);
> @@ -18904,7 +18916,12 @@ cp_parser_namespace_definition (cp_parser* parser)
>   	}
>   
>         if (cp_lexer_next_token_is_not (parser->lexer, CPP_SCOPE))
> -	break;
> +	{
> +	  /* Don't forget that the innermost namespace might have been
> +	     marked as inline.  */
> +	  is_inline |= nested_inline_p;

This looks wrong: an inline namespace does not make its nested 
namespaces inline as well.

Jason

Re: C++ PATCH to implement P1094R2, Nested inline namespaces

[Marek Polacek]

On Tue, Nov 20, 2018 at 10:36:32AM +0100, Jakub Jelinek wrote:
> On Tue, Nov 20, 2018 at 10:25:01AM +0100, Richard Biener wrote:
> > > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> > 
> > Just another small comment - given the usual high number of
> > C++ regressions delaying the release is Stage3 the right time
> > to add new language features?
> 
> I'd say this is small enough and worth an exception, it is just useful syntactic
> sugar, and couldn't be submitted (much) earlier as it has been voted in
> during the week when stage1 closed.

Yeah, this one is very low risk and I think it would be nice to have it in for
GCC 9 to make the C++20 support better.
FWIW, I don't plan on working on other C++20 features for this stage; I'll be
addressing C++ bugs now.

Marek

Re: C++ PATCH to implement P1094R2, Nested inline namespaces

[Jakub Jelinek]

On Tue, Nov 20, 2018 at 10:25:01AM +0100, Richard Biener wrote:
> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> 
> Just another small comment - given the usual high number of
> C++ regressions delaying the release is Stage3 the right time
> to add new language features?

I'd say this is small enough and worth an exception, it is just useful syntactic
sugar, and couldn't be submitted (much) earlier as it has been voted in
during the week when stage1 closed.

	Jakub

Re: C++ PATCH to implement P1094R2, Nested inline namespaces

[Richard Biener]

On Mon, Nov 19, 2018 at 11:12 PM Marek Polacek <polacek@redhat.com> wrote:
>
> On Mon, Nov 19, 2018 at 10:33:17PM +0100, Jakub Jelinek wrote:
> > On Mon, Nov 19, 2018 at 04:21:19PM -0500, Marek Polacek wrote:
> > > 2018-11-19  Marek Polacek  <polacek@redhat.com>
> > >
> > >     Implement P1094R2, Nested inline namespaces.
> > >     * g++.dg/cpp2a/nested-inline-ns1.C: New test.
> > >     * g++.dg/cpp2a/nested-inline-ns2.C: New test.
> > >     * g++.dg/cpp2a/nested-inline-ns3.C: New test.
> >
> > Just a small testsuite comment.
> >
> > > --- /dev/null
> > > +++ gcc/testsuite/g++.dg/cpp2a/nested-inline-ns1.C
> > > @@ -0,0 +1,26 @@
> > > +// P1094R2
> > > +// { dg-do compile { target c++2a } }
> >
> > Especially because 2a testing isn't included by default, but also
> > to make sure it works right even with -std=c++17, wouldn't it be better to
> > drop the nested-inline-ns3.C test, make this test c++17 or
> > even better always enabled, add dg-options "-Wpedantic" and
> > just add dg-warning with c++17_down and c++14_down what should be
> > warned on the 3 lines (with .-1 for c++14_down)?
> >
> > Or if you want add some further testcases that will test how
> > c++17 etc. will dg-error on those with -pedantic-errors etc.
>
> Sure, I've made it { target c++11 } and dropped the third test:
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?

Just another small comment - given the usual high number of
C++ regressions delaying the release is Stage3 the right time
to add new language features?

> 2018-11-19  Marek Polacek  <polacek@redhat.com>
>
>         Implement P1094R2, Nested inline namespaces.
>         * parser.c (cp_parser_namespace_definition): Parse the optional inline
>         keyword in a nested-namespace-definition.  Adjust push_namespace call.
>         Formatting fix.
>
>         * g++.dg/cpp2a/nested-inline-ns1.C: New test.
>         * g++.dg/cpp2a/nested-inline-ns2.C: New test.
>
> diff --git gcc/cp/parser.c gcc/cp/parser.c
> index 292cce15676..f39e9d753d2 100644
> --- gcc/cp/parser.c
> +++ gcc/cp/parser.c
> @@ -18872,6 +18872,7 @@ cp_parser_namespace_definition (cp_parser* parser)
>    cp_ensure_no_oacc_routine (parser);
>
>    bool is_inline = cp_lexer_next_token_is_keyword (parser->lexer, RID_INLINE);
> +  const bool topmost_inline_p = is_inline;
>
>    if (is_inline)
>      {
> @@ -18890,6 +18891,17 @@ cp_parser_namespace_definition (cp_parser* parser)
>      {
>        identifier = NULL_TREE;
>
> +      bool nested_inline_p = cp_lexer_next_token_is_keyword (parser->lexer,
> +                                                            RID_INLINE);
> +      if (nested_inline_p && nested_definition_count != 0)
> +       {
> +         if (cxx_dialect < cxx2a)
> +           pedwarn (cp_lexer_peek_token (parser->lexer)->location,
> +                    OPT_Wpedantic, "nested inline namespace definitions only "
> +                    "available with -std=c++2a or -std=gnu++2a");
> +         cp_lexer_consume_token (parser->lexer);
> +       }
> +
>        if (cp_lexer_next_token_is (parser->lexer, CPP_NAME))
>         {
>           identifier = cp_parser_identifier (parser);
> @@ -18904,7 +18916,12 @@ cp_parser_namespace_definition (cp_parser* parser)
>         }
>
>        if (cp_lexer_next_token_is_not (parser->lexer, CPP_SCOPE))
> -       break;
> +       {
> +         /* Don't forget that the innermost namespace might have been
> +            marked as inline.  */
> +         is_inline |= nested_inline_p;
> +         break;
> +       }
>
>        if (!nested_definition_count && cxx_dialect < cxx17)
>          pedwarn (input_location, OPT_Wpedantic,
> @@ -18913,7 +18930,9 @@ cp_parser_namespace_definition (cp_parser* parser)
>
>        /* Nested namespace names can create new namespaces (unlike
>          other qualified-ids).  */
> -      if (int count = identifier ? push_namespace (identifier) : 0)
> +      if (int count = (identifier
> +                      ? push_namespace (identifier, nested_inline_p)
> +                      : 0))
>         nested_definition_count += count;
>        else
>         cp_parser_error (parser, "nested namespace name required");
> @@ -18926,7 +18945,7 @@ cp_parser_namespace_definition (cp_parser* parser)
>    if (nested_definition_count && attribs)
>      error_at (token->location,
>               "a nested namespace definition cannot have attributes");
> -  if (nested_definition_count && is_inline)
> +  if (nested_definition_count && topmost_inline_p)
>      error_at (token->location,
>               "a nested namespace definition cannot be inline");
>
> @@ -18935,7 +18954,7 @@ cp_parser_namespace_definition (cp_parser* parser)
>
>    bool has_visibility = handle_namespace_attrs (current_namespace, attribs);
>
> -  warning  (OPT_Wnamespaces, "namespace %qD entered", current_namespace);
> +  warning (OPT_Wnamespaces, "namespace %qD entered", current_namespace);
>
>    /* Look for the `{' to validate starting the namespace.  */
>    matching_braces braces;
> diff --git gcc/testsuite/g++.dg/cpp2a/nested-inline-ns1.C gcc/testsuite/g++.dg/cpp2a/nested-inline-ns1.C
> new file mode 100644
> index 00000000000..8c9573ea5db
> --- /dev/null
> +++ gcc/testsuite/g++.dg/cpp2a/nested-inline-ns1.C
> @@ -0,0 +1,29 @@
> +// P1094R2
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-Wpedantic" }
> +
> +namespace A::inline B::C { // { dg-warning "nested inline namespace definitions only" "" { target c++17_down } }
> +// { dg-warning "nested namespace definitions only available" "" { target c++14_down } .-1 }
> +  int i;
> +}
> +
> +namespace D::E::inline F { // { dg-warning "nested inline namespace definitions only" "" { target c++17_down } }
> +// { dg-warning "nested namespace definitions only available" "" { target c++14_down } .-1 }
> +  int j;
> +}
> +
> +inline namespace X {
> +  int x;
> +}
> +
> +// Make sure the namespaces are marked inline.
> +void
> +g ()
> +{
> +  A::B::C::i++;
> +  A::C::i++;
> +  D::E::j++;
> +  D::E::F::j++;
> +  X::x++;
> +  x++;
> +}
> diff --git gcc/testsuite/g++.dg/cpp2a/nested-inline-ns2.C gcc/testsuite/g++.dg/cpp2a/nested-inline-ns2.C
> new file mode 100644
> index 00000000000..9b5f2cab47b
> --- /dev/null
> +++ gcc/testsuite/g++.dg/cpp2a/nested-inline-ns2.C
> @@ -0,0 +1,26 @@
> +// P1094R2
> +// { dg-do compile { target c++2a } }
> +
> +inline namespace A::B { // { dg-error "a nested namespace definition cannot be inline" }
> +  int i;
> +}
> +
> +namespace inline C::D { // { dg-error "expected|does not name a type" }
> +  int i;
> +}
> +
> +namespace E::F inline { // { dg-error "expected" }
> +  int i;
> +}
> +
> +namespace inline G { // { dg-error "expected|does not name a type" }
> +  int i;
> +}
> +
> +inline namespace inline H { // { dg-error "expected|does not name a type" }
> +  int i;
> +}
> +
> +inline namespace inline I::J { // { dg-error "expected|does not name a type" }
> +  int i;
> +}

Re: C++ PATCH to implement P1094R2, Nested inline namespaces

[Marek Polacek]

On Mon, Nov 19, 2018 at 10:33:17PM +0100, Jakub Jelinek wrote:
> On Mon, Nov 19, 2018 at 04:21:19PM -0500, Marek Polacek wrote:
> > 2018-11-19  Marek Polacek  <polacek@redhat.com>
> > 
> > 	Implement P1094R2, Nested inline namespaces.
> > 	* g++.dg/cpp2a/nested-inline-ns1.C: New test.
> > 	* g++.dg/cpp2a/nested-inline-ns2.C: New test.
> > 	* g++.dg/cpp2a/nested-inline-ns3.C: New test.
> 
> Just a small testsuite comment.
> 
> > --- /dev/null
> > +++ gcc/testsuite/g++.dg/cpp2a/nested-inline-ns1.C
> > @@ -0,0 +1,26 @@
> > +// P1094R2
> > +// { dg-do compile { target c++2a } }
> 
> Especially because 2a testing isn't included by default, but also
> to make sure it works right even with -std=c++17, wouldn't it be better to
> drop the nested-inline-ns3.C test, make this test c++17 or
> even better always enabled, add dg-options "-Wpedantic" and
> just add dg-warning with c++17_down and c++14_down what should be
> warned on the 3 lines (with .-1 for c++14_down)?
> 
> Or if you want add some further testcases that will test how
> c++17 etc. will dg-error on those with -pedantic-errors etc.

Sure, I've made it { target c++11 } and dropped the third test:

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

2018-11-19  Marek Polacek  <polacek@redhat.com>

	Implement P1094R2, Nested inline namespaces.
	* parser.c (cp_parser_namespace_definition): Parse the optional inline
	keyword in a nested-namespace-definition.  Adjust push_namespace call.
	Formatting fix.

	* g++.dg/cpp2a/nested-inline-ns1.C: New test.
	* g++.dg/cpp2a/nested-inline-ns2.C: New test.

diff --git gcc/cp/parser.c gcc/cp/parser.c
index 292cce15676..f39e9d753d2 100644
--- gcc/cp/parser.c
+++ gcc/cp/parser.c
@@ -18872,6 +18872,7 @@ cp_parser_namespace_definition (cp_parser* parser)
   cp_ensure_no_oacc_routine (parser);
 
   bool is_inline = cp_lexer_next_token_is_keyword (parser->lexer, RID_INLINE);
+  const bool topmost_inline_p = is_inline;
 
   if (is_inline)
     {
@@ -18890,6 +18891,17 @@ cp_parser_namespace_definition (cp_parser* parser)
     {
       identifier = NULL_TREE;
       
+      bool nested_inline_p = cp_lexer_next_token_is_keyword (parser->lexer,
+							     RID_INLINE);
+      if (nested_inline_p && nested_definition_count != 0)
+	{
+	  if (cxx_dialect < cxx2a)
+	    pedwarn (cp_lexer_peek_token (parser->lexer)->location,
+		     OPT_Wpedantic, "nested inline namespace definitions only "
+		     "available with -std=c++2a or -std=gnu++2a");
+	  cp_lexer_consume_token (parser->lexer);
+	}
+
       if (cp_lexer_next_token_is (parser->lexer, CPP_NAME))
 	{
 	  identifier = cp_parser_identifier (parser);
@@ -18904,7 +18916,12 @@ cp_parser_namespace_definition (cp_parser* parser)
 	}
 
       if (cp_lexer_next_token_is_not (parser->lexer, CPP_SCOPE))
-	break;
+	{
+	  /* Don't forget that the innermost namespace might have been
+	     marked as inline.  */
+	  is_inline |= nested_inline_p;
+	  break;
+	}
   
       if (!nested_definition_count && cxx_dialect < cxx17)
         pedwarn (input_location, OPT_Wpedantic,
@@ -18913,7 +18930,9 @@ cp_parser_namespace_definition (cp_parser* parser)
 
       /* Nested namespace names can create new namespaces (unlike
 	 other qualified-ids).  */
-      if (int count = identifier ? push_namespace (identifier) : 0)
+      if (int count = (identifier
+		       ? push_namespace (identifier, nested_inline_p)
+		       : 0))
 	nested_definition_count += count;
       else
 	cp_parser_error (parser, "nested namespace name required");
@@ -18926,7 +18945,7 @@ cp_parser_namespace_definition (cp_parser* parser)
   if (nested_definition_count && attribs)
     error_at (token->location,
 	      "a nested namespace definition cannot have attributes");
-  if (nested_definition_count && is_inline)
+  if (nested_definition_count && topmost_inline_p)
     error_at (token->location,
 	      "a nested namespace definition cannot be inline");
 
@@ -18935,7 +18954,7 @@ cp_parser_namespace_definition (cp_parser* parser)
 
   bool has_visibility = handle_namespace_attrs (current_namespace, attribs);
 
-  warning  (OPT_Wnamespaces, "namespace %qD entered", current_namespace);
+  warning (OPT_Wnamespaces, "namespace %qD entered", current_namespace);
 
   /* Look for the `{' to validate starting the namespace.  */
   matching_braces braces;
diff --git gcc/testsuite/g++.dg/cpp2a/nested-inline-ns1.C gcc/testsuite/g++.dg/cpp2a/nested-inline-ns1.C
new file mode 100644
index 00000000000..8c9573ea5db
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp2a/nested-inline-ns1.C
@@ -0,0 +1,29 @@
+// P1094R2
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wpedantic" }
+
+namespace A::inline B::C { // { dg-warning "nested inline namespace definitions only" "" { target c++17_down } }
+// { dg-warning "nested namespace definitions only available" "" { target c++14_down } .-1 }
+  int i;
+}
+
+namespace D::E::inline F { // { dg-warning "nested inline namespace definitions only" "" { target c++17_down } }
+// { dg-warning "nested namespace definitions only available" "" { target c++14_down } .-1 }
+  int j;
+}
+
+inline namespace X {
+  int x;
+}
+
+// Make sure the namespaces are marked inline.
+void
+g ()
+{
+  A::B::C::i++;
+  A::C::i++;
+  D::E::j++;
+  D::E::F::j++;
+  X::x++;
+  x++;
+}
diff --git gcc/testsuite/g++.dg/cpp2a/nested-inline-ns2.C gcc/testsuite/g++.dg/cpp2a/nested-inline-ns2.C
new file mode 100644
index 00000000000..9b5f2cab47b
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp2a/nested-inline-ns2.C
@@ -0,0 +1,26 @@
+// P1094R2
+// { dg-do compile { target c++2a } }
+
+inline namespace A::B { // { dg-error "a nested namespace definition cannot be inline" }
+  int i;
+}
+
+namespace inline C::D { // { dg-error "expected|does not name a type" }
+  int i;
+}
+
+namespace E::F inline { // { dg-error "expected" }
+  int i;
+}
+
+namespace inline G { // { dg-error "expected|does not name a type" }
+  int i;
+}
+
+inline namespace inline H { // { dg-error "expected|does not name a type" }
+  int i;
+}
+
+inline namespace inline I::J { // { dg-error "expected|does not name a type" }
+  int i;
+}

Re: C++ PATCH to implement P1094R2, Nested inline namespaces

[Jakub Jelinek]

On Mon, Nov 19, 2018 at 04:21:19PM -0500, Marek Polacek wrote:
> 2018-11-19  Marek Polacek  <polacek@redhat.com>
> 
> 	Implement P1094R2, Nested inline namespaces.
> 	* g++.dg/cpp2a/nested-inline-ns1.C: New test.
> 	* g++.dg/cpp2a/nested-inline-ns2.C: New test.
> 	* g++.dg/cpp2a/nested-inline-ns3.C: New test.

Just a small testsuite comment.

> --- /dev/null
> +++ gcc/testsuite/g++.dg/cpp2a/nested-inline-ns1.C
> @@ -0,0 +1,26 @@
> +// P1094R2
> +// { dg-do compile { target c++2a } }

Especially because 2a testing isn't included by default, but also
to make sure it works right even with -std=c++17, wouldn't it be better to
drop the nested-inline-ns3.C test, make this test c++17 or
even better always enabled, add dg-options "-Wpedantic" and
just add dg-warning with c++17_down and c++14_down what should be
warned on the 3 lines (with .-1 for c++14_down)?

Or if you want add some further testcases that will test how
c++17 etc. will dg-error on those with -pedantic-errors etc.

> +
> +namespace A::inline B::C {
> +  int i;
> +}
> +
> +namespace D::E::inline F {
> +  int j;
> +}
> +
> +inline namespace X {
> +  int x;
> +}

> +// Make sure the namespaces are marked inline.
> +void
> +g ()
> +{
> +  A::B::C::i++;
> +  A::C::i++;
> +  D::E::j++;
> +  D::E::F::j++;
> +  X::x++;
> +  x++;
> +}

	Jakub

C++ PATCH to implement P1094R2, Nested inline namespaces

[Marek Polacek]

This patch implements another C++20 feature, nested inline namespaces.

This was fairly simple, one just has to be careful not to blithely consume the
inline keyword, making a non-valid program valid.  Another minor gotcha was
to handle the innermost 'inline' correctly.

Note that 

  inline namespace A::B { ... }

continues to be invalid.

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

2018-11-19  Marek Polacek  <polacek@redhat.com>

	Implement P1094R2, Nested inline namespaces.
	* parser.c (cp_parser_namespace_definition): Parse the optional inline
	keyword in a nested-namespace-definition.  Adjust push_namespace call.
	Formatting fix.

	* g++.dg/cpp2a/nested-inline-ns1.C: New test.
	* g++.dg/cpp2a/nested-inline-ns2.C: New test.
	* g++.dg/cpp2a/nested-inline-ns3.C: New test.

diff --git gcc/cp/parser.c gcc/cp/parser.c
index 88fc426102b..8b8304acca7 100644
--- gcc/cp/parser.c
+++ gcc/cp/parser.c
@@ -18864,6 +18864,7 @@ cp_parser_namespace_definition (cp_parser* parser)
   cp_ensure_no_oacc_routine (parser);
 
   bool is_inline = cp_lexer_next_token_is_keyword (parser->lexer, RID_INLINE);
+  const bool topmost_inline_p = is_inline;
 
   if (is_inline)
     {
@@ -18882,6 +18883,17 @@ cp_parser_namespace_definition (cp_parser* parser)
     {
       identifier = NULL_TREE;
       
+      bool nested_inline_p = cp_lexer_next_token_is_keyword (parser->lexer,
+							     RID_INLINE);
+      if (nested_inline_p && nested_definition_count != 0)
+	{
+	  if (cxx_dialect < cxx2a)
+	    pedwarn (cp_lexer_peek_token (parser->lexer)->location,
+		     OPT_Wpedantic, "nested inline namespace definitions only "
+		     "available with -std=c++2a or -std=gnu++2a");
+	  cp_lexer_consume_token (parser->lexer);
+	}
+
       if (cp_lexer_next_token_is (parser->lexer, CPP_NAME))
 	{
 	  identifier = cp_parser_identifier (parser);
@@ -18896,7 +18908,12 @@ cp_parser_namespace_definition (cp_parser* parser)
 	}
 
       if (cp_lexer_next_token_is_not (parser->lexer, CPP_SCOPE))
-	break;
+	{
+	  /* Don't forget that the innermost namespace might have been
+	     marked as inline.  */
+	  is_inline |= nested_inline_p;
+	  break;
+	}
   
       if (!nested_definition_count && cxx_dialect < cxx17)
         pedwarn (input_location, OPT_Wpedantic,
@@ -18905,7 +18922,9 @@ cp_parser_namespace_definition (cp_parser* parser)
 
       /* Nested namespace names can create new namespaces (unlike
 	 other qualified-ids).  */
-      if (int count = identifier ? push_namespace (identifier) : 0)
+      if (int count = (identifier
+		       ? push_namespace (identifier, nested_inline_p)
+		       : 0))
 	nested_definition_count += count;
       else
 	cp_parser_error (parser, "nested namespace name required");
@@ -18918,7 +18937,7 @@ cp_parser_namespace_definition (cp_parser* parser)
   if (nested_definition_count && attribs)
     error_at (token->location,
 	      "a nested namespace definition cannot have attributes");
-  if (nested_definition_count && is_inline)
+  if (nested_definition_count && topmost_inline_p)
     error_at (token->location,
 	      "a nested namespace definition cannot be inline");
 
@@ -18927,7 +18946,7 @@ cp_parser_namespace_definition (cp_parser* parser)
 
   bool has_visibility = handle_namespace_attrs (current_namespace, attribs);
 
-  warning  (OPT_Wnamespaces, "namespace %qD entered", current_namespace);
+  warning (OPT_Wnamespaces, "namespace %qD entered", current_namespace);
 
   /* Look for the `{' to validate starting the namespace.  */
   matching_braces braces;
diff --git gcc/testsuite/g++.dg/cpp2a/nested-inline-ns1.C gcc/testsuite/g++.dg/cpp2a/nested-inline-ns1.C
new file mode 100644
index 00000000000..95b4d3378d1
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp2a/nested-inline-ns1.C
@@ -0,0 +1,26 @@
+// P1094R2
+// { dg-do compile { target c++2a } }
+
+namespace A::inline B::C {
+  int i;
+}
+
+namespace D::E::inline F {
+  int j;
+}
+
+inline namespace X {
+  int x;
+}
+
+// Make sure the namespaces are marked inline.
+void
+g ()
+{
+  A::B::C::i++;
+  A::C::i++;
+  D::E::j++;
+  D::E::F::j++;
+  X::x++;
+  x++;
+}
diff --git gcc/testsuite/g++.dg/cpp2a/nested-inline-ns2.C gcc/testsuite/g++.dg/cpp2a/nested-inline-ns2.C
new file mode 100644
index 00000000000..9b5f2cab47b
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp2a/nested-inline-ns2.C
@@ -0,0 +1,26 @@
+// P1094R2
+// { dg-do compile { target c++2a } }
+
+inline namespace A::B { // { dg-error "a nested namespace definition cannot be inline" }
+  int i;
+}
+
+namespace inline C::D { // { dg-error "expected|does not name a type" }
+  int i;
+}
+
+namespace E::F inline { // { dg-error "expected" }
+  int i;
+}
+
+namespace inline G { // { dg-error "expected|does not name a type" }
+  int i;
+}
+
+inline namespace inline H { // { dg-error "expected|does not name a type" }
+  int i;
+}
+
+inline namespace inline I::J { // { dg-error "expected|does not name a type" }
+  int i;
+}
diff --git gcc/testsuite/g++.dg/cpp2a/nested-inline-ns3.C gcc/testsuite/g++.dg/cpp2a/nested-inline-ns3.C
new file mode 100644
index 00000000000..8b81c1383db
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp2a/nested-inline-ns3.C
@@ -0,0 +1,11 @@
+// P1094R2
+// { dg-do compile { target c++17 } }
+// { dg-options "-Wpedantic" }
+
+namespace A::inline B::C { // { dg-warning "nested inline namespace definitions only" "" { target c++17_down } }
+  int i;
+}
+
+namespace D::E::inline F { // { dg-warning "nested inline namespace definitions only" "" { target c++17_down } }
+  int i;
+}