From patchwork Sat Jul 9 01:29:35 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gab Charette X-Patchwork-Id: 103954 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 3176F1007D4 for ; Sat, 9 Jul 2011 11:29:55 +1000 (EST) Received: (qmail 13863 invoked by alias); 9 Jul 2011 01:29:54 -0000 Received: (qmail 13855 invoked by uid 22791); 9 Jul 2011 01:29:53 -0000 X-SWARE-Spam-Status: No, hits=-1.5 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, SPF_HELO_PASS, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (74.125.121.67) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 09 Jul 2011 01:29:39 +0000 Received: from wpaz17.hot.corp.google.com (wpaz17.hot.corp.google.com [172.24.198.81]) by smtp-out.google.com with ESMTP id p691TbKF015596; Fri, 8 Jul 2011 18:29:38 -0700 Received: from gchare.mtv.corp.google.com (gchare.mtv.corp.google.com [172.18.111.122]) by wpaz17.hot.corp.google.com with ESMTP id p691TZ7S018316; Fri, 8 Jul 2011 18:29:36 -0700 Received: by gchare.mtv.corp.google.com (Postfix, from userid 138564) id B28761C3720; Fri, 8 Jul 2011 18:29:35 -0700 (PDT) To: reply@codereview.appspotmail.com, crowl@google.com, dnovillo@google.com, gcc-patches@gcc.gnu.org Subject: [pph] Remove protection for NAMESPACE_LEVEL being null when adding namespaces (issue4675069) Message-Id: <20110709012935.B28761C3720@gchare.mtv.corp.google.com> Date: Fri, 8 Jul 2011 18:29:35 -0700 (PDT) From: gchare@google.com (Gabriel Charette) X-System-Of-Record: true X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org 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 * gcc/cp/pph-streamer-in.c (pph_add_bindings_to_namespace): NAMESPACE_LEVEL should never be null for a namespace, removed check. --- This patch is available for review at http://codereview.appspot.com/4675069 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); } }