Re: [C++ Patch] PR 26155 (improved patch)
OK. Jason
[C++ Patch] PR 26155 (improved patch)
Hi,
this is my last and best try ;) Seriously, compared to the last posted
version:
1- I added a rather large comment explaining what's going on wrt error
recovery.
2- Took the occasion to clean-up a bit the code about bool vs int for
flags (we had a mix).
3- Cleaned-up a bit the code itself (didn't notice that need_new is
initialized as true)
Tested x86_64-linux, as usual.
Thanks,
Paolo.
/cp
2012-06-01 Paolo Carlini
PR c++/26155
* name-lookup.c (push_namespace): When error recovery is
impossible just error out in duplicate_decls.
/testsuite
2012-06-01 Paolo Carlini
PR c++/26155
* g++.dg/parse/namespace-alias-1.C: New.
Index: testsuite/g++.dg/parse/namespace-alias-1.C
===
--- testsuite/g++.dg/parse/namespace-alias-1.C (revision 0)
+++ testsuite/g++.dg/parse/namespace-alias-1.C (revision 0)
@@ -0,0 +1,7 @@
+// PR c++/26155
+
+namespace N
+{
+ namespace M = N; // { dg-error "previous declaration" }
+ namespace M {}// { dg-error "declaration of namespace" }
+}
Index: cp/name-lookup.c
===
--- cp/name-lookup.c(revision 188104)
+++ cp/name-lookup.c(working copy)
@@ -3518,8 +3518,8 @@ void
push_namespace (tree name)
{
tree d = NULL_TREE;
- int need_new = 1;
- int implicit_use = 0;
+ bool need_new = true;
+ bool implicit_use = false;
bool anon = !name;
bool subtime = timevar_cond_start (TV_NAME_LOOKUP);
@@ -3535,8 +3535,8 @@ push_namespace (tree name)
d = IDENTIFIER_NAMESPACE_VALUE (name);
if (d)
/* Reopening anonymous namespace. */
- need_new = 0;
- implicit_use = 1;
+ need_new = false;
+ implicit_use = true;
}
else
{
@@ -3544,13 +3544,36 @@ push_namespace (tree name)
d = IDENTIFIER_NAMESPACE_VALUE (name);
if (d != NULL_TREE && TREE_CODE (d) == NAMESPACE_DECL)
{
- need_new = 0;
- if (DECL_NAMESPACE_ALIAS (d))
- {
- error ("namespace alias %qD not allowed here, assuming %qD",
-d, DECL_NAMESPACE_ALIAS (d));
- d = DECL_NAMESPACE_ALIAS (d);
+ tree dna = DECL_NAMESPACE_ALIAS (d);
+ if (dna)
+ {
+ /* We do some error recovery for, eg, the redeclaration
+of M here:
+
+namespace N {}
+namespace M = N;
+namespace M {}
+
+However, in nasty cases like:
+
+namespace N
+{
+ namespace M = N;
+ namespace M {}
+}
+
+we just error out below, in duplicate_decls. */
+ if (NAMESPACE_LEVEL (dna)->level_chain
+ == current_binding_level)
+ {
+ error ("namespace alias %qD not allowed here, "
+"assuming %qD", d, dna);
+ d = dna;
+ need_new = false;
+ }
}
+ else
+ need_new = false;
}
}
Re: [C++ Patch] PR 26155
.. Ah! I think I have a much better tentative fix, which, as it happens,
also leads to a diagnostics quite close to that produced by the EDG
front-end.
The idea is checking for the offending case and not handling it in any
special way besides turning need_new = 1, as if for a normal new
declaration: actually in this case it conflicts with the existing one
from the namespace alias declaration, and leads to a guaranteed error a
bit later. Patch passes testing.
Thanks,
Paolo.
///
Index: testsuite/g++.dg/parse/namespace-alias-1.C
===
--- testsuite/g++.dg/parse/namespace-alias-1.C (revision 0)
+++ testsuite/g++.dg/parse/namespace-alias-1.C (revision 0)
@@ -0,0 +1,7 @@
+// PR c++/26155
+
+namespace N
+{
+ namespace M = N; // { dg-error "previous declaration" }
+ namespace M {}// { dg-error "declaration of namespace" }
+}
Index: cp/name-lookup.c
===
--- cp/name-lookup.c(revision 187991)
+++ cp/name-lookup.c(working copy)
@@ -3544,12 +3544,20 @@ push_namespace (tree name)
d = IDENTIFIER_NAMESPACE_VALUE (name);
if (d != NULL_TREE && TREE_CODE (d) == NAMESPACE_DECL)
{
+ tree dna = DECL_NAMESPACE_ALIAS (d);
need_new = 0;
- if (DECL_NAMESPACE_ALIAS (d))
+ if (dna)
{
- error ("namespace alias %qD not allowed here, assuming %qD",
-d, DECL_NAMESPACE_ALIAS (d));
- d = DECL_NAMESPACE_ALIAS (d);
+ if (NAMESPACE_LEVEL (dna)->level_chain
+ == current_binding_level)
+ {
+ error ("namespace alias %qD not allowed here, "
+"assuming %qD", d, dna);
+ d = dna;
+ }
+ else
+ /* We error out below. */
+ need_new = 1;
}
}
}
[C++ Patch] PR 26155
Hi,
in this pretty old issue we crash for this testcase:
namespace N
{
namespace M = N;
namespace M {}// { dg-error "namespace alias" }
}
after the error message, because error recovery after error fails. We
try to do:
error ("namespace alias %qD not allowed here, assuming %qD",
d, DECL_NAMESPACE_ALIAS (d));
d = DECL_NAMESPACE_ALIAS (d);
but then we crash in resume_scope because:
/* Also, resuming a non-directly nested namespace is a no-no. */
gcc_assert (b->level_chain == current_binding_level);
Thus, in the first patch below I changed push_namespace to stay away
from this nasty situation and produce and simpler diagnostics and no
special error recovery in this case: a false is returned, and the
caller, cp_parser_namespace_definition, makes sure to not call a
matching pop_namespace. Diagnostics is good, patch passes testing.
Alternately, the second patch below does the latter unconditionally,
without even trying the "assuming" thing. Same diagnostics for the
testcase at issue, no regressions in this case too. I don't have a
strong personal preference, I suppose the current "assuming" thing may
lead to fewer cascading errors, but I don't know for sure, certainly the
second patch is simpler.
Thanks,
Paolo.
/cp
2012-05-29 Paolo Carlini
PR c++/26155
* name-lookup.c (push_namespace): Simply return false after error
when error recovery is impossible.
* name-lookup.h (push_namespace): Update prototype.
* parser.c (cp_parser_namespace_definition): Chech push_namespace
return value and don't call pop_namespace when false.
/testsuite
2012-05-29 Paolo Carlini
PR c++/26155
* g++.dg/parse/namespace-alias-1.C: New.
Index: testsuite/g++.dg/parse/namespace-alias-1.C
===
--- testsuite/g++.dg/parse/namespace-alias-1.C (revision 0)
+++ testsuite/g++.dg/parse/namespace-alias-1.C (revision 0)
@@ -0,0 +1,7 @@
+// PR c++/26155
+
+namespace N
+{
+ namespace M = N;
+ namespace M {}// { dg-error "namespace alias" }
+}
Index: cp/parser.c
===
--- cp/parser.c (revision 187968)
+++ cp/parser.c (working copy)
@@ -14738,6 +14738,7 @@ cp_parser_namespace_definition (cp_parser* parser)
tree identifier, attribs;
bool has_visibility;
bool is_inline;
+ bool ok;
if (cp_lexer_next_token_is_keyword (parser->lexer, RID_INLINE))
{
@@ -14766,7 +14767,7 @@ cp_parser_namespace_definition (cp_parser* parser)
/* Look for the `{' to start the namespace. */
cp_parser_require (parser, CPP_OPEN_BRACE, RT_OPEN_BRACE);
/* Start the namespace. */
- push_namespace (identifier);
+ ok = push_namespace (identifier);
/* "inline namespace" is equivalent to a stub namespace definition
followed by a strong using directive. */
@@ -14792,7 +14793,8 @@ cp_parser_namespace_definition (cp_parser* parser)
pop_visibility (1);
/* Finish the namespace. */
- pop_namespace ();
+ if (ok)
+pop_namespace ();
/* Look for the final `}'. */
cp_parser_require (parser, CPP_CLOSE_BRACE, RT_CLOSE_BRACE);
}
Index: cp/name-lookup.c
===
--- cp/name-lookup.c(revision 187968)
+++ cp/name-lookup.c(working copy)
@@ -3512,9 +3512,10 @@ handle_namespace_attrs (tree ns, tree attributes)
}
/* Push into the scope of the NAME namespace. If NAME is NULL_TREE, then we
- select a name that is unique to this compilation unit. */
+ select a name that is unique to this compilation unit. Return false if
+ something goes very badly wrong, true otherwise. */
-void
+bool
push_namespace (tree name)
{
tree d = NULL_TREE;
@@ -3544,12 +3545,23 @@ push_namespace (tree name)
d = IDENTIFIER_NAMESPACE_VALUE (name);
if (d != NULL_TREE && TREE_CODE (d) == NAMESPACE_DECL)
{
+ tree dna = DECL_NAMESPACE_ALIAS (d);
need_new = 0;
- if (DECL_NAMESPACE_ALIAS (d))
+ if (dna)
{
- error ("namespace alias %qD not allowed here, assuming %qD",
-d, DECL_NAMESPACE_ALIAS (d));
- d = DECL_NAMESPACE_ALIAS (d);
+ if (NAMESPACE_LEVEL (dna)->level_chain
+ == current_binding_level)
+ {
+ error ("namespace alias %qD not allowed here, "
+"assuming %qD", d, dna);
+ d = dna;
+ }
+ else
+ {
+ error ("namespace alias %qD not allowed here", d);
+ timevar_cond_stop (TV_NAME_LOOKUP, subtime);
+ return false;
+ }
}
}
}
@@ -3583,6 +3595,7 @@ push_namespace (tree name)
current_namespace = d;
timevar_cond_stop (TV_NAME_LOOKUP, subtime);
+ return
