From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28132 invoked by alias); 25 Apr 2014 16:31:28 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 28118 invoked by uid 89); 25 Apr 2014 16:31:27 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-ee0-f49.google.com Received: from mail-ee0-f49.google.com (HELO mail-ee0-f49.google.com) (74.125.83.49) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Fri, 25 Apr 2014 16:31:25 +0000 Received: by mail-ee0-f49.google.com with SMTP id c41so2932075eek.22 for ; Fri, 25 Apr 2014 09:31:21 -0700 (PDT) X-Received: by 10.14.215.198 with SMTP id e46mr1820698eep.85.1398443481729; Fri, 25 Apr 2014 09:31:21 -0700 (PDT) Received: from [192.168.1.12] (ppp-94-65-246-173.home.otenet.gr. [94.65.246.173]) by mx.google.com with ESMTPSA id a4sm26051426eep.12.2014.04.25.09.31.19 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 25 Apr 2014 09:31:20 -0700 (PDT) Message-ID: <535A8EA8.5040706@gmail.com> Date: Fri, 25 Apr 2014 16:50:00 -0000 From: Dimitris Papavasiliou User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0.12) Gecko/20130116 Icedove/10.0.12 MIME-Version: 1.0 To: Mike Stump CC: "gcc-patches@gcc.gnu.org Patches" Subject: Re: [PATCH] Add a couple of dialect and warning options regarding Objective-C instance variable scope References: <52F35508.4080205@gmail.com> <5486E33D-B8F6-473F-8F33-6A7DEECB45A8@comcast.net> <5359999C.7090607@gmail.com> <757BB622-2CA8-4C9A-BCA9-AAD1FDF8F14F@comcast.net> In-Reply-To: <757BB622-2CA8-4C9A-BCA9-AAD1FDF8F14F@comcast.net> Content-Type: multipart/mixed; boundary="------------010907050103020406090506" X-IsSubscribed: yes X-SW-Source: 2014-04/txt/msg01692.txt.bz2 This is a multi-part message in MIME format. --------------010907050103020406090506 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit Content-length: 1885 On 04/25/2014 04:07 AM, Mike Stump wrote: > On Apr 24, 2014, at 4:09 PM, Dimitris Papavasiliou wrote: >> On 04/24/2014 07:00 PM, Mike Stump wrote: >>> On Feb 6, 2014, at 1:25 AM, Dimitris Papavasiliou wrote: >>>> This is a patch regarding a couple of Objective-C related dialect options and warning switches. >>> >>> Ok. >>> >>> Committed revision 209753. >>> >>> If you could, please add documentation and a test case. >> >> Thanks for taking the time to look at this, although to be honest I didn't expect a straight merge into the trunk. > > Don’t submit changes you don’t want! :-) > >> I'll add documentation and test cases as soon as I figure out how. > > Just copy testsuite/objc.dg/private-2.m into shadow-1.m and then `fix’ it to test what you want. If you need one with and one without a flag, copy it twice and use something like: > > // { dg-options "-Wshadow" } > > on it. Type make RUNTESTFLAGS=dg.exp=shadow-1.m check-objc to test it. > > For the doc, just find a simple option in the part of the manual you want to put it in, and copy it. We document the non-default, (Wno-shadow-ivar for example) options. I'm attaching three additional patches: documentation.patch: This adds documentation for the three additional switches plus a small change to reflect the fact that -Wshadow now controls instance variable shadowing as well. tests.patch: This adds 9 test cases that test both the intended behavior of the added switches as well as the default behavior in their absence. enabledby_wshadow.patch: This adds the proposed change to allow specifying -Wno-shadow to turn off -Wshadow-ivar as well plus one more test to make sure that it happens. I've run make check-objc with all these changes and everything seems to be fine. Let me know if you need anything else and thanks again for your time. Dimitris --------------010907050103020406090506 Content-Type: text/x-patch; name="documentation.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="documentation.patch" Content-length: 2872 Index: gcc/doc/invoke.texi =================================================================== --- gcc/doc/invoke.texi (revision 209787) +++ gcc/doc/invoke.texi (working copy) @@ -216,6 +216,8 @@ -fobjc-gc @gol -fobjc-nilcheck @gol -fobjc-std=objc1 @gol +-fno-local-ivars @gol +-fivar-visibility=@var{public|protected|private|package} @gol -freplace-objc-classes @gol -fzero-link @gol -gen-decls @gol @@ -261,7 +263,7 @@ -Wparentheses -Wpedantic-ms-format -Wno-pedantic-ms-format @gol -Wpointer-arith -Wno-pointer-to-int-cast @gol -Wredundant-decls -Wno-return-local-addr @gol --Wreturn-type -Wsequence-point -Wshadow @gol +-Wreturn-type -Wsequence-point -Wshadow -Wshadow-ivar @gol -Wsign-compare -Wsign-conversion -Wfloat-conversion @gol -Wsizeof-pointer-memaccess @gol -Wstack-protector -Wstack-usage=@var{len} -Wstrict-aliasing @gol @@ -2975,6 +2977,21 @@ The GNU runtime currently always retains calls to @code{objc_get_class("@dots{}")} regardless of command-line options. +@item -fno-local-ivars +@opindex fno-local-ivars +By default instance variables in Objective-C can be accessed as if +they were local variables from within the methods of the class they're +declared in. This can lead to shadowing between instance variables +and other variables declared either locally inside a class method or +globally with the same name. Specifying the @option{-fno-local-ivars} +flag disables this behavior thus avoiding variable shadowing issues. + +@item -fivar-visibility=@var{public|protected|private|package} +@opindex fivar-visibility +Set the default instance variable visibility to the specified option +so that instance variables declared outside the scope of any access +modifier directives default to the specified visibility. + @item -gen-decls @opindex gen-decls Dump interface declarations for all classes seen in the source file to a @@ -4349,11 +4366,18 @@ @item -Wshadow @opindex Wshadow @opindex Wno-shadow -Warn whenever a local variable or type declaration shadows another variable, -parameter, type, or class member (in C++), or whenever a built-in function -is shadowed. Note that in C++, the compiler warns if a local variable -shadows an explicit typedef, but not if it shadows a struct/class/enum. +Warn whenever a local variable or type declaration shadows another +variable, parameter, type, class member (in C++), or instance variable +(in Objective-C) or whenever a built-in function is shadowed. Note +that in C++, the compiler warns if a local variable shadows an +explicit typedef, but not if it shadows a struct/class/enum. +@item -Wshadow-ivar @r{(Objective-C only)} +@opindex Wshadow-ivar +@opindex Wno-shadow-ivar +Warn whenever a local variable shadows an instance variable in an +Objective-C method. + @item -Wlarger-than=@var{len} @opindex Wlarger-than=@var{len} @opindex Wlarger-than-@var{len} --------------010907050103020406090506 Content-Type: text/x-patch; name="enabledby_wshadow.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="enabledby_wshadow.patch" Content-length: 1417 Index: gcc/testsuite/objc.dg/shadow-2.m =================================================================== --- gcc/testsuite/objc.dg/shadow-2.m (revision 0) +++ gcc/testsuite/objc.dg/shadow-2.m (revision 0) @@ -0,0 +1,33 @@ +/* Test disabling of warnings for shadowing instance variables. */ +/* Author: Dimitris Papavasiliou . */ +/* { dg-do compile } */ +/* { dg-additional-options "-Wno-shadow" } */ +#include + +@interface MyClass +{ +@private + int private; + +@protected + int protected; + +@public + int public; +} +- (void) test; +@end + +@implementation MyClass +- (void) test +{ + int private = 12; + int protected = 12; + int public = 12; + int a; + + a = private; /* { dg-warning "hides instance variable" "" { xfail *-*-* } } */ + a = protected; /* { dg-warning "hides instance variable" "" { xfail *-*-* } } */ + a = public; /* { dg-warning "hides instance variable" "" { xfail *-*-* } } */ +} +@end Index: gcc/c-family/c.opt =================================================================== --- gcc/c-family/c.opt (revision 209787) +++ gcc/c-family/c.opt (working copy) @@ -685,7 +685,7 @@ Warn if a selector has multiple methods Wshadow-ivar -ObjC ObjC++ Var(warn_shadow_ivar) Init(1) Warning +ObjC ObjC++ Var(warn_shadow_ivar) EnabledBy(Wshadow) Init(1) Warning Warn if a local declaration hides an instance variable Wsequence-point --------------010907050103020406090506 Content-Type: text/x-patch; name="tests.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="tests.patch" Content-length: 8817 Index: gcc/testsuite/objc.dg/shadow-1.m =================================================================== --- gcc/testsuite/objc.dg/shadow-1.m (revision 0) +++ gcc/testsuite/objc.dg/shadow-1.m (revision 0) @@ -0,0 +1,33 @@ +/* Test disabling of warnings for shadowing instance variables. */ +/* Author: Dimitris Papavasiliou . */ +/* { dg-do compile } */ +/* { dg-additional-options "-Wno-shadow-ivar" } */ +#include + +@interface MyClass +{ +@private + int private; + +@protected + int protected; + +@public + int public; +} +- (void) test; +@end + +@implementation MyClass +- (void) test +{ + int private = 12; + int protected = 12; + int public = 12; + int a; + + a = private; /* { dg-warning "hides instance variable" "" { xfail *-*-* } } */ + a = protected; /* { dg-warning "hides instance variable" "" { xfail *-*-* } } */ + a = public; /* { dg-warning "hides instance variable" "" { xfail *-*-* } } */ +} +@end Index: gcc/testsuite/objc.dg/ivar-scope-1.m =================================================================== --- gcc/testsuite/objc.dg/ivar-scope-1.m (revision 0) +++ gcc/testsuite/objc.dg/ivar-scope-1.m (revision 0) @@ -0,0 +1,21 @@ +/* Test instance variable scope. */ +/* Author: Dimitris Papavasiliou . */ +/* { dg-do compile } */ +#include + +@interface MyClass +{ + int someivar; +} +- (void) test; +@end + +@implementation MyClass +- (void) test +{ + int a; + + a = self->someivar; /* Ok */ + a = someivar; /* { dg-error ".someivar. undeclared" "" { xfail *-*-* } } */ +} +@end Index: gcc/testsuite/objc.dg/ivar-scope-2.m =================================================================== --- gcc/testsuite/objc.dg/ivar-scope-2.m (revision 0) +++ gcc/testsuite/objc.dg/ivar-scope-2.m (revision 0) @@ -0,0 +1,34 @@ +/* Test instance variable scope. */ +/* Author: Dimitris Papavasiliou . */ +/* { dg-do compile } */ +/* { dg-additional-options "-fno-local-ivars" } */ +#include + +@interface MyClass +{ + int someivar; +} +- (void) testscope; +- (void) testshadowing; +@end + +@implementation MyClass +- (void) testscope +{ + int a; + + a = self->someivar; /* Ok */ + a = someivar; /* { dg-error ".someivar. undeclared" } */ +} + +- (void) testshadowing +{ + int someivar = 1; + int a; + + /* Since instance variables don't have local scope no shadowing + should occur. */ + + a = someivar; /* { dg-warning "hides instance variable" "" { xfail *-*-* } } */ +} +@end Index: gcc/testsuite/objc.dg/ivar-scope-3.m =================================================================== --- gcc/testsuite/objc.dg/ivar-scope-3.m (revision 0) +++ gcc/testsuite/objc.dg/ivar-scope-3.m (revision 0) @@ -0,0 +1,60 @@ +/* Test instance variable scope. */ +/* Author: Dimitris Papavasiliou . */ +/* { dg-do run } */ +/* { dg-additional-options "-Wno-shadow-ivar" } */ +#include "../objc-obj-c++-shared/TestsuiteObject.m" +#include + +extern void abort(void); + +int someivar = 1; + +@interface MyClass: TestsuiteObject +{ + int someivar; +} +- (int) get; +- (int) getHidden; +@end + +@implementation MyClass +- init +{ + someivar = 2; + + return self; +} + +- (int) get +{ + return someivar; +} + +- (int) getHidden +{ + int someivar = 3; + + return someivar; +} +@end + +int main(void) +{ + MyClass *object; + + object = [[MyClass alloc] init]; + + /* Check whether the instance variable hides the global variable. */ + + if ([object get] != 2) { + abort(); + } + + /* Check whether the local variable hides the instance variable. */ + + if ([object getHidden] != 3) { + abort(); + } + + return 0; +} Index: gcc/testsuite/objc.dg/ivar-scope-4.m =================================================================== --- gcc/testsuite/objc.dg/ivar-scope-4.m (revision 0) +++ gcc/testsuite/objc.dg/ivar-scope-4.m (revision 0) @@ -0,0 +1,86 @@ +/* Test instance variable scope. */ +/* Author: Dimitris Papavasiliou . */ +/* { dg-do run } */ +/* { dg-additional-options "-Wno-shadow-ivar -fno-local-ivars" } */ +#include "../objc-obj-c++-shared/runtime.h" +#include + +extern void abort(void); + +int someivar = 1; + +@interface MyClass +{ + Class isa; + int someivar; +} + ++ (id) new; ++ (id) alloc; +- (id) init; +- (int) getGlobal; +- (int) getInstance; +- (int) getHidden; +@end + +@implementation MyClass ++ (id) new +{ + return [[self alloc] init]; +} + ++ (id) alloc +{ + return class_createInstance (self, 0); +} + +- (id) init +{ + self->someivar = 2; + + return self; +} + +- (int) getGlobal +{ + return someivar; +} + +- (int) getInstance +{ + return self->someivar; +} + +- (int) getHidden +{ + int someivar = 3; + + return someivar; +} +@end + +int main(void) +{ + id object; + + object = [MyClass new]; + + /* Check for aliasing between instance variable and global + variable. */ + + if ([object getGlobal] != 1) { + abort(); + } + + if ([object getInstance] != 2) { + abort(); + } + + /* Check wheter the local variable hides the instance variable. */ + + if ([object getHidden] != 3) { + abort(); + } + + return 0; +} Index: gcc/testsuite/objc.dg/ivar-visibility-1.m =================================================================== --- gcc/testsuite/objc.dg/ivar-visibility-1.m (revision 0) +++ gcc/testsuite/objc.dg/ivar-visibility-1.m (revision 0) @@ -0,0 +1,33 @@ +/* Test instance variable visibility. */ +/* Author: Dimitris Papavasiliou . */ +/* { dg-do compile } */ +#include + +@interface MySuperClass +{ + int someivar; +} +@end + +@implementation MySuperClass +@end + + +@interface MyClass : MySuperClass +@end + +@implementation MyClass +@end + +@interface MyOtherClass +- (void) test: (MyClass *) object; +@end + +@implementation MyOtherClass +- (void) test: (MyClass *) object +{ + int a; + + a = object->someivar; /* { dg-error "instance variable .someivar. is declared protected" } */ +} +@end Index: gcc/testsuite/objc.dg/ivar-visibility-2.m =================================================================== --- gcc/testsuite/objc.dg/ivar-visibility-2.m (revision 0) +++ gcc/testsuite/objc.dg/ivar-visibility-2.m (revision 0) @@ -0,0 +1,34 @@ +/* Test instance variable visibility. */ +/* Author: Dimitris Papavasiliou . */ +/* { dg-do compile } */ +/* { dg-additional-options "-fivar-visibility=protected" } */ +#include + +@interface MySuperClass +{ + int someivar; +} +@end + +@implementation MySuperClass +@end + + +@interface MyClass : MySuperClass +@end + +@implementation MyClass +@end + +@interface MyOtherClass +- (void) test: (MyClass *) object; +@end + +@implementation MyOtherClass +- (void) test: (MyClass *) object +{ + int a; + + a = object->someivar; /* { dg-error "instance variable .someivar. is declared protected" } */ +} +@end Index: gcc/testsuite/objc.dg/ivar-visibility-3.m =================================================================== --- gcc/testsuite/objc.dg/ivar-visibility-3.m (revision 0) +++ gcc/testsuite/objc.dg/ivar-visibility-3.m (revision 0) @@ -0,0 +1,34 @@ +/* Test instance variable visibility. */ +/* Author: Dimitris Papavasiliou . */ +/* { dg-do compile } */ +/* { dg-additional-options "-fivar-visibility=private" } */ +#include + +@interface MySuperClass +{ + int someivar; +} +@end + +@implementation MySuperClass +@end + + +@interface MyClass : MySuperClass +@end + +@implementation MyClass +@end + +@interface MyOtherClass +- (void) test: (MyClass *) object; +@end + +@implementation MyOtherClass +- (void) test: (MyClass *) object +{ + int a; + + a = object->someivar; /* { dg-error "instance variable .someivar. is declared private" } */ +} +@end Index: gcc/testsuite/objc.dg/ivar-visibility-4.m =================================================================== --- gcc/testsuite/objc.dg/ivar-visibility-4.m (revision 0) +++ gcc/testsuite/objc.dg/ivar-visibility-4.m (revision 0) @@ -0,0 +1,34 @@ +/* Test instance variable visibility. */ +/* Author: Dimitris Papavasiliou . */ +/* { dg-do compile } */ +/* { dg-additional-options "-fivar-visibility=public" } */ +#include + +@interface MySuperClass +{ + int someivar; +} +@end + +@implementation MySuperClass +@end + + +@interface MyClass : MySuperClass +@end + +@implementation MyClass +@end + +@interface MyOtherClass +- (void) test: (MyClass *) object; +@end + +@implementation MyOtherClass +- (void) test: (MyClass *) object +{ + int a; + + a = object->someivar; /* { dg-error "instance variable .someivar. is declared (private|protected)" "" { xfail *-*-* } } */ +} +@end --------------010907050103020406090506--