[PATCH] PR c++/54401 - Confusing diagnostics about type-alias at class scope

[PATCH] PR c++/54401 - Confusing diagnostics about type-alias at class scope

[Dodji Seketeli]

Hello,

Consider this invalid example given in the PR, where T is not defined:

     1	template<typename>
     2	struct X {
     3	    using type = T;
     4	};

g++ yields the confusing diagnostics:

test.cc:3:10: error: expected nested-name-specifier before 'type'
    using type = T;
          ^
test.cc:3:10: error: using-declaration for non-member at class scope
test.cc:3:15: error: expected ';' before '=' token
    using type = T;
               ^
test.cc:3:15: error: expected unqualified-id before '=' token

I think this is because in cp_parser_member_declaration we tentatively
parse an alias declaration; we then have a somewhat meaningful
diagnostic which alas is not emitted because we are parsing
tentatively.  As the parsing didn't succeed (because the input is
invalid) we try to parse a using declaration, which fails as well; but
then the diagnostic emitted is the one for the failed attempt at
parsing a using declaration, not an alias declaration.  Oops.

The idea of this patch is to detect in advance that we want to parse
an alias declaration, parse it non-tentatively, and then if an error
arises, emit it.

I also changed cp_parser_alias_declaration to get out directly when it
detects that the type-id is invalid, rather than going on nonetheless
and emitting more (irrelevant) error diagnostics.

We are now getting the following output:

    test.cc:3:18: erreur: expected type-specifier before ‘T’
	 using type = T;
		      ^
    test.cc:3:18: erreur: ‘T’ does not name a type

I don't really like the "before 'T'" there, but I think we maybe could
revisit the format of what cp_parser_error emits in general, now that
we have caret diagnostics;  We could maybe do away with the "before T"
altogether?

In the mean time, it seems to me that this patch brings an improvement
over what we already have in trunk, and the issue above could be
addressed separately.

Tested on x86_64-unknown-linux-gnu against trunk.

gcc/cp/

	* parser.c (cp_parser_expecting_alias_declaration_p): New static
	function.
	(cp_parser_block_declaration): Use it.
	(cp_parser_member_declaration): Likewise.  Don't parse the using
	declaration tentatively.
	(cp_parser_alias_declaration): Get out if the type-id is invalid.

gcc/testsuite/

	* g++.dg/cpp0x/alias-decl-23.C: New test.
---
 gcc/cp/parser.c                            | 38 +++++++++++++++++++++++-------
 gcc/testsuite/g++.dg/cpp0x/alias-decl-23.C |  7 ++++++
 2 files changed, 36 insertions(+), 9 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/alias-decl-23.C

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index e8c0378..cab2d09 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -1937,6 +1937,8 @@ static bool cp_parser_using_declaration
   (cp_parser *, bool);
 static void cp_parser_using_directive
   (cp_parser *);
+static bool cp_parser_expecting_alias_declaration_p
+  (cp_parser*);
 static tree cp_parser_alias_declaration
   (cp_parser *);
 static void cp_parser_asm_definition
@@ -10292,11 +10294,7 @@ cp_parser_block_declaration (cp_parser *parser,
 	cp_parser_using_directive (parser);
       /* If the second token after 'using' is '=', then we have an
 	 alias-declaration.  */
-      else if (cxx_dialect >= cxx0x
-	       && token2->type == CPP_NAME
-	       && ((cp_lexer_peek_nth_token (parser->lexer, 3)->type == CPP_EQ)
-		   || (cp_lexer_peek_nth_token (parser->lexer, 3)->keyword
-		       == RID_ATTRIBUTE)))
+      else if (cp_parser_expecting_alias_declaration_p (parser))
 	cp_parser_alias_declaration (parser);
       /* Otherwise, it's a using-declaration.  */
       else
@@ -15079,6 +15077,24 @@ cp_parser_using_declaration (cp_parser* parser,
   return true;
 }
 
+/* Return TRUE if the coming tokens reasonably denote the beginning of
+   an alias declaration.  */
+
+static bool
+cp_parser_expecting_alias_declaration_p (cp_parser* parser)
+{
+  if (cxx_dialect < cxx0x)
+    return false;
+  cp_parser_parse_tentatively (parser);
+  cp_parser_require_keyword (parser, RID_USING, RT_USING);
+  cp_parser_identifier (parser);
+  cp_parser_attributes_opt (parser);
+  cp_parser_require (parser, CPP_EQ, RT_EQ);
+  bool is_ok = !cp_parser_error_occurred (parser);
+  cp_parser_abort_tentative_parse (parser);
+  return is_ok;
+}
+
 /* Parse an alias-declaration.
 
    alias-declaration:
@@ -15141,6 +15157,9 @@ cp_parser_alias_declaration (cp_parser* parser)
   if (parser->num_template_parameter_lists)
     parser->type_definition_forbidden_message = saved_message;
 
+  if (type == error_mark_node)
+    return error_mark_node;
+
   cp_parser_require (parser, CPP_SEMICOLON, RT_SEMICOLON);
 
   if (cp_parser_error_occurred (parser))
@@ -18849,10 +18868,11 @@ cp_parser_member_declaration (cp_parser* parser)
       else
 	{
 	  tree decl;
-	  cp_parser_parse_tentatively (parser);
-	  decl = cp_parser_alias_declaration (parser);
-	  if (cp_parser_parse_definitely (parser))
-	    finish_member_declaration (decl);
+	  if (cp_parser_expecting_alias_declaration_p (parser))
+	    {
+	      decl = cp_parser_alias_declaration (parser);
+	      finish_member_declaration (decl);
+	    }
 	  else
 	    cp_parser_using_declaration (parser,
 					 /*access_declaration_p=*/false);
diff --git a/gcc/testsuite/g++.dg/cpp0x/alias-decl-23.C b/gcc/testsuite/g++.dg/cpp0x/alias-decl-23.C
new file mode 100644
index 0000000..086b5e5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/alias-decl-23.C
@@ -0,0 +1,7 @@
+// Origin: PR c++/54401
+// { dg-do compile { target c++11 } }
+
+template<typename>
+struct X {
+    using type = T; // { dg-error "expected type-specifier|does not name a type" }
+};
-- 
1.7.11.4


-- 
		Dodji

[PING] [PATCH] PR c++/54401 - Confusing diagnostics about type-alias at class scope

[Dodji Seketeli]

I am friendly pinging the patch below ...

Dodji Seketeli <dodji@redhat.com> a écrit:

> Hello,
>
> Consider this invalid example given in the PR, where T is not defined:
>
>      1	template<typename>
>      2	struct X {
>      3	    using type = T;
>      4	};
>
> g++ yields the confusing diagnostics:
>
> test.cc:3:10: error: expected nested-name-specifier before 'type'
>     using type = T;
>           ^
> test.cc:3:10: error: using-declaration for non-member at class scope
> test.cc:3:15: error: expected ';' before '=' token
>     using type = T;
>                ^
> test.cc:3:15: error: expected unqualified-id before '=' token
>
> I think this is because in cp_parser_member_declaration we tentatively
> parse an alias declaration; we then have a somewhat meaningful
> diagnostic which alas is not emitted because we are parsing
> tentatively.  As the parsing didn't succeed (because the input is
> invalid) we try to parse a using declaration, which fails as well; but
> then the diagnostic emitted is the one for the failed attempt at
> parsing a using declaration, not an alias declaration.  Oops.
>
> The idea of this patch is to detect in advance that we want to parse
> an alias declaration, parse it non-tentatively, and then if an error
> arises, emit it.
>
> I also changed cp_parser_alias_declaration to get out directly when it
> detects that the type-id is invalid, rather than going on nonetheless
> and emitting more (irrelevant) error diagnostics.
>
> We are now getting the following output:
>
>     test.cc:3:18: erreur: expected type-specifier before ‘T’
> 	 using type = T;
> 		      ^
>     test.cc:3:18: erreur: ‘T’ does not name a type
>
> I don't really like the "before 'T'" there, but I think we maybe could
> revisit the format of what cp_parser_error emits in general, now that
> we have caret diagnostics;  We could maybe do away with the "before T"
> altogether?
>
> In the mean time, it seems to me that this patch brings an improvement
> over what we already have in trunk, and the issue above could be
> addressed separately.
>
> Tested on x86_64-unknown-linux-gnu against trunk.
>
> gcc/cp/
>
> 	* parser.c (cp_parser_expecting_alias_declaration_p): New static
> 	function.
> 	(cp_parser_block_declaration): Use it.
> 	(cp_parser_member_declaration): Likewise.  Don't parse the using
> 	declaration tentatively.
> 	(cp_parser_alias_declaration): Get out if the type-id is invalid.
>
> gcc/testsuite/
>
> 	* g++.dg/cpp0x/alias-decl-23.C: New test.
> ---
>  gcc/cp/parser.c                            | 38 +++++++++++++++++++++++-------
>  gcc/testsuite/g++.dg/cpp0x/alias-decl-23.C |  7 ++++++
>  2 files changed, 36 insertions(+), 9 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/cpp0x/alias-decl-23.C
>
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index e8c0378..cab2d09 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -1937,6 +1937,8 @@ static bool cp_parser_using_declaration
>    (cp_parser *, bool);
>  static void cp_parser_using_directive
>    (cp_parser *);
> +static bool cp_parser_expecting_alias_declaration_p
> +  (cp_parser*);
>  static tree cp_parser_alias_declaration
>    (cp_parser *);
>  static void cp_parser_asm_definition
> @@ -10292,11 +10294,7 @@ cp_parser_block_declaration (cp_parser *parser,
>  	cp_parser_using_directive (parser);
>        /* If the second token after 'using' is '=', then we have an
>  	 alias-declaration.  */
> -      else if (cxx_dialect >= cxx0x
> -	       && token2->type == CPP_NAME
> -	       && ((cp_lexer_peek_nth_token (parser->lexer, 3)->type == CPP_EQ)
> -		   || (cp_lexer_peek_nth_token (parser->lexer, 3)->keyword
> -		       == RID_ATTRIBUTE)))
> +      else if (cp_parser_expecting_alias_declaration_p (parser))
>  	cp_parser_alias_declaration (parser);
>        /* Otherwise, it's a using-declaration.  */
>        else
> @@ -15079,6 +15077,24 @@ cp_parser_using_declaration (cp_parser* parser,
>    return true;
>  }
>  
> +/* Return TRUE if the coming tokens reasonably denote the beginning of
> +   an alias declaration.  */
> +
> +static bool
> +cp_parser_expecting_alias_declaration_p (cp_parser* parser)
> +{
> +  if (cxx_dialect < cxx0x)
> +    return false;
> +  cp_parser_parse_tentatively (parser);
> +  cp_parser_require_keyword (parser, RID_USING, RT_USING);
> +  cp_parser_identifier (parser);
> +  cp_parser_attributes_opt (parser);
> +  cp_parser_require (parser, CPP_EQ, RT_EQ);
> +  bool is_ok = !cp_parser_error_occurred (parser);
> +  cp_parser_abort_tentative_parse (parser);
> +  return is_ok;
> +}
> +
>  /* Parse an alias-declaration.
>  
>     alias-declaration:
> @@ -15141,6 +15157,9 @@ cp_parser_alias_declaration (cp_parser* parser)
>    if (parser->num_template_parameter_lists)
>      parser->type_definition_forbidden_message = saved_message;
>  
> +  if (type == error_mark_node)
> +    return error_mark_node;
> +
>    cp_parser_require (parser, CPP_SEMICOLON, RT_SEMICOLON);
>  
>    if (cp_parser_error_occurred (parser))
> @@ -18849,10 +18868,11 @@ cp_parser_member_declaration (cp_parser* parser)
>        else
>  	{
>  	  tree decl;
> -	  cp_parser_parse_tentatively (parser);
> -	  decl = cp_parser_alias_declaration (parser);
> -	  if (cp_parser_parse_definitely (parser))
> -	    finish_member_declaration (decl);
> +	  if (cp_parser_expecting_alias_declaration_p (parser))
> +	    {
> +	      decl = cp_parser_alias_declaration (parser);
> +	      finish_member_declaration (decl);
> +	    }
>  	  else
>  	    cp_parser_using_declaration (parser,
>  					 /*access_declaration_p=*/false);
> diff --git a/gcc/testsuite/g++.dg/cpp0x/alias-decl-23.C b/gcc/testsuite/g++.dg/cpp0x/alias-decl-23.C
> new file mode 100644
> index 0000000..086b5e5
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/alias-decl-23.C
> @@ -0,0 +1,7 @@
> +// Origin: PR c++/54401
> +// { dg-do compile { target c++11 } }
> +
> +template<typename>
> +struct X {
> +    using type = T; // { dg-error "expected type-specifier|does not name a type" }
> +};
> -- 
> 1.7.11.4

-- 
		Dodji

Re: [PING] [PATCH] PR c++/54401 - Confusing diagnostics about type-alias at class scope

[Jason Merrill]

Would it work to just do a cp_parser_commit_to_tentative_parse after we 
see the '='?

Jason

Re: [PING] [PATCH] PR c++/54401 - Confusing diagnostics about type-alias at class scope

[Gabriel Dos Reis]

On Fri, Nov 16, 2012 at 8:51 AM, Jason Merrill <jason@redhat.com> wrote:
> Would it work to just do a cp_parser_commit_to_tentative_parse after we see
> the '='?

I like that solution better.

-- Gaby

Re: [PING] [PATCH] PR c++/54401 - Confusing diagnostics about type-alias at class scope

[Dodji Seketeli]

Gabriel Dos Reis <gdr@integrable-solutions.net> a écrit:

> On Fri, Nov 16, 2012 at 8:51 AM, Jason Merrill <jason@redhat.com> wrote:
>> Would it work to just do a cp_parser_commit_to_tentative_parse after we see
>> the '='?

I guess you mean in cp_parser_alias_declaration like in the updated
patch below?

> I like that solution better.

Ok, thanks.
 
I just had to adjust the call to cp_parser_alias_declaration when called
in  the context of a tentative parse, to handle the case where an error
occurs before we reach the '=' token; which means that the tentative
parse can still be cancelled.

Bootstrapped and tested on x86_64-unknown-linux-gnu against trunk.

gcc/cp/

	* parser.c (cp_parser_alias_declaration): Commit to tentative
	parse when see the '=' token.  Get out if the type-id is invalid.
	Update function comment.
	(cp_parser_member_declaration): Don't try to parse a using
	declaration if we know that we expected an alias declaration; that
	is, if we see the '=' token after the identifier.

gcc/testsuite/

	* g++.dg/cpp0x/alias-decl-28.C: New test.
	* g++.dg/cpp0x/alias-decl-16.C: Update.
---
 gcc/cp/parser.c                            | 23 +++++++++++++++++++++--
 gcc/testsuite/g++.dg/cpp0x/alias-decl-16.C |  2 +-
 gcc/testsuite/g++.dg/cpp0x/alias-decl-28.C |  7 +++++++
 3 files changed, 29 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/alias-decl-28.C

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 7107134..2b76670 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -15260,7 +15260,11 @@ cp_parser_using_declaration (cp_parser* parser,
 /* Parse an alias-declaration.
 
    alias-declaration:
-     using identifier attribute-specifier-seq [opt] = type-id  */
+     using identifier attribute-specifier-seq [opt] = type-id
+
+   Note that if this function is used in the context of a tentative
+   parse, it commits the currently active tentative parse after it
+   sees the '=' token.  */
 
 static tree
 cp_parser_alias_declaration (cp_parser* parser)
@@ -15293,6 +15297,8 @@ cp_parser_alias_declaration (cp_parser* parser)
   if (cp_parser_error_occurred (parser))
     return error_mark_node;
 
+  cp_parser_commit_to_tentative_parse (parser);
+
   /* Now we are going to parse the type-id of the declaration.  */
 
   /*
@@ -15322,6 +15328,9 @@ cp_parser_alias_declaration (cp_parser* parser)
   if (parser->num_template_parameter_lists)
     parser->type_definition_forbidden_message = saved_message;
 
+  if (type == error_mark_node)
+    return error_mark_node;
+
   cp_parser_require (parser, CPP_SEMICOLON, RT_SEMICOLON);
 
   if (cp_parser_error_occurred (parser))
@@ -19058,9 +19067,19 @@ cp_parser_member_declaration (cp_parser* parser)
       else
 	{
 	  tree decl;
+	  bool alias_decl_expected;
 	  cp_parser_parse_tentatively (parser);
 	  decl = cp_parser_alias_declaration (parser);
-	  if (cp_parser_parse_definitely (parser))
+	  /* Note that if we actually see the '=' token after the
+	     identifier, cp_parser_alias_declaration commits the
+	     tentative parse.  In that case, we really expects an
+	     alias-declaration.  Otherwise, we expect a using
+	     declaration.  */
+	  alias_decl_expected =
+	    !cp_parser_uncommitted_to_tentative_parse_p (parser);
+	  cp_parser_parse_definitely (parser);
+
+	  if (alias_decl_expected)
 	    finish_member_declaration (decl);
 	  else
 	    cp_parser_using_declaration (parser,
diff --git a/gcc/testsuite/g++.dg/cpp0x/alias-decl-16.C b/gcc/testsuite/g++.dg/cpp0x/alias-decl-16.C
index d66660a..ce6ad0a 100644
--- a/gcc/testsuite/g++.dg/cpp0x/alias-decl-16.C
+++ b/gcc/testsuite/g++.dg/cpp0x/alias-decl-16.C
@@ -23,6 +23,6 @@ template<class T>
 using A3 =
     enum B3 {b = 0;}; //{ dg-error "types may not be defined in alias template" }
 
-A3<int> a3;
+A3<int> a3; // { dg-error "'A3' does not name a type" }
 
 int main() { }
diff --git a/gcc/testsuite/g++.dg/cpp0x/alias-decl-28.C b/gcc/testsuite/g++.dg/cpp0x/alias-decl-28.C
new file mode 100644
index 0000000..086b5e5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/alias-decl-28.C
@@ -0,0 +1,7 @@
+// Origin: PR c++/54401
+// { dg-do compile { target c++11 } }
+
+template<typename>
+struct X {
+    using type = T; // { dg-error "expected type-specifier|does not name a type" }
+};
-- 
		Dodji

Re: [PING] [PATCH] PR c++/54401 - Confusing diagnostics about type-alias at class scope

[Jason Merrill]

Oops, thought I had sent this before.

On 11/17/2012 10:23 AM, Dodji Seketeli wrote:
> -	  if (cp_parser_parse_definitely (parser))
> +	  /* Note that if we actually see the '=' token after the
> +	     identifier, cp_parser_alias_declaration commits the
> +	     tentative parse.  In that case, we really expects an
> +	     alias-declaration.  Otherwise, we expect a using
> +	     declaration.  */
> +	  alias_decl_expected =
> +	    !cp_parser_uncommitted_to_tentative_parse_p (parser);
> +	  cp_parser_parse_definitely (parser);

Maybe just check whether decl is error_mark_node?

> +  if (type == error_mark_node)
> +    return error_mark_node;

I think for error-recovery we might want to 
cp_parser_skip_to_end_of_block_or_statement as well.

Jason

Re: [PING] [PATCH] PR c++/54401 - Confusing diagnostics about type-alias at class scope

[Dodji Seketeli]

Jason Merrill <jason@redhat.com> writes:

> Oops, thought I had sent this before.

No problem.  Thank you for replying now×

> On 11/17/2012 10:23 AM, Dodji Seketeli wrote:
> > -	  if (cp_parser_parse_definitely (parser))
> > +	  /* Note that if we actually see the '=' token after the
> > +	     identifier, cp_parser_alias_declaration commits the
> > +	     tentative parse.  In that case, we really expects an
> > +	     alias-declaration.  Otherwise, we expect a using
> > +	     declaration.  */
> > +	  alias_decl_expected =
> > +	    !cp_parser_uncommitted_to_tentative_parse_p (parser);
> > +	  cp_parser_parse_definitely (parser);
> 
> Maybe just check whether decl is error_mark_node?

As we further discussed this on IRC, when cp_parser_alias_declaration
returns error_mark_node, it is either because we encountered an error
before the hypothetical '=', in which case we might want to try to
parse a using declaration, or it is because we encountered an error
after the the '=', in which case we committed to parsing an alias
declaration and we must emit a hard error.

And I believe what we want here is to tell the two possible cases
apart.

> 
> > +  if (type == error_mark_node)
> > +    return error_mark_node;
> 
> I think for error-recovery we might want to
> cp_parser_skip_to_end_of_block_or_statement as well.

Right, done.

Bootstrapped and tested on x86_64-unknown-linux-gnu against trunk.

gcc/cp/

	* parser.c (cp_parser_alias_declaration): Commit to tentative
	parse when see the '=' token.  Get out if the type-id is invalid.
	Update function comment.
	(cp_parser_member_declaration): Don't try to parse a using
	declaration if we know that we expected an alias declaration; that
	is, if we see the '=' token after the identifier.

gcc/testsuite/

	* g++.dg/cpp0x/alias-decl-28.C: New test.
	* g++.dg/cpp0x/alias-decl-16.C: Update.
---
 gcc/cp/parser.c                            | 31 +++++++++++++++++++++++++++---
 gcc/testsuite/g++.dg/cpp0x/alias-decl-16.C |  2 +-
 gcc/testsuite/g++.dg/cpp0x/alias-decl-28.C |  7 +++++++
 3 files changed, 36 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/alias-decl-28.C

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index a010f1f..c39ef30 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -15262,7 +15262,11 @@ cp_parser_using_declaration (cp_parser* parser,
 /* Parse an alias-declaration.
 
    alias-declaration:
-     using identifier attribute-specifier-seq [opt] = type-id  */
+     using identifier attribute-specifier-seq [opt] = type-id
+
+   Note that if this function is used in the context of a tentative
+   parse, it commits the currently active tentative parse after it
+   sees the '=' token.  */
 
 static tree
 cp_parser_alias_declaration (cp_parser* parser)
@@ -15295,6 +15299,8 @@ cp_parser_alias_declaration (cp_parser* parser)
   if (cp_parser_error_occurred (parser))
     return error_mark_node;
 
+  cp_parser_commit_to_tentative_parse (parser);
+
   /* Now we are going to parse the type-id of the declaration.  */
 
   /*
@@ -15324,10 +15330,19 @@ cp_parser_alias_declaration (cp_parser* parser)
   if (parser->num_template_parameter_lists)
     parser->type_definition_forbidden_message = saved_message;
 
+  if (type == error_mark_node)
+    {
+      cp_parser_skip_to_end_of_block_or_statement (parser);
+      return error_mark_node;
+    }
+
   cp_parser_require (parser, CPP_SEMICOLON, RT_SEMICOLON);
 
   if (cp_parser_error_occurred (parser))
-    return error_mark_node;
+    {
+      cp_parser_skip_to_end_of_block_or_statement (parser);
+      return error_mark_node;
+    }
 
   /* A typedef-name can also be introduced by an alias-declaration. The
      identifier following the using keyword becomes a typedef-name. It has
@@ -19063,9 +19078,19 @@ cp_parser_member_declaration (cp_parser* parser)
       else
 	{
 	  tree decl;
+	  bool alias_decl_expected;
 	  cp_parser_parse_tentatively (parser);
 	  decl = cp_parser_alias_declaration (parser);
-	  if (cp_parser_parse_definitely (parser))
+	  /* Note that if we actually see the '=' token after the
+	     identifier, cp_parser_alias_declaration commits the
+	     tentative parse.  In that case, we really expects an
+	     alias-declaration.  Otherwise, we expect a using
+	     declaration.  */
+	  alias_decl_expected =
+	    !cp_parser_uncommitted_to_tentative_parse_p (parser);
+	  cp_parser_parse_definitely (parser);
+
+	  if (alias_decl_expected)
 	    finish_member_declaration (decl);
 	  else
 	    cp_parser_using_declaration (parser,
diff --git a/gcc/testsuite/g++.dg/cpp0x/alias-decl-16.C b/gcc/testsuite/g++.dg/cpp0x/alias-decl-16.C
index d66660a..ce6ad0a 100644
--- a/gcc/testsuite/g++.dg/cpp0x/alias-decl-16.C
+++ b/gcc/testsuite/g++.dg/cpp0x/alias-decl-16.C
@@ -23,6 +23,6 @@ template<class T>
 using A3 =
     enum B3 {b = 0;}; //{ dg-error "types may not be defined in alias template" }
 
-A3<int> a3;
+A3<int> a3; // { dg-error "'A3' does not name a type" }
 
 int main() { }
diff --git a/gcc/testsuite/g++.dg/cpp0x/alias-decl-28.C b/gcc/testsuite/g++.dg/cpp0x/alias-decl-28.C
new file mode 100644
index 0000000..086b5e5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/alias-decl-28.C
@@ -0,0 +1,7 @@
+// Origin: PR c++/54401
+// { dg-do compile { target c++11 } }
+
+template<typename>
+struct X {
+    using type = T; // { dg-error "expected type-specifier|does not name a type" }
+};
-- 
1.7.11.7


-- 
		Dodji

Re: [PING] [PATCH] PR c++/54401 - Confusing diagnostics about type-alias at class scope

[Gabriel Dos Reis]

On Fri, Dec 7, 2012 at 7:33 AM, Dodji Seketeli <dodji@redhat.com> wrote:
> Jason Merrill <jason@redhat.com> writes:
>
>> Oops, thought I had sent this before.
>
> No problem.  Thank you for replying now×
>
>> On 11/17/2012 10:23 AM, Dodji Seketeli wrote:
>> > -     if (cp_parser_parse_definitely (parser))
>> > +     /* Note that if we actually see the '=' token after the
>> > +        identifier, cp_parser_alias_declaration commits the
>> > +        tentative parse.  In that case, we really expects an
>> > +        alias-declaration.  Otherwise, we expect a using
>> > +        declaration.  */
>> > +     alias_decl_expected =
>> > +       !cp_parser_uncommitted_to_tentative_parse_p (parser);
>> > +     cp_parser_parse_definitely (parser);
>>
>> Maybe just check whether decl is error_mark_node?
>
> As we further discussed this on IRC, when cp_parser_alias_declaration
> returns error_mark_node, it is either because we encountered an error
> before the hypothetical '=', in which case we might want to try to
> parse a using declaration, or it is because we encountered an error
> after the the '=', in which case we committed to parsing an alias
> declaration and we must emit a hard error.
>
> And I believe what we want here is to tell the two possible cases
> apart.

Yes!

>
>>
>> > +  if (type == error_mark_node)
>> > +    return error_mark_node;
>>
>> I think for error-recovery we might want to
>> cp_parser_skip_to_end_of_block_or_statement as well.
>
> Right, done.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu against trunk.
>
> gcc/cp/
>
>         * parser.c (cp_parser_alias_declaration): Commit to tentative
>         parse when see the '=' token.  Get out if the type-id is invalid.
>         Update function comment.
>         (cp_parser_member_declaration): Don't try to parse a using
>         declaration if we know that we expected an alias declaration; that
>         is, if we see the '=' token after the identifier.
>
> gcc/testsuite/
>
>         * g++.dg/cpp0x/alias-decl-28.C: New test.
>         * g++.dg/cpp0x/alias-decl-16.C: Update.
> ---
>  gcc/cp/parser.c                            | 31 +++++++++++++++++++++++++++---
>  gcc/testsuite/g++.dg/cpp0x/alias-decl-16.C |  2 +-
>  gcc/testsuite/g++.dg/cpp0x/alias-decl-28.C |  7 +++++++
>  3 files changed, 36 insertions(+), 4 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/cpp0x/alias-decl-28.C
>
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index a010f1f..c39ef30 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -15262,7 +15262,11 @@ cp_parser_using_declaration (cp_parser* parser,
>  /* Parse an alias-declaration.
>
>     alias-declaration:
> -     using identifier attribute-specifier-seq [opt] = type-id  */
> +     using identifier attribute-specifier-seq [opt] = type-id
> +
> +   Note that if this function is used in the context of a tentative
> +   parse, it commits the currently active tentative parse after it
> +   sees the '=' token.  */

This comment is repeated below; I don't think it is necessary
or should be part of the specification.  In general we don't (and shouldn't)
say this unless a parsing function deviates from the general
parsing strategy -- this one doesn't.


>
>  static tree
>  cp_parser_alias_declaration (cp_parser* parser)
> @@ -15295,6 +15299,8 @@ cp_parser_alias_declaration (cp_parser* parser)
>    if (cp_parser_error_occurred (parser))
>      return error_mark_node;
>
> +  cp_parser_commit_to_tentative_parse (parser);
> +
>    /* Now we are going to parse the type-id of the declaration.  */
>
>    /*
> @@ -15324,10 +15330,19 @@ cp_parser_alias_declaration (cp_parser* parser)
>    if (parser->num_template_parameter_lists)
>      parser->type_definition_forbidden_message = saved_message;
>
> +  if (type == error_mark_node)
> +    {
> +      cp_parser_skip_to_end_of_block_or_statement (parser);
> +      return error_mark_node;
> +    }
> +
>    cp_parser_require (parser, CPP_SEMICOLON, RT_SEMICOLON);
>
>    if (cp_parser_error_occurred (parser))
> -    return error_mark_node;
> +    {
> +      cp_parser_skip_to_end_of_block_or_statement (parser);
> +      return error_mark_node;
> +    }
>
>    /* A typedef-name can also be introduced by an alias-declaration. The
>       identifier following the using keyword becomes a typedef-name. It has
> @@ -19063,9 +19078,19 @@ cp_parser_member_declaration (cp_parser* parser)
>        else
>         {
>           tree decl;
> +         bool alias_decl_expected;
>           cp_parser_parse_tentatively (parser);
>           decl = cp_parser_alias_declaration (parser);
> -         if (cp_parser_parse_definitely (parser))
> +         /* Note that if we actually see the '=' token after the
> +            identifier, cp_parser_alias_declaration commits the
> +            tentative parse.  In that case, we really expects an
> +            alias-declaration.  Otherwise, we expect a using
> +            declaration.  */
> +         alias_decl_expected =
> +           !cp_parser_uncommitted_to_tentative_parse_p (parser);
> +         cp_parser_parse_definitely (parser);
> +
> +         if (alias_decl_expected)
>             finish_member_declaration (decl);
>           else
>             cp_parser_using_declaration (parser,
> diff --git a/gcc/testsuite/g++.dg/cpp0x/alias-decl-16.C b/gcc/testsuite/g++.dg/cpp0x/alias-decl-16.C
> index d66660a..ce6ad0a 100644
> --- a/gcc/testsuite/g++.dg/cpp0x/alias-decl-16.C
> +++ b/gcc/testsuite/g++.dg/cpp0x/alias-decl-16.C
> @@ -23,6 +23,6 @@ template<class T>
>  using A3 =
>      enum B3 {b = 0;}; //{ dg-error "types may not be defined in alias template" }
>
> -A3<int> a3;
> +A3<int> a3; // { dg-error "'A3' does not name a type" }
>
>  int main() { }
> diff --git a/gcc/testsuite/g++.dg/cpp0x/alias-decl-28.C b/gcc/testsuite/g++.dg/cpp0x/alias-decl-28.C
> new file mode 100644
> index 0000000..086b5e5
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/alias-decl-28.C
> @@ -0,0 +1,7 @@
> +// Origin: PR c++/54401
> +// { dg-do compile { target c++11 } }
> +
> +template<typename>
> +struct X {
> +    using type = T; // { dg-error "expected type-specifier|does not name a type" }
> +};
> --
> 1.7.11.7
>
>
> --
>                 Dodji

Sound good to me (except for the comment).
thanks!

-- Gaby

Re: [PING] [PATCH] PR c++/54401 - Confusing diagnostics about type-alias at class scope

[Jason Merrill]

On 12/07/2012 08:42 AM, Gabriel Dos Reis wrote:
> Sound good to me (except for the comment).

Me, too.

Jason

Re: [PING] [PATCH] PR c++/54401 - Confusing diagnostics about type-alias at class scope

[Dodji Seketeli]

Gabriel Dos Reis <gdr@integrable-solutions.net> writes:

> On Fri, Dec 7, 2012 at 7:33 AM, Dodji Seketeli <dodji@redhat.com> wrote:

>> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
>> index a010f1f..c39ef30 100644
>> --- a/gcc/cp/parser.c
>> +++ b/gcc/cp/parser.c
>> @@ -15262,7 +15262,11 @@ cp_parser_using_declaration (cp_parser* parser,
>>  /* Parse an alias-declaration.
>>
>>     alias-declaration:
>> -     using identifier attribute-specifier-seq [opt] = type-id  */
>> +     using identifier attribute-specifier-seq [opt] = type-id
>> +
>> +   Note that if this function is used in the context of a tentative
>> +   parse, it commits the currently active tentative parse after it
>> +   sees the '=' token.  */
>
> This comment is repeated below; I don't think it is necessary
> or should be part of the specification.  In general we don't (and shouldn't)
> say this unless a parsing function deviates from the general
> parsing strategy -- this one doesn't.

Fair enough.  I have removed this comment then.  Thanks.

> Sound good to me (except for the comment).

Thanks!

Here is what I am committing to trunk.

gcc/cp/

	* parser.c (cp_parser_alias_declaration): Commit to tentative
	parse when see the '=' token.  Get out if the type-id is invalid.
	Update function comment.
	(cp_parser_member_declaration): Don't try to parse a using
	declaration if we know that we expected an alias declaration; that
	is, if we see the '=' token after the identifier.

gcc/testsuite/

	* g++.dg/cpp0x/alias-decl-28.C: New test.
	* g++.dg/cpp0x/alias-decl-16.C: Update.
---
 gcc/cp/ChangeLog                           | 10 ++++++++++
 gcc/cp/parser.c                            | 25 +++++++++++++++++++++++--
 gcc/testsuite/ChangeLog                    |  6 ++++++
 gcc/testsuite/g++.dg/cpp0x/alias-decl-16.C |  2 +-
 gcc/testsuite/g++.dg/cpp0x/alias-decl-28.C |  7 +++++++
 5 files changed, 47 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/alias-decl-28.C

diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 7bec9ca..81c2048 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,13 @@
+2012-12-07  Dodji Seketeli  <dodji@redhat.com>
+
+	PR c++/54401
+	* parser.c (cp_parser_alias_declaration): Commit to tentative
+	parse when see the '=' token.  Get out if the type-id is invalid.
+	Update function comment.
+	(cp_parser_member_declaration): Don't try to parse a using
+	declaration if we know that we expected an alias declaration; that
+	is, if we see the '=' token after the identifier.
+
 2012-12-06  Jason Merrill  <jason@redhat.com>
 
 	PR c++/54325
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 3566d74..3dc2ec6 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -15295,6 +15295,8 @@ cp_parser_alias_declaration (cp_parser* parser)
   if (cp_parser_error_occurred (parser))
     return error_mark_node;
 
+  cp_parser_commit_to_tentative_parse (parser);
+
   /* Now we are going to parse the type-id of the declaration.  */
 
   /*
@@ -15324,10 +15326,19 @@ cp_parser_alias_declaration (cp_parser* parser)
   if (parser->num_template_parameter_lists)
     parser->type_definition_forbidden_message = saved_message;
 
+  if (type == error_mark_node)
+    {
+      cp_parser_skip_to_end_of_block_or_statement (parser);
+      return error_mark_node;
+    }
+
   cp_parser_require (parser, CPP_SEMICOLON, RT_SEMICOLON);
 
   if (cp_parser_error_occurred (parser))
-    return error_mark_node;
+    {
+      cp_parser_skip_to_end_of_block_or_statement (parser);
+      return error_mark_node;
+    }
 
   /* A typedef-name can also be introduced by an alias-declaration. The
      identifier following the using keyword becomes a typedef-name. It has
@@ -19063,9 +19074,19 @@ cp_parser_member_declaration (cp_parser* parser)
       else
 	{
 	  tree decl;
+	  bool alias_decl_expected;
 	  cp_parser_parse_tentatively (parser);
 	  decl = cp_parser_alias_declaration (parser);
-	  if (cp_parser_parse_definitely (parser))
+	  /* Note that if we actually see the '=' token after the
+	     identifier, cp_parser_alias_declaration commits the
+	     tentative parse.  In that case, we really expects an
+	     alias-declaration.  Otherwise, we expect a using
+	     declaration.  */
+	  alias_decl_expected =
+	    !cp_parser_uncommitted_to_tentative_parse_p (parser);
+	  cp_parser_parse_definitely (parser);
+
+	  if (alias_decl_expected)
 	    finish_member_declaration (decl);
 	  else
 	    cp_parser_using_declaration (parser,
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index d50d73b..1fd69fa 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2012-12-07  Dodji Seketeli  <dodji@redhat.com>
+
+	PR c++/54401
+	* g++.dg/cpp0x/alias-decl-28.C: New test.
+	* g++.dg/cpp0x/alias-decl-16.C: Update.
+
 2012-12-07  Martin Jambor  <mjambor@suse.cz>
 
 	PR tree-optimization/55590
diff --git a/gcc/testsuite/g++.dg/cpp0x/alias-decl-16.C b/gcc/testsuite/g++.dg/cpp0x/alias-decl-16.C
index d66660a..ce6ad0a 100644
--- a/gcc/testsuite/g++.dg/cpp0x/alias-decl-16.C
+++ b/gcc/testsuite/g++.dg/cpp0x/alias-decl-16.C
@@ -23,6 +23,6 @@ template<class T>
 using A3 =
     enum B3 {b = 0;}; //{ dg-error "types may not be defined in alias template" }
 
-A3<int> a3;
+A3<int> a3; // { dg-error "'A3' does not name a type" }
 
 int main() { }
diff --git a/gcc/testsuite/g++.dg/cpp0x/alias-decl-28.C b/gcc/testsuite/g++.dg/cpp0x/alias-decl-28.C
new file mode 100644
index 0000000..086b5e5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/alias-decl-28.C
@@ -0,0 +1,7 @@
+// Origin: PR c++/54401
+// { dg-do compile { target c++11 } }
+
+template<typename>
+struct X {
+    using type = T; // { dg-error "expected type-specifier|does not name a type" }
+};
-- 
		Dodji

Re: [PING] [PATCH] PR c++/54401 - Confusing diagnostics about type-alias at class scope

[Gabriel Dos Reis]

On Fri, Dec 7, 2012 at 10:06 AM, Dodji Seketeli <dodji@redhat.com> wrote:

> Here is what I am committing to trunk.

Thank you!

-- Gaby