Diego, I know you're working in that area, but the bug fix I'm working on ...
13 years, 10 months ago
(2011-07-09 01:29:38 UTC)
#1
Diego, I know you're working in that area, but the bug fix I'm working on is
kinda coming up to it too.
I realize you readded those two lines (in patch below) because of an ICE
(although I still think they don't make sense, i.e. adding the bindings of a
namespace to itself when they're already in there...). I checked and the only
reason this fixes your ICE is because it goes through and calls
varpool_finalize_decl for the names in that namespace bindings, the rest does
absolutely nothing...
I know you're restructuring this right now, but just in case you're keeping some
of that logic, I'm sharing my analysis on the matter now instead of patching it
later...
Also as highlighted in this patch, I don't think we need to validate that
NAMESPACE_LEVEL != NULL with the "if". It shouldn't be (I think... I even tested
it with an empty namespace...), if anything maybe we want to assert, but not
skip if it's NULL in my opinion.
Tested with bootstrap and pph regression testing.
Gab
2011-07-08 Gabriel Charette <gchare@google.com>
* gcc/cp/pph-streamer-in.c (pph_add_bindings_to_namespace):
NAMESPACE_LEVEL should never be null for a namespace, removed check.
diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c
index d78ee91..55f7e12 100644
--- a/gcc/cp/pph-streamer-in.c
+++ b/gcc/cp/pph-streamer-in.c
@@ -1168,8 +1168,7 @@ pph_add_bindings_to_namespace (struct cp_binding_level
*bl, tree ns)
Preserve it. */
chain = DECL_CHAIN (t);
pushdecl_into_namespace (t, ns);
- if (NAMESPACE_LEVEL (t))
- pph_add_bindings_to_namespace (NAMESPACE_LEVEL (t), t);
+ pph_add_bindings_to_namespace (NAMESPACE_LEVEL (t), t);
}
}
--
This patch is available for review at http://codereview.appspot.com/4675069
On Fri, Jul 8, 2011 at 21:29, Gabriel Charette <gchare@google.com> wrote: > 2011-07-08 Gabriel Charette ...
13 years, 9 months ago
(2011-07-12 12:32:37 UTC)
#2
On Fri, Jul 8, 2011 at 21:29, Gabriel Charette <gchare@google.com> wrote:
> 2011-07-08 Gabriel Charette <gchare@google.com>
>
> * gcc/cp/pph-streamer-in.c (pph_add_bindings_to_namespace):
> NAMESPACE_LEVEL should never be null for a namespace, removed check.
OK. I committed it to the branch. I need to adjust the other two
patches you sent as I'm starting to fix something in the same area.
Diego.
Issue 4675069: [pph] Remove protection for NAMESPACE_LEVEL being null when adding namespaces
(Closed)
Created 13 years, 10 months ago by Gabriel Charette
Modified 13 years, 9 months ago
Reviewers: Lawrence Crowl, Diego Novillo
Base URL:
Comments: 0