From patchwork Thu Nov 25 19:08:23 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicola Pero X-Patchwork-Id: 73116 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 D3012B70AA for ; Fri, 26 Nov 2010 06:09:20 +1100 (EST) Received: (qmail 26022 invoked by alias); 25 Nov 2010 19:09:15 -0000 Received: (qmail 26009 invoked by uid 22791); 25 Nov 2010 19:09:11 -0000 X-SWARE-Spam-Status: No, hits=-0.0 required=5.0 tests=AWL, BAYES_50, TW_BJ, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from fencepost.gnu.org (HELO fencepost.gnu.org) (140.186.70.10) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 25 Nov 2010 19:09:05 +0000 Received: from eggs.gnu.org ([140.186.70.92]:60932) by fencepost.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.69) (envelope-from ) id 1PLhBl-0001kH-EP for gcc-patches@gnu.org; Thu, 25 Nov 2010 14:09:01 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PLhBk-0002J1-7c for gcc-patches@gnu.org; Thu, 25 Nov 2010 14:09:03 -0500 Received: from smtp131.iad.emailsrvr.com ([207.97.245.131]:58286) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PLhBk-0002Iu-4s for gcc-patches@gnu.org; Thu, 25 Nov 2010 14:09:00 -0500 Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp43.relay.iad1a.emailsrvr.com (SMTP Server) with ESMTP id 0BC652D0395 for ; Thu, 25 Nov 2010 14:08:24 -0500 (EST) Received: from dynamic3.wm-web.iad.mlsrvr.com (dynamic3.wm-web.iad1a.rsapps.net [192.168.2.152]) by smtp43.relay.iad1a.emailsrvr.com (SMTP Server) with ESMTP id EE0522D0385 for ; Thu, 25 Nov 2010 14:08:23 -0500 (EST) Received: from meta-innovation.com (localhost [127.0.0.1]) by dynamic3.wm-web.iad.mlsrvr.com (Postfix) with ESMTP id E026B332006E for ; Thu, 25 Nov 2010 14:08:23 -0500 (EST) Received: by www2.webmail.us (Authenticated sender: nicola.pero@meta-innovation.com, from: nicola.pero@meta-innovation.com) with HTTP; Thu, 25 Nov 2010 20:08:23 +0100 (CET) Date: Thu, 25 Nov 2010 20:08:23 +0100 (CET) Subject: ObjC/ObjC++: fix for classes with more than 15 instance variables From: "Nicola Pero" To: "gcc-patches@gnu.org" MIME-Version: 1.0 X-Type: plain Message-ID: <1290712103.915816754@192.168.2.228> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) 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 This patch fixes a bug in the Objective-C compiler that would cause root classes with more than 15 instance variables to trigger spurious warnings. The patch includes a testcase that shows a spurious warning with a root class; that's how I found the bug, but looking at the code it seems likely that you may get other spurious warnings with non-root classes if they have over 15 instance variables (I don't have a testcase for that handy though). The reason for the bug is that when the struct associated with the class is built, the C frontend generates an internal sorted list of the fields if there are more than 15 fields (that is where the otherwise puzzling limit of 15 instance variables come from). That sorting is then stored in lang_type, which is the same place where the ObjC info for interfaces and protocols is stored, and storing the sorting info wipes the Objc info out. The ObjC frontend already had code to restore the protocol info associated with type variants. Unfortunately, it didn't have code to restore the actual interface that the new struct is associated with, so it would no longer be associated with it, and you'd get spurious warnings when trying to use it as the type-checking would no longer work ;-) This patch fixes it, and includes testcases. Ok to commit ? Thanks PS: I think the whole lang_type/objc_info thing should be revisited, but that's not for stage 3. This patch simply fits in the existing framework and fixes the immediate bug. Index: objc/objc-act.c =================================================================== --- objc/objc-act.c (revision 167099) +++ objc/objc-act.c (working copy) @@ -2137,10 +2137,15 @@ objc_build_struct (tree klass, tree fields, tree s fields = base; } - /* NB: Calling finish_struct() may cause type TYPE_LANG_SPECIFIC fields - in all variants of this RECORD_TYPE to be clobbered, but it is therein - that we store protocol conformance info (e.g., 'NSObject '). - Hence, we must squirrel away the ObjC-specific information before calling + /* NB: Calling finish_struct() may cause type TYPE_LANG_SPECIFIC + fields in all variants of this RECORD_TYPE to be clobbered (this + is because the C frontend stores a sorted version of the list of + fields in lang_type if it deems appropriate, and will update and + propagate that list to all variants ignoring the fact that we use + lang_type for something else and that such propagation will wipe + the objc_info away), but it is therein that we store protocol + conformance info (e.g., 'NSObject '). Hence, we must + squirrel away the ObjC-specific information before calling finish_struct(), and then reinstate it afterwards. */ for (t = TYPE_NEXT_VARIANT (s); t; t = TYPE_NEXT_VARIANT (t)) @@ -2153,12 +2158,16 @@ objc_build_struct (tree klass, tree fields, tree s VEC_safe_push (tree, heap, objc_info, TYPE_OBJC_INFO (t)); } - /* Point the struct at its related Objective-C class. */ - INIT_TYPE_OBJC_INFO (s); + s = objc_finish_struct (s, fields); + + /* Point the struct at its related Objective-C class. We do this + after calling finish_struct() because otherwise finish_struct() + would wipe TYPE_OBJC_INTERFACE() out. */ + if (!TYPE_HAS_OBJC_INFO (s)) + INIT_TYPE_OBJC_INFO (s); + TYPE_OBJC_INTERFACE (s) = klass; - s = objc_finish_struct (s, fields); - for (i = 0, t = TYPE_NEXT_VARIANT (s); t; t = TYPE_NEXT_VARIANT (t), i++) { TYPE_OBJC_INFO (t) = VEC_index (tree, objc_info, i); Index: objc/ChangeLog =================================================================== --- objc/ChangeLog (revision 167099) +++ objc/ChangeLog (working copy) @@ -1,3 +1,10 @@ +2010-11-25 Nicola Pero + + * objc-act.c (objc_build_struct): Install TYPE_OBJC_INTERFACE + after finish_struct, not before, otherwise it may be wiped out by + it. This fixes spurious warnings when a class has more than 15 + instance variables. + 2010-11-23 Nicola Pero PR objc/24358 Index: testsuite/ChangeLog =================================================================== --- testsuite/ChangeLog (revision 167099) +++ testsuite/ChangeLog (working copy) @@ -1,3 +1,8 @@ +2010-11-25 Nicola Pero + + * objc.dg/ivar-problem-1.m: New. + * obj-c++.dg/ivar-problem-1.mm: New. + 2010-11-23 Iain Sandoe * gcc.dg/darwin-cfstring-1.c: Adjust format messages. Index: testsuite/objc.dg/ivar-problem-1.m =================================================================== --- testsuite/objc.dg/ivar-problem-1.m (revision 0) +++ testsuite/objc.dg/ivar-problem-1.m (revision 0) @@ -0,0 +1,65 @@ +/* Contributed by Nicola Pero , November 2010. */ +/* { dg-do compile } */ + +/* This test checks what happens if there are 16 instance variables. + In that case, the class was not created correctly. In this testcase, + we have two classes, one with 15 variables and one with 16. Older + GCCs would generate a bogus warning for the second class but not + for the first one. */ + +#include +#include + +@interface MyRootClass1 +{ + Class isa; + int v2; + int v3; + int v4; + int v5; + int v6; + int v7; + int v8; + int v9; + int v10; + int v11; + int v12; + int v13; + int v14; + int v15; +} +- (id) init; +@end + +@implementation MyRootClass1 +- (id) init { return self; } +@end + + +@interface MyRootClass2 +{ + Class isa; + int v2; + int v3; + int v4; + int v5; + int v6; + int v7; + int v8; + int v9; + int v10; + int v11; + int v12; + int v13; + int v14; + int v15; + /* Adding the 16th variable used to cause bogus warnings to be + generated. */ + int v16; +} +- (id) init; +@end + +@implementation MyRootClass2 +- (id) init { return self; } /* This should not generate a bogus warning. */ +@end Index: testsuite/obj-c++.dg/ivar-problem-1.mm =================================================================== --- testsuite/obj-c++.dg/ivar-problem-1.mm (revision 0) +++ testsuite/obj-c++.dg/ivar-problem-1.mm (revision 0) @@ -0,0 +1,66 @@ +/* Contributed by Nicola Pero , November 2010. */ +/* { dg-do compile } */ + +/* This test checks what happens if there are 16 instance variables. + In that case, the class was not created correctly. In this testcase, + we have two classes, one with 15 variables and one with 16. Older + GCCs would generate a bogus warning for the second class but not + for the first one. This only happened for ObjC, but it's good to + test ObjC++ as well. */ + +#include +#include + +@interface MyRootClass1 +{ + Class isa; + int v2; + int v3; + int v4; + int v5; + int v6; + int v7; + int v8; + int v9; + int v10; + int v11; + int v12; + int v13; + int v14; + int v15; +} +- (id) init; +@end + +@implementation MyRootClass1 +- (id) init { return self; } +@end + + +@interface MyRootClass2 +{ + Class isa; + int v2; + int v3; + int v4; + int v5; + int v6; + int v7; + int v8; + int v9; + int v10; + int v11; + int v12; + int v13; + int v14; + int v15; + /* Adding the 16th variable used to cause bogus warnings to be + generated. */ + int v16; +} +- (id) init; +@end + +@implementation MyRootClass2 +- (id) init { return self; } /* This should not generate a bogus warning. */ +@end