Re: [C++ Patch] PR 58980
On 01/22/2014 04:29 PM, Paolo Carlini wrote: On 01/22/2014 10:10 PM, Jason Merrill wrote: Yep, that's along the lines I was thinking of. But again, prev_scope is irrelevant here, so the new code shouldn't mention it at all. Well, in practice I have to mention it in the error_at itself. Why? prev_scope is the context where the name is written, i.e. current_class_type. Why should the diagnostic treat it as an explicit qualifier? Jason
Re: [C++ Patch] PR 58980
Hi, On 01/23/2014 03:05 PM, Jason Merrill wrote: On 01/22/2014 04:29 PM, Paolo Carlini wrote: On 01/22/2014 10:10 PM, Jason Merrill wrote: Yep, that's along the lines I was thinking of. But again, prev_scope is irrelevant here, so the new code shouldn't mention it at all. Well, in practice I have to mention it in the error_at itself. Why? prev_scope is the context where the name is written, i.e. current_class_type. Why should the diagnostic treat it as an explicit qualifier? Jason, no problem, you choose. I only wanted to use the same compact form 'A::B' used in the non-template case. Which form do you prefer? Paolo.
Re: [C++ Patch] PR 58980
On 01/23/2014 04:01 PM, Paolo Carlini wrote: Hi, On 01/23/2014 03:05 PM, Jason Merrill wrote: On 01/22/2014 04:29 PM, Paolo Carlini wrote: On 01/22/2014 10:10 PM, Jason Merrill wrote: Yep, that's along the lines I was thinking of. But again, prev_scope is irrelevant here, so the new code shouldn't mention it at all. Well, in practice I have to mention it in the error_at itself. Why? prev_scope is the context where the name is written, i.e. current_class_type. Why should the diagnostic treat it as an explicit qualifier? Jason, no problem, you choose. I only wanted to use the same compact form 'A::B' used in the non-template case. Which form do you prefer? To be clear if we use %qT with nested_name_specifier we get: 58980.C:5:8: error: ‘typename A template-parameter-1-1 ::B’ has not been declared which frankly seems suboptimal to me, both vs the non-template case and the use of typename. Thus, as far as I can see, either what I posted, which is consistent with the '... has not been declared' message of the non-template case (I understand that we want to focus on the 'has not been declared' issue) or just '%%E%' or even just '%E' and nested_name_specifier. Paolo.
Re: [C++ Patch] PR 58980
On 01/23/2014 10:30 AM, Paolo Carlini wrote: To be clear if we use %qT with nested_name_specifier we get: 58980.C:5:8: error: ‘typename A template-parameter-1-1 ::B’ has not been declared which frankly seems suboptimal to me, both vs the non-template case and the use of typename. If you want to break apart the typename and do %T::%E with TYPE_CONTEXT (nested_name_specifier) on the left side, that's fine too. Jason
Re: [C++ Patch] PR 58980
On 01/23/2014 04:50 PM, Jason Merrill wrote:
On 01/23/2014 10:30 AM, Paolo Carlini wrote:
To be clear if we use %qT with nested_name_specifier we get:
58980.C:5:8: error: ‘typename A template-parameter-1-1 ::B’ has not
been declared
which frankly seems suboptimal to me, both vs the non-template case and
the use of typename.
If you want to break apart the typename and do %T::%E with
TYPE_CONTEXT (nested_name_specifier) on the left side, that's fine too.
Ah, Ok, I was missing that we really want to use TYPE_CONTEXT. To
confirm, then we get:
58980.C:5:8: error: ‘A template-parameter-1-1 ::B’ has not been declared
I'm finishing testing the below.
Thanks,
Paolo.
///
Index: cp/parser.c
===
--- cp/parser.c (revision 206958)
+++ cp/parser.c (working copy)
@@ -15469,9 +15469,18 @@ cp_parser_enum_specifier (cp_parser* parser)
error_at (type_start_token-location, cannot add an enumerator
list to a template instantiation);
+ if (TREE_CODE (nested_name_specifier) == TYPENAME_TYPE)
+ {
+ error_at (type_start_token-location,
+ %%T::%E% has not been declared,
+ TYPE_CONTEXT (nested_name_specifier),
+ nested_name_specifier);
+ type = error_mark_node;
+ }
/* If that scope does not contain the scope in which the
class was originally declared, the program is invalid. */
- if (prev_scope !is_ancestor (prev_scope, nested_name_specifier))
+ else if (prev_scope !is_ancestor (prev_scope,
+ nested_name_specifier))
{
if (at_namespace_scope_p ())
error_at (type_start_token-location,
@@ -15480,7 +15489,8 @@ cp_parser_enum_specifier (cp_parser* parser)
type, prev_scope, nested_name_specifier);
else
error_at (type_start_token-location,
- declaration of %qD in %qD which does not enclose
%qD,
+ declaration of %qD in %qD which does not
+ enclose %qD,
type, prev_scope, nested_name_specifier);
type = error_mark_node;
}
Index: testsuite/g++.dg/parse/enum11.C
===
--- testsuite/g++.dg/parse/enum11.C (revision 0)
+++ testsuite/g++.dg/parse/enum11.C (working copy)
@@ -0,0 +1,6 @@
+// PR c++/58980
+
+templatetypename struct A
+{
+ enum A::B::C {}; // { dg-error has not been declared }
+};
Re: [C++ Patch] PR 58980
OK, thanks. Jason
Re: [C++ Patch] PR 58980
On 01/21/2014 04:16 PM, Paolo Carlini wrote: + if (TREE_CODE (child) == TYPENAME_TYPE) +return false; + Maybe we want instead to do if (WILDCARD_TYPE_P (child)) return false; (which also passes testing, of course). Thanks, Paolo.
Re: [C++ Patch] PR 58980
On 01/21/2014 09:55 AM, Jason Merrill wrote: I think I would prefer to change the child assert to be MAYBE_CLASS_TYPE_P rather than CLASS_TYPE_P. On second thought, no, I think we do want to specifically handle TYPENAME_TYPE. But I think we want a different error message; getting a TYPENAME_TYPE here means that B has not been declared. The current scope is irrelevant. So we want to check for TYPENAME_TYPE before we think about checking prev_scope. Jason
Re: [C++ Patch] PR 58980
Hi,
On 01/22/2014 06:13 PM, Jason Merrill wrote:
On 01/21/2014 09:55 AM, Jason Merrill wrote:
I think I would prefer to change the child assert to be
MAYBE_CLASS_TYPE_P rather than CLASS_TYPE_P.
On second thought, no, I think we do want to specifically handle
TYPENAME_TYPE. But I think we want a different error message; getting
a TYPENAME_TYPE here means that B has not been declared. The current
scope is irrelevant. So we want to check for TYPENAME_TYPE before we
think about checking prev_scope.
Ok. In fact I entertained myself this kind of reasoning, a couple of
days ago...
I tested the below, which uses by hand %Es instead of %qT for more
concise error messages (consistent with the non-template case).
Thanks,
Paolo.
/cp
2014-01-22 Paolo Carlini paolo.carl...@oracle.com
PR c++/58980
* parser.c (cp_parser_enum_specifier): Handle TYPENAME_TYPE as
nested_name_specifier.
/testsuite
2014-01-22 Paolo Carlini paolo.carl...@oracle.com
PR c++/58980
* g++.dg/parse/enum11.C: New.
Index: cp/parser.c
===
--- cp/parser.c (revision 206933)
+++ cp/parser.c (working copy)
@@ -15469,9 +15469,17 @@ cp_parser_enum_specifier (cp_parser* parser)
error_at (type_start_token-location, cannot add an enumerator
list to a template instantiation);
+ if (prev_scope TREE_CODE (nested_name_specifier) == TYPENAME_TYPE)
+ {
+ error_at (type_start_token-location,
+ %%E::%E% has not been declared,
+ prev_scope, nested_name_specifier);
+ type = error_mark_node;
+ }
/* If that scope does not contain the scope in which the
class was originally declared, the program is invalid. */
- if (prev_scope !is_ancestor (prev_scope, nested_name_specifier))
+ else if (prev_scope !is_ancestor (prev_scope,
+ nested_name_specifier))
{
if (at_namespace_scope_p ())
error_at (type_start_token-location,
@@ -15480,7 +15488,8 @@ cp_parser_enum_specifier (cp_parser* parser)
type, prev_scope, nested_name_specifier);
else
error_at (type_start_token-location,
- declaration of %qD in %qD which does not enclose
%qD,
+ declaration of %qD in %qD which does not
+ enclose %qD,
type, prev_scope, nested_name_specifier);
type = error_mark_node;
}
Index: testsuite/g++.dg/parse/enum11.C
===
--- testsuite/g++.dg/parse/enum11.C (revision 0)
+++ testsuite/g++.dg/parse/enum11.C (working copy)
@@ -0,0 +1,6 @@
+// PR c++/58980
+
+templatetypename struct A
+{
+ enum A::B::C {}; // { dg-error has not been declared }
+};
Re: [C++ Patch] PR 58980
Yep, that's along the lines I was thinking of. But again, prev_scope is irrelevant here, so the new code shouldn't mention it at all. Original Message From: Paolo Carlini paolo.carl...@oracle.com Sent: Wed, Jan 22, 2014 03:34 PM To: Jason Merrill ja...@redhat.com; gcc-patches@gcc.gnu.org CC: Subject: Re: [C++ Patch] PR 58980 Hi, On 01/22/2014 06:13 PM, Jason Merrill wrote: On 01/21/2014 09:55 AM, Jason Merrill wrote: I think I would prefer to change the child assert to be MAYBE_CLASS_TYPE_P rather than CLASS_TYPE_P. On second thought, no, I think we do want to specifically handle TYPENAME_TYPE. But I think we want a different error message; getting a TYPENAME_TYPE here means that B has not been declared. The current scope is irrelevant. So we want to check for TYPENAME_TYPE before we think about checking prev_scope. Ok. In fact I entertained myself this kind of reasoning, a couple of days ago... I tested the below, which uses by hand %Es instead of %qT for more concise error messages (consistent with the non-template case). Thanks, Paolo.
Re: [C++ Patch] PR 58980
On 01/22/2014 10:10 PM, Jason Merrill wrote:
Yep, that's along the lines I was thinking of. But again, prev_scope is
irrelevant here, so the new code shouldn't mention it at all.
Well, in practice I have to mention it in the error_at itself. Otherwise
I'm finishing testing the below.
Paolo.
//
Index: cp/parser.c
===
--- cp/parser.c (revision 206933)
+++ cp/parser.c (working copy)
@@ -15469,9 +15469,17 @@ cp_parser_enum_specifier (cp_parser* parser)
error_at (type_start_token-location, cannot add an enumerator
list to a template instantiation);
+ if (TREE_CODE (nested_name_specifier) == TYPENAME_TYPE)
+ {
+ error_at (type_start_token-location,
+ %%E::%E% has not been declared,
+ prev_scope, nested_name_specifier);
+ type = error_mark_node;
+ }
/* If that scope does not contain the scope in which the
class was originally declared, the program is invalid. */
- if (prev_scope !is_ancestor (prev_scope, nested_name_specifier))
+ else if (prev_scope !is_ancestor (prev_scope,
+ nested_name_specifier))
{
if (at_namespace_scope_p ())
error_at (type_start_token-location,
@@ -15480,7 +15488,8 @@ cp_parser_enum_specifier (cp_parser* parser)
type, prev_scope, nested_name_specifier);
else
error_at (type_start_token-location,
- declaration of %qD in %qD which does not enclose
%qD,
+ declaration of %qD in %qD which does not
+ enclose %qD,
type, prev_scope, nested_name_specifier);
type = error_mark_node;
}
Index: testsuite/g++.dg/parse/enum11.C
===
--- testsuite/g++.dg/parse/enum11.C (revision 0)
+++ testsuite/g++.dg/parse/enum11.C (working copy)
@@ -0,0 +1,6 @@
+// PR c++/58980
+
+templatetypename struct A
+{
+ enum A::B::C {}; // { dg-error has not been declared }
+};
[C++ Patch] PR 58980
Hi,
in this relatively serious ICE on invalid regression (we don't emit any
sensible diagnostic before ICE-ing) the problem is that is_ancestor
simply asserts that the second argument can be only a NAMESPACE_DECL or
a CLASS_TYPE_P, whereas in the erroneous input at issue it's a
TYPENAME_TYPE. Thus the idea of not calling is_ancestor at all in this
case + adjusting the two error_at calls to use %qT instead of %qD
(dump_type can handle a NAMESPACE_DECL whereas dump_type can't handle a
TYPENAME_TYPE).
Tested x86_64-linux.
Thanks,
Paolo.
//
/cp
2014-01-21 Paolo Carlini paolo.carl...@oracle.com
PR c++/58980
* parser.c (cp_parser_enum_specifier): Handle TYPENAME_TYPE as
nested_name_specifier.
/testsuite
2014-01-21 Paolo Carlini paolo.carl...@oracle.com
PR c++/58980
* g++.dg/parse/enum11.C: New.
Index: cp/parser.c
===
--- cp/parser.c (revision 206875)
+++ cp/parser.c (working copy)
@@ -15471,16 +15471,19 @@ cp_parser_enum_specifier (cp_parser* parser)
/* If that scope does not contain the scope in which the
class was originally declared, the program is invalid. */
- if (prev_scope !is_ancestor (prev_scope, nested_name_specifier))
+ if (prev_scope
+ (TREE_CODE (nested_name_specifier) == TYPENAME_TYPE
+ || !is_ancestor (prev_scope, nested_name_specifier)))
{
if (at_namespace_scope_p ())
error_at (type_start_token-location,
declaration of %qD in namespace %qD which does not
- enclose %qD,
+ enclose %qT,
type, prev_scope, nested_name_specifier);
else
error_at (type_start_token-location,
- declaration of %qD in %qD which does not enclose
%qD,
+ declaration of %qD in %qD which does not
+ enclose %qT,
type, prev_scope, nested_name_specifier);
type = error_mark_node;
}
Index: testsuite/g++.dg/parse/enum11.C
===
--- testsuite/g++.dg/parse/enum11.C (revision 0)
+++ testsuite/g++.dg/parse/enum11.C (working copy)
@@ -0,0 +1,6 @@
+// PR c++/58980
+
+templatetypename struct A
+{
+ enum A::B::C {}; // { dg-error does not enclose }
+};
Re: [C++ Patch] PR 58980
I think I would prefer to change the child assert to be MAYBE_CLASS_TYPE_P rather than CLASS_TYPE_P. Jason
Re: [C++ Patch] PR 58980
Hi,
On 01/21/2014 03:55 PM, Jason Merrill wrote:
I think I would prefer to change the child assert to be
MAYBE_CLASS_TYPE_P rather than CLASS_TYPE_P.
.. Ok, but then it seems to me that we have to explicitly handle the
TYPENAME_TYPE case, otherwise we end up simply accepting the testcase
(even if the template is instantiated!). Something like the below...
Paolo.
//
Index: cp/name-lookup.c
===
--- cp/name-lookup.c(revision 206880)
+++ cp/name-lookup.c(working copy)
@@ -2723,12 +2723,15 @@ is_ancestor (tree root, tree child)
|| TREE_CODE (root) == FUNCTION_DECL
|| CLASS_TYPE_P (root)));
gcc_assert ((TREE_CODE (child) == NAMESPACE_DECL
- || CLASS_TYPE_P (child)));
+ || MAYBE_CLASS_TYPE_P (child)));
/* The global namespace encloses everything. */
if (root == global_namespace)
return true;
+ if (TREE_CODE (child) == TYPENAME_TYPE)
+return false;
+
while (true)
{
/* If we've run out of scopes, stop. */
Index: cp/parser.c
===
--- cp/parser.c (revision 206880)
+++ cp/parser.c (working copy)
@@ -15476,11 +15476,12 @@ cp_parser_enum_specifier (cp_parser* parser)
if (at_namespace_scope_p ())
error_at (type_start_token-location,
declaration of %qD in namespace %qD which does not
- enclose %qD,
+ enclose %qT,
type, prev_scope, nested_name_specifier);
else
error_at (type_start_token-location,
- declaration of %qD in %qD which does not enclose
%qD,
+ declaration of %qD in %qD which does not
+ enclose %qT,
type, prev_scope, nested_name_specifier);
type = error_mark_node;
}
Index: testsuite/g++.dg/parse/enum11.C
===
--- testsuite/g++.dg/parse/enum11.C (revision 0)
+++ testsuite/g++.dg/parse/enum11.C (working copy)
@@ -0,0 +1,6 @@
+// PR c++/58980
+
+templatetypename struct A
+{
+ enum A::B::C {}; // { dg-error does not enclose }
+};
