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  <pola...@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  <pola...@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