[PATCH] Add a couple of dialect and warning options regarding Objective-C instance variable scope

[PATCH] Add a couple of dialect and warning options regarding Objective-C instance variable scope

[Dimitris Papavasiliou]

[-- Attachment #1: Type: text/plain, Size: 3170 bytes --]

Hello,

First, let me say that I have consciously broken most of the rules 
mentioned about patch submission at gcc.gnu.org but I have done so in 
order to spare myself from wasting time to provide a proper patch in 
case the implemented functionality is not deemed worthy of approval and 
adoption into GCC.  If any of the implemented switches prove to be 
welcome I'll be more than happy to split them into separate patches, add 
test-cases and add ChangLog entries as needed.

Two of these switches are related to a feature request I submitted a 
while ago, Bug 56044 
(http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56044).  I won't reproduce 
the entire argument here since it is available in the feature request. 
The relevant functionality in the patch comes in the form of two switches:

-Wshadow-ivars which controls the "local declaration of ‘somevar’ hides 
instance variable" warning which curiously is enabled by default instead 
of being controlled at least by -Wshadow.  The patch changes it so that 
this warning can be enabled and disabled specifically through 
-Wshadow-ivars as well as with all other shadowing-related warnings 
through -Wshadow.

The reason for the extra switch is that, while searching through the 
Internet for a solution to this problem I have found out that other 
people are inconvenienced by this particular warning as well so it might 
be useful to be able to turn it off while keeping all the other 
shadowing-related warnings enabled.

-flocal-ivars which when true, as it is by default, treats instance 
variables as having local scope.  If false (-fno-local-ivars) instance 
variables must always be referred to as self->ivarname and references of 
ivarname resolve to the local or global scope as usual.

I've also taken the opportunity of adding another switch unrelated to 
the above but related to instance variables:

-fivar-visibility which can be set to either private, protected (the 
default), public and package.  This sets the default instance variable 
visibility which normally is implicitly protected.  My use-case for it 
is basically to be able to set it to public and thus effectively disable 
this visibility mechanism altogether which I find no use for and 
therefore have to circumvent.  I'm not sure if anyone else feels the 
same way towards this but I figured it was worth a try.

I'm attaching a preliminary patch against the current revision in case 
anyone wants to have a look.  The changes are very small and any blatant 
mistakes should be immediately obvious.  I have to admit to having 
virtually no knowledge of the internals of GCC but I have tried to keep 
in line with formatting guidelines and general style as well as looking 
up the particulars of the way options are handled in the available 
documentation to avoid blind copy-pasting.  I have also tried to test 
the functionality both in my own (relatively large, or at least not too 
small) project and with small test programs and everything works as 
expected.  Finallly, I tried running the tests too but these fail to 
complete both in the patched and unpatched version, possibly due to the 
way I've configured GCC.

Dimitris

[-- Attachment #2: objc_ivar_scope.diff --]
[-- Type: text/x-patch, Size: 4828 bytes --]

Index: gcc/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt	(revision 199867)
+++ gcc/c-family/c.opt	(working copy)
@@ -661,6 +661,10 @@ Wselector
 ObjC ObjC++ Var(warn_selector) Warning
 Warn if a selector has multiple methods
 
+Wshadow-ivar
+ObjC ObjC++ Var(warn_shadow_ivar) Init(-1) Warning
+Warn if a local declaration hides an instance variable
+
 Wsequence-point
 C ObjC C++ ObjC++ Var(warn_sequence_point) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
 Warn about possible violations of sequence point rules
@@ -1018,6 +1022,29 @@ fnil-receivers
 ObjC ObjC++ Var(flag_nil_receivers) Init(1)
 Assume that receivers of Objective-C messages may be nil
 
+flocal-ivars
+ObjC ObjC++ Var(flag_local_ivars) Init(1)
+Allow access to instance variables as if they were local declarations within instance method implementations.
+
+fivar-visibility=
+ObjC ObjC++ Joined RejectNegative Enum(ivar_visibility) Var(default_ivar_visibility) Init(IVAR_VISIBILITY_PROTECTED)
+-fvisibility=[private|protected|public|package]	Set the default symbol visibility
+
+Enum
+Name(ivar_visibility) Type(enum ivar_visibility) UnknownError(unrecognized ivar visibility value %qs)
+
+EnumValue
+Enum(ivar_visibility) String(private) Value(IVAR_VISIBILITY_PRIVATE)
+
+EnumValue
+Enum(ivar_visibility) String(protected) Value(IVAR_VISIBILITY_PROTECTED)
+
+EnumValue
+Enum(ivar_visibility) String(public) Value(IVAR_VISIBILITY_PUBLIC)
+
+EnumValue
+Enum(ivar_visibility) String(package) Value(IVAR_VISIBILITY_PACKAGE)
+
 fnonansi-builtins
 C++ ObjC++ Var(flag_no_nonansi_builtin, 0)
 
Index: gcc/flag-types.h
===================================================================
--- gcc/flag-types.h	(revision 199867)
+++ gcc/flag-types.h	(working copy)
@@ -104,6 +104,16 @@ enum symbol_visibility
 };
 #endif
 
+/* Enumerate Objective-c instance variable visibility settings. */
+
+enum ivar_visibility
+{
+  IVAR_VISIBILITY_PRIVATE,
+  IVAR_VISIBILITY_PROTECTED,
+  IVAR_VISIBILITY_PUBLIC,
+  IVAR_VISIBILITY_PACKAGE
+};
+
 /* The stack reuse level.  */
 enum stack_reuse_level
 {
Index: gcc/objc/objc-act.c
===================================================================
--- gcc/objc/objc-act.c	(revision 199867)
+++ gcc/objc/objc-act.c	(working copy)
@@ -223,7 +223,7 @@ struct imp_entry *imp_list = 0;
 int imp_count = 0;	/* `@implementation' */
 int cat_count = 0;	/* `@category' */
 
-objc_ivar_visibility_kind objc_ivar_visibility;
+objc_ivar_visibility_kind objc_ivar_visibility, objc_default_ivar_visibility;
 
 /* Use to generate method labels.  */
 static int method_slot = 0;
@@ -391,6 +391,22 @@ objc_init (void)
   if (!ok)
     return false;
 
+  /* Determine the default visibility for instance variables. */
+  switch (default_ivar_visibility)
+    {
+    case IVAR_VISIBILITY_PRIVATE:
+        objc_default_ivar_visibility = OBJC_IVAR_VIS_PRIVATE;
+        break;
+    case IVAR_VISIBILITY_PUBLIC:
+        objc_default_ivar_visibility = OBJC_IVAR_VIS_PUBLIC;
+        break;
+    case IVAR_VISIBILITY_PACKAGE:
+        objc_default_ivar_visibility = OBJC_IVAR_VIS_PACKAGE;
+        break;
+    default:
+        objc_default_ivar_visibility = OBJC_IVAR_VIS_PROTECTED;
+    }
+      
   /* Generate general types and push runtime-specific decls to file scope.  */
   synth_module_prologue ();
 
@@ -565,7 +581,7 @@ objc_start_class_interface (tree klass,
   objc_interface_context
     = objc_ivar_context
     = start_class (CLASS_INTERFACE_TYPE, klass, super_class, protos, attributes);
-  objc_ivar_visibility = OBJC_IVAR_VIS_PROTECTED;
+  objc_ivar_visibility = objc_default_ivar_visibility;
 }
 
 void
@@ -643,7 +659,7 @@ objc_start_class_implementation (tree kl
     = objc_ivar_context
     = start_class (CLASS_IMPLEMENTATION_TYPE, klass, super_class, NULL_TREE,
 		   NULL_TREE);
-  objc_ivar_visibility = OBJC_IVAR_VIS_PROTECTED;
+  objc_ivar_visibility = objc_default_ivar_visibility;
 }
 
 void
@@ -9360,6 +9376,12 @@ objc_lookup_ivar (tree other, tree id)
       && other && other != error_mark_node)
     return other;
 
+  /* Don't look up the ivar if the user has explicitly advised against
+   * it with -fno-local-ivars. */
+
+  if (!flag_local_ivars)
+    return other;
+
   /* Look up the ivar, but do not use it if it is not accessible.  */
   ivar = is_ivar (objc_ivar_chain, id);
 
@@ -9376,8 +9398,11 @@ objc_lookup_ivar (tree other, tree id)
       && !DECL_FILE_SCOPE_P (other))
 #endif
     {
-      warning (0, "local declaration of %qE hides instance variable", id);
-
+      if (warn_shadow_ivar == 1 || (warn_shadow && warn_shadow_ivar != 0)) {
+          warning (warn_shadow_ivar ? OPT_Wshadow_ivar : OPT_Wshadow,
+                   "local declaration of %qE hides instance variable", id);
+      }
+        
       return other;
     }
 

Re: [PATCH] Add a couple of dialect and warning options regarding Objective-C instance variable scope

[Dimitris Papavasiliou]

Ping!  Would anybody care to comment on this?

On 06/10/2013 12:44 AM, Dimitris Papavasiliou wrote:
> Hello,
>
> First, let me say that I have consciously broken most of the rules
> mentioned about patch submission at gcc.gnu.org but I have done so in
> order to spare myself from wasting time to provide a proper patch in
> case the implemented functionality is not deemed worthy of approval and
> adoption into GCC. If any of the implemented switches prove to be
> welcome I'll be more than happy to split them into separate patches, add
> test-cases and add ChangLog entries as needed.
>
> Two of these switches are related to a feature request I submitted a
> while ago, Bug 56044
> (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56044). I won't reproduce
> the entire argument here since it is available in the feature request.
> The relevant functionality in the patch comes in the form of two switches:
>
> -Wshadow-ivars which controls the "local declaration of ‘somevar’ hides
> instance variable" warning which curiously is enabled by default instead
> of being controlled at least by -Wshadow. The patch changes it so that
> this warning can be enabled and disabled specifically through
> -Wshadow-ivars as well as with all other shadowing-related warnings
> through -Wshadow.
>
> The reason for the extra switch is that, while searching through the
> Internet for a solution to this problem I have found out that other
> people are inconvenienced by this particular warning as well so it might
> be useful to be able to turn it off while keeping all the other
> shadowing-related warnings enabled.
>
> -flocal-ivars which when true, as it is by default, treats instance
> variables as having local scope. If false (-fno-local-ivars) instance
> variables must always be referred to as self->ivarname and references of
> ivarname resolve to the local or global scope as usual.
>
> I've also taken the opportunity of adding another switch unrelated to
> the above but related to instance variables:
>
> -fivar-visibility which can be set to either private, protected (the
> default), public and package. This sets the default instance variable
> visibility which normally is implicitly protected. My use-case for it is
> basically to be able to set it to public and thus effectively disable
> this visibility mechanism altogether which I find no use for and
> therefore have to circumvent. I'm not sure if anyone else feels the same
> way towards this but I figured it was worth a try.
>
> I'm attaching a preliminary patch against the current revision in case
> anyone wants to have a look. The changes are very small and any blatant
> mistakes should be immediately obvious. I have to admit to having
> virtually no knowledge of the internals of GCC but I have tried to keep
> in line with formatting guidelines and general style as well as looking
> up the particulars of the way options are handled in the available
> documentation to avoid blind copy-pasting. I have also tried to test the
> functionality both in my own (relatively large, or at least not too
> small) project and with small test programs and everything works as
> expected. Finallly, I tried running the tests too but these fail to
> complete both in the patched and unpatched version, possibly due to the
> way I've configured GCC.
>
> Dimitris

Re: [PATCH] Add a couple of dialect and warning options regarding Objective-C instance variable scope

[Dimitris Papavasiliou]

Since there obviously is no interest in this patch I'm simply going to 
attach it to the ticket mentioned below (Bug 56044) in case someone 
decides to look into it in the future.  If anyone should want to comment 
on this please do so at the bug tracker since I probably won't be 
following this list.

Thanks,
Dimitris

On 06/24/2013 03:04 PM, Dimitris Papavasiliou wrote:
> Ping! Would anybody care to comment on this?
>
> On 06/10/2013 12:44 AM, Dimitris Papavasiliou wrote:
>> Hello,
>>
>> First, let me say that I have consciously broken most of the rules
>> mentioned about patch submission at gcc.gnu.org but I have done so in
>> order to spare myself from wasting time to provide a proper patch in
>> case the implemented functionality is not deemed worthy of approval and
>> adoption into GCC. If any of the implemented switches prove to be
>> welcome I'll be more than happy to split them into separate patches, add
>> test-cases and add ChangLog entries as needed.
>>
>> Two of these switches are related to a feature request I submitted a
>> while ago, Bug 56044
>> (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56044). I won't reproduce
>> the entire argument here since it is available in the feature request.
>> The relevant functionality in the patch comes in the form of two
>> switches:
>>
>> -Wshadow-ivars which controls the "local declaration of ‘somevar’ hides
>> instance variable" warning which curiously is enabled by default instead
>> of being controlled at least by -Wshadow. The patch changes it so that
>> this warning can be enabled and disabled specifically through
>> -Wshadow-ivars as well as with all other shadowing-related warnings
>> through -Wshadow.
>>
>> The reason for the extra switch is that, while searching through the
>> Internet for a solution to this problem I have found out that other
>> people are inconvenienced by this particular warning as well so it might
>> be useful to be able to turn it off while keeping all the other
>> shadowing-related warnings enabled.
>>
>> -flocal-ivars which when true, as it is by default, treats instance
>> variables as having local scope. If false (-fno-local-ivars) instance
>> variables must always be referred to as self->ivarname and references of
>> ivarname resolve to the local or global scope as usual.
>>
>> I've also taken the opportunity of adding another switch unrelated to
>> the above but related to instance variables:
>>
>> -fivar-visibility which can be set to either private, protected (the
>> default), public and package. This sets the default instance variable
>> visibility which normally is implicitly protected. My use-case for it is
>> basically to be able to set it to public and thus effectively disable
>> this visibility mechanism altogether which I find no use for and
>> therefore have to circumvent. I'm not sure if anyone else feels the same
>> way towards this but I figured it was worth a try.
>>
>> I'm attaching a preliminary patch against the current revision in case
>> anyone wants to have a look. The changes are very small and any blatant
>> mistakes should be immediately obvious. I have to admit to having
>> virtually no knowledge of the internals of GCC but I have tried to keep
>> in line with formatting guidelines and general style as well as looking
>> up the particulars of the way options are handled in the available
>> documentation to avoid blind copy-pasting. I have also tried to test the
>> functionality both in my own (relatively large, or at least not too
>> small) project and with small test programs and everything works as
>> expected. Finallly, I tried running the tests too but these fail to
>> complete both in the patched and unpatched version, possibly due to the
>> way I've configured GCC.
>>
>> Dimitris
>

Re: [PATCH] Add a couple of dialect and warning options regarding Objective-C instance variable scope

[Dimitris Papavasiliou]

On 05/12/2014 11:53 PM, Mike Stump wrote:
> I put in one small fix:
>
> Doing diffs in testsuite/objc.dg/ivar-visibility-4.m.~1~:
> --- testsuite/objc.dg/ivar-visibility-4.m.~1~	2014-05-12 12:04:16.000000000 -0700
> +++ testsuite/objc.dg/ivar-visibility-4.m	2014-05-12 13:50:53.000000000 -0700
> @@ -29,7 +29,7 @@
>   {
>     int a;
>
> -  /* someivar is public so we shoudn't get any errors here. */
> +  /* someivar is public so we shouldn't get any errors here. */
>
>     a = object->someivar;
>   }
> ———————
>
>

Ok, thanks for that and for all your help in general.  I tried checking 
out the current master branch and running objective-c and objective-c++ 
related tests and everything seems to be in order so hopefully that 
concludes the business with this patch.

Dimitris

Re: [PATCH] Add a couple of dialect and warning options regarding Objective-C instance variable scope

[Mike Stump]

I put in one small fix:

Doing diffs in testsuite/objc.dg/ivar-visibility-4.m.~1~:
--- testsuite/objc.dg/ivar-visibility-4.m.~1~	2014-05-12 12:04:16.000000000 -0700
+++ testsuite/objc.dg/ivar-visibility-4.m	2014-05-12 13:50:53.000000000 -0700
@@ -29,7 +29,7 @@
 {
   int a;
 
-  /* someivar is public so we shoudn't get any errors here. */
+  /* someivar is public so we shouldn't get any errors here. */
   
   a = object->someivar;
 }
———————

Re: [PATCH] Add a couple of dialect and warning options regarding Objective-C instance variable scope

[Mike Stump]

On May 12, 2014, at 11:56 AM, Dimitris Papavasiliou <dpapavas@gmail.com> wrote:
> So we [ .. ] expect that they silently shadow the instance variables, that is that warnings are _not_ generated.

Perfect, thanks.  I’ve checked it in:

Committed revision 210333.

Thanks for all your hard work.

> So I take it that I should change these to dg-bogus since this is behavior expected in the past but not now, as well as any other expected non-warnings in the other test cases?  Is it important in some way that dg-bogus makes no distinction between warning and error messages?

No.  One could, but, not necessary.

Re: [PATCH] Add a couple of dialect and warning options regarding Objective-C instance variable scope

[Dimitris Papavasiliou]

On 05/12/2014 07:24 PM, Mike Stump wrote:
> On Apr 28, 2014, at 3:35 AM, Dimitris Papavasiliou <dpapavas@gmail.com> wrote:
>>>> +  a = private;    /* { dg-warning "hides instance variable" "" { xfail *-*-* } } */
>>>> +  a = protected;  /* { dg-warning "hides instance variable" "" { xfail *-*-* } } */
>>>> +  a = public;     /* { dg-warning "hides instance variable" "" { xfail *-*-* } } */
>>>
>>> No, we donÂ’t expect failures.  We makes the compiler do what we wants or it gets the hose again.  Then, we expect it to be perfect.  If you wonÂ’t want warning, and non are produces, then just remove the /* Â… */, or write /* no warning */.
>>
>> I've fixed these as per your request.  For the record though, this form of test seems to be fairly common in the test suites as this output indicates:
>>
>> dimitris@debian:~/sandbox/gcc-build$ find ../gcc-source/gcc/testsuite/ -name "*.c" -o -name "*.C" -o -name "*.m" | xargs grep "xfail \*-\*-\*" | wc -l
>> 354
>>
>> Many of these seem to be in error or warning messages which are expected not to show up.  In any case if the messages do show up they'll trigger the excessive messages test so I suppose that's enough.
>
>>> Once we resolve the 3 warning tests above, this will be ok.
>>
>> Actually, there were a few more { xfail *-*-* } in the other test cases.  I've removed these as well.
>
> So, letÂ’s make sure weÂ’re on the same pageÂ…
>
> Take shadow-2.m for example, before you said:
>
> +  a = private;    /* { dg-warning "hides instance variable" "" { xfail *-*-* } } */
>
> and now you say:
>
> +  a = private;    /* No warning. */
>
> So, the first question is, what do we _want_ in the end here?  Warning or no warning?  And what do we actually do now, warn or not?
>
> If in the end, we want a warning, then the previous form is the right eventually form.  If we donÂ’t want a warning, then the second is correct, though, there is another form that is not unreasonable:
>
>    i += [Base class_func1];  /* { dg-bogus "invalid receiver type" } */
>
> this says that we used to generate a warning (or an error message), but that was wrong, and we no longer want to expect a warning, and that the bug has been fixed and that no warning is generated.
>
> I think we are on the same page, just wanted to make sure...
>

Well the point of this test is to make sure that the introduced -Wshadow 
also controls the warnings related to the shadowing of instance 
variables which was not the case in the past.  So we declare a couple of 
local variables of the same name and expect that they silently shadow 
the instance variables, that is that warnings are _not_ generated.

So I take it that I should change these to dg-bogus since this is 
behavior expected in the past but not now, as well as any other expected 
non-warnings in the other test cases?  Is it important in some way that 
dg-bogus makes no distinction between warning and error messages?

Re: [PATCH] Add a couple of dialect and warning options regarding Objective-C instance variable scope

[Mike Stump]

On Apr 28, 2014, at 3:35 AM, Dimitris Papavasiliou <dpapavas@gmail.com> wrote:
>>> +  a = private;    /* { dg-warning "hides instance variable" "" { xfail *-*-* } } */
>>> +  a = protected;  /* { dg-warning "hides instance variable" "" { xfail *-*-* } } */
>>> +  a = public;     /* { dg-warning "hides instance variable" "" { xfail *-*-* } } */
>> 
>> No, we don’t expect failures.  We makes the compiler do what we wants or it gets the hose again.  Then, we expect it to be perfect.  If you won’t want warning, and non are produces, then just remove the /* … */, or write /* no warning */.
> 
> I've fixed these as per your request.  For the record though, this form of test seems to be fairly common in the test suites as this output indicates:
> 
> dimitris@debian:~/sandbox/gcc-build$ find ../gcc-source/gcc/testsuite/ -name "*.c" -o -name "*.C" -o -name "*.m" | xargs grep "xfail \*-\*-\*" | wc -l
> 354
> 
> Many of these seem to be in error or warning messages which are expected not to show up.  In any case if the messages do show up they'll trigger the excessive messages test so I suppose that's enough.

>> Once we resolve the 3 warning tests above, this will be ok.
> 
> Actually, there were a few more { xfail *-*-* } in the other test cases.  I've removed these as well.

So, let’s make sure we’re on the same page…

Take shadow-2.m for example, before you said:

+  a = private;    /* { dg-warning "hides instance variable" "" { xfail *-*-* } } */

and now you say:

+  a = private;    /* No warning. */

So, the first question is, what do we _want_ in the end here?  Warning or no warning?  And what do we actually do now, warn or not?

If in the end, we want a warning, then the previous form is the right eventually form.  If we don’t want a warning, then the second is correct, though, there is another form that is not unreasonable:

  i += [Base class_func1];  /* { dg-bogus "invalid receiver type" } */

this says that we used to generate a warning (or an error message), but that was wrong, and we no longer want to expect a warning, and that the bug has been fixed and that no warning is generated.

I think we are on the same page, just wanted to make sure...

Re: [PATCH] Add a couple of dialect and warning options regarding Objective-C instance variable scope

[Dimitris Papavasiliou]

Ping!

On 05/05/2014 10:35 AM, Dimitris Papavasiliou wrote:
> Ping!
>
> On 04/28/2014 01:35 PM, Dimitris Papavasiliou wrote:
>> On 04/25/2014 07:50 PM, Mike Stump wrote:
>>> On Apr 25, 2014, at 9:34 AM, Dimitris Papavasiliou<dpapavas@gmail.com>
>>> wrote:
>>>
>>>> --Wreturn-type -Wsequence-point -Wshadow @gol
>>>> +-Wreturn-type -Wsequence-point -Wshadow -Wshadow-ivar @gol
>>>
>>> This has to be -Wno-shadow-ivar, we document the non-defaultÂ…
>>>
>>>> +@item -Wshadow-ivar @r{(Objective-C only)}
>>>
>>> Likewise.
>>>
>>>> + /* Check wheter the local variable hides the instance variable. */
>>>
>>> spelling, whether...
>>
>> Fixed these.
>>
>>>> + a = private; /* { dg-warning "hides instance variable" "" { xfail
>>>> *-*-* } } */
>>>> + a = protected; /* { dg-warning "hides instance variable" "" { xfail
>>>> *-*-* } } */
>>>> + a = public; /* { dg-warning "hides instance variable" "" { xfail
>>>> *-*-* } } */
>>>
>>> No, we donÂ’t expect failures. We makes the compiler do what we wants
>>> or it gets the hose again. Then, we expect it to be perfect. If you
>>> wonÂ’t want warning, and non are produces, then just remove the /* Â…
>>> */, or write /* no warning */.
>>
>> I've fixed these as per your request. For the record though, this form
>> of test seems to be fairly common in the test suites as this output
>> indicates:
>>
>> dimitris@debian:~/sandbox/gcc-build$ find ../gcc-source/gcc/testsuite/
>> -name "*.c" -o -name "*.C" -o -name "*.m" | xargs grep "xfail \*-\*-\*"
>> | wc -l
>> 354
>>
>> Many of these seem to be in error or warning messages which are expected
>> not to show up. In any case if the messages do show up they'll trigger
>> the excessive messages test so I suppose that's enough.
>>
>>> Also, synth up the ChnageLogsÂ… :-), they are trivial enough.
>>
>> Done.
>>
>>> And, just pop them all into one patch (cd ..; svn diff), 3 is 3x the
>>> work for me.
>>
>> Attached.
>>
>>> Once we resolve the 3 warning tests above, this will be ok.
>>
>> Actually, there were a few more { xfail *-*-* } in the other test cases.
>> I've removed these as well.
>>
>> Dimitris
>>
>

Re: [PATCH] Add a couple of dialect and warning options regarding Objective-C instance variable scope

[Dimitris Papavasiliou]

Ping!

On 04/28/2014 01:35 PM, Dimitris Papavasiliou wrote:
> On 04/25/2014 07:50 PM, Mike Stump wrote:
>> On Apr 25, 2014, at 9:34 AM, Dimitris Papavasiliou<dpapavas@gmail.com>
>> wrote:
>>
>>> --Wreturn-type -Wsequence-point -Wshadow @gol
>>> +-Wreturn-type -Wsequence-point -Wshadow -Wshadow-ivar @gol
>>
>> This has to be -Wno-shadow-ivar, we document the non-defaultÂ…
>>
>>> +@item -Wshadow-ivar @r{(Objective-C only)}
>>
>> Likewise.
>>
>>> + /* Check wheter the local variable hides the instance variable. */
>>
>> spelling, whether...
>
> Fixed these.
>
>>> + a = private; /* { dg-warning "hides instance variable" "" { xfail
>>> *-*-* } } */
>>> + a = protected; /* { dg-warning "hides instance variable" "" { xfail
>>> *-*-* } } */
>>> + a = public; /* { dg-warning "hides instance variable" "" { xfail
>>> *-*-* } } */
>>
>> No, we donÂ’t expect failures. We makes the compiler do what we wants
>> or it gets the hose again. Then, we expect it to be perfect. If you
>> wonÂ’t want warning, and non are produces, then just remove the /* Â…
>> */, or write /* no warning */.
>
> I've fixed these as per your request. For the record though, this form
> of test seems to be fairly common in the test suites as this output
> indicates:
>
> dimitris@debian:~/sandbox/gcc-build$ find ../gcc-source/gcc/testsuite/
> -name "*.c" -o -name "*.C" -o -name "*.m" | xargs grep "xfail \*-\*-\*"
> | wc -l
> 354
>
> Many of these seem to be in error or warning messages which are expected
> not to show up. In any case if the messages do show up they'll trigger
> the excessive messages test so I suppose that's enough.
>
>> Also, synth up the ChnageLogsÂ… :-), they are trivial enough.
>
> Done.
>
>> And, just pop them all into one patch (cd ..; svn diff), 3 is 3x the
>> work for me.
>
> Attached.
>
>> Once we resolve the 3 warning tests above, this will be ok.
>
> Actually, there were a few more { xfail *-*-* } in the other test cases.
> I've removed these as well.
>
> Dimitris
>

Re: [PATCH] Add a couple of dialect and warning options regarding Objective-C instance variable scope

[Dimitris Papavasiliou]

[-- Attachment #1: Type: text/plain, Size: 1860 bytes --]

On 04/25/2014 07:50 PM, Mike Stump wrote:
> On Apr 25, 2014, at 9:34 AM, Dimitris Papavasiliou<dpapavas@gmail.com>  wrote:
>
>> --Wreturn-type  -Wsequence-point  -Wshadow @gol
>> +-Wreturn-type  -Wsequence-point  -Wshadow  -Wshadow-ivar @gol
>
> This has to be -Wno-shadow-ivar, we document the non-defaultÂ…
>
>> +@item -Wshadow-ivar @r{(Objective-C only)}
>
> Likewise.
>
>> +  /* Check wheter the local variable hides the instance variable. */
>
> spelling, whether...

Fixed these.

>> +  a = private;    /* { dg-warning "hides instance variable" "" { xfail *-*-* } } */
>> +  a = protected;  /* { dg-warning "hides instance variable" "" { xfail *-*-* } } */
>> +  a = public;     /* { dg-warning "hides instance variable" "" { xfail *-*-* } } */
>
> No, we donÂ’t expect failures.  We makes the compiler do what we wants or it gets the hose again.  Then, we expect it to be perfect.  If you wonÂ’t want warning, and non are produces, then just remove the /* Â… */, or write /* no warning */.

I've fixed these as per your request.  For the record though, this form 
of test seems to be fairly common in the test suites as this output 
indicates:

dimitris@debian:~/sandbox/gcc-build$ find ../gcc-source/gcc/testsuite/ 
-name "*.c" -o -name "*.C" -o -name "*.m" | xargs grep "xfail \*-\*-\*" 
| wc -l
354

Many of these seem to be in error or warning messages which are expected 
not to show up.  In any case if the messages do show up they'll trigger 
the excessive messages test so I suppose that's enough.

> Also, synth up the ChnageLogsÂ… :-), they are trivial enough.

Done.

> And, just pop them all into one patch (cd ..; svn diff), 3 is 3x the work for me.

Attached.

> Once we resolve the 3 warning tests above, this will be ok.

Actually, there were a few more { xfail *-*-* } in the other test cases. 
  I've removed these as well.

Dimitris


[-- Attachment #2: tests_and_docs.patch --]
[-- Type: text/x-patch, Size: 14192 bytes --]

Index: gcc/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt	(revision 209852)
+++ 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
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 209852)
+++ 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  -Wno-shadow-ivar @gol
 -Wsign-compare  -Wsign-conversion -Wfloat-conversion @gol
 -Wsizeof-pointer-memaccess @gol
 -Wstack-protector -Wstack-usage=@var{len} -Wstrict-aliasing @gol
@@ -2976,6 +2978,22 @@
 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
+@opindex flocal-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
@@ -4350,11 +4368,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 -Wno-shadow-ivar @r{(Objective-C only)}
+@opindex Wno-shadow-ivar
+@opindex Wshadow-ivar
+Do not 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}
Index: gcc/ChangeLog
===================================================================
--- gcc/ChangeLog	(revision 209852)
+++ gcc/ChangeLog	(working copy)
@@ -1,3 +1,11 @@
+2014-04-28  Dimitris Papavasiliou  <dpapavas@gmail.com>
+
+	* doc/invoke.texi: Document new switches -Wno-shadow-ivar,
+	-fno-local-ivars and -fivar-visibility.
+	* c-family/c.opt: Make -Wshadow also implicitly enable
+	-Wshadow-ivar.
+
+
 2014-04-28  Eric Botcazou  <ebotcazou@adacore.com>
 
 	* configure.ac: Tweak GAS check for LEON instructions on SPARC.
Index: gcc/testsuite/ChangeLog
===================================================================
--- gcc/testsuite/ChangeLog	(revision 209852)
+++ gcc/testsuite/ChangeLog	(working copy)
@@ -1,3 +1,16 @@
+2014-04-28  Dimitris Papavasiliou  <dpapavas@gmail.com>
+
+	* objc.dg/shadow-1.m: New test.
+	* objc.dg/shadow-2.m: New test.
+	* objc.dg/ivar-scope-1.m: New test.
+	* objc.dg/ivar-scope-2.m: New test.
+	* objc.dg/ivar-scope-3.m: New test.
+	* objc.dg/ivar-scope-4.m: New test.
+	* objc.dg/ivar-visibility-1.m: New test.
+	* objc.dg/ivar-visibility-2.m: New test.
+	* objc.dg/ivar-visibility-3.m: New test.
+	* objc.dg/ivar-visibility-4.m: New test.
+
 2014-03-27  Thomas Koenig  <tkoenig@gcc.gnu.org>
 
 	PR fortran/59604
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 <dpapavas@gmail.com>.  */
+/* { dg-do compile } */
+/* { dg-additional-options "-fivar-visibility=protected" } */
+#include <objc/objc.h>
+
+@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-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,24 @@
+/* Test instance variable scope.  */
+/* Author: Dimitris Papavasiliou <dpapavas@gmail.com>.  */
+/* { dg-do compile } */
+#include <objc/objc.h>
+
+@interface MyClass
+{
+  int someivar;
+}
+- (void) test;
+@end
+
+@implementation MyClass
+- (void) test
+{
+  int a;
+
+  /* Make sure instance variables do have local scope when
+     -fno-local-ivar isn't specified. */
+  
+  a = self->someivar;  /* No warning or error. */
+  a = someivar;        /* No error. */
+}
+@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 <dpapavas@gmail.com>.  */
+/* { dg-do compile } */
+/* { dg-additional-options "-fivar-visibility=private" } */
+#include <objc/objc.h>
+
+@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-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 <dpapavas@gmail.com>.  */
+/* { dg-do compile } */
+/* { dg-additional-options "-fno-local-ivars" } */
+#include <objc/objc.h>
+
+@interface MyClass
+{
+  int someivar;
+}
+- (void) testscope;
+- (void) testshadowing;
+@end
+
+@implementation MyClass
+- (void) testscope
+{
+  int a;
+
+  a = self->someivar;  /* No warning or error. */
+  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; /* No warning. */
+}
+@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,36 @@
+/* Test instance variable visibility.  */
+/* Author: Dimitris Papavasiliou <dpapavas@gmail.com>.  */
+/* { dg-do compile } */
+/* { dg-additional-options "-fivar-visibility=public" } */
+#include <objc/objc.h>
+
+@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;
+
+  /* someivar is public so we shoudn't get any errors here. */
+  
+  a = object->someivar;
+}
+@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 <dpapavas@gmail.com>.  */
+/* { dg-do run } */
+/* { dg-additional-options "-Wno-shadow-ivar" } */
+#include "../objc-obj-c++-shared/TestsuiteObject.m"
+#include <objc/objc.h>
+
+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,83 @@
+/* Test instance variable scope.  */
+/* Author: Dimitris Papavasiliou <dpapavas@gmail.com>.  */
+/* { dg-do run } */
+/* { dg-additional-options "-Wno-shadow-ivar -fno-local-ivars" } */
+#include "../objc-obj-c++-shared/runtime.h"
+#include <objc/objc.h>
+
+extern void abort(void);
+
+int someivar = 1;
+
+/* The testsuite object depends on local variable scope so we need to
+   implement our own minimal base object here. */
+
+@interface MyClass
+{
+  Class isa;
+  int someivar;
+}
+
++ (id) alloc;
+- (id) init;
+- (int) getGlobal;
+- (int) getInstance;
+- (int) getHidden;
+@end
+
+@implementation MyClass
++ (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 alloc] init];
+
+  /* Check for aliasing between instance variable and global
+     variable. */
+
+  if ([object getGlobal] != 1) {
+    abort();
+  }
+  
+  if ([object getInstance] != 2) {
+    abort();
+  }
+
+  /* Check whether the local variable hides the instance variable. */
+  
+  if ([object getHidden] != 3) {
+    abort();
+  }
+
+  return 0;
+}
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 <dpapavas@gmail.com>.  */
+/* { dg-do compile } */
+/* { dg-additional-options "-Wno-shadow-ivar" } */
+#include <objc/objc.h>
+
+@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;    /* No warning. */
+  a = protected;  /* No warning. */
+  a = public;     /* No warning. */
+}
+@end
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 <dpapavas@gmail.com>.  */
+/* { dg-do compile } */
+#include <objc/objc.h>
+
+@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/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 <dpapavas@gmail.com>.  */
+/* { dg-do compile } */
+/* { dg-additional-options "-Wno-shadow" } */
+#include <objc/objc.h>
+
+@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;    /* No warning. */
+  a = protected;  /* No warning. */
+  a = public;     /* No warning. */
+}
+@end

Re: [PATCH] Add a couple of dialect and warning options regarding Objective-C instance variable scope

[Mike Stump]

On Apr 25, 2014, at 9:34 AM, Dimitris Papavasiliou <dpapavas@gmail.com> wrote:

> --Wreturn-type  -Wsequence-point  -Wshadow @gol
> +-Wreturn-type  -Wsequence-point  -Wshadow  -Wshadow-ivar @gol

This has to be -Wno-shadow-ivar, we document the non-default…

> +@item -Wshadow-ivar @r{(Objective-C only)}

Likewise.

> +  /* Check wheter the local variable hides the instance variable. */

spelling, whether...

> +  a = private;    /* { dg-warning "hides instance variable" "" { xfail *-*-* } } */
> +  a = protected;  /* { dg-warning "hides instance variable" "" { xfail *-*-* } } */
> +  a = public;     /* { dg-warning "hides instance variable" "" { xfail *-*-* } } */

No, we don’t expect failures.  We makes the compiler do what we wants or it gets the hose again.  Then, we expect it to be perfect.  If you won’t want warning, and non are produces, then just remove the /* … */, or write /* no warning */.

Also, synth up the ChnageLogs… :-), they are trivial enough.

And, just pop them all into one patch (cd ..; svn diff), 3 is 3x the work for me.

Once we resolve the 3 warning tests above, this will be ok.

Re: [PATCH] Add a couple of dialect and warning options regarding Objective-C instance variable scope

[Dimitris Papavasiliou]

[-- Attachment #1: Type: text/plain, Size: 1888 bytes --]

On 04/25/2014 04:07 AM, Mike Stump wrote:
> On Apr 24, 2014, at 4:09 PM, Dimitris Papavasiliou<dpapavas@gmail.com>  wrote:
>> On 04/24/2014 07:00 PM, Mike Stump wrote:
>>> On Feb 6, 2014, at 1:25 AM, Dimitris Papavasiliou<dpapavas@gmail.com>  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

[-- Attachment #2: documentation.patch --]
[-- Type: text/x-patch, Size: 2872 bytes --]

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}

[-- Attachment #3: enabledby_wshadow.patch --]
[-- Type: text/x-patch, Size: 1417 bytes --]

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 <dpapavas@gmail.com>.  */
+/* { dg-do compile } */
+/* { dg-additional-options "-Wno-shadow" } */
+#include <objc/objc.h>
+
+@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

[-- Attachment #4: tests.patch --]
[-- Type: text/x-patch, Size: 8817 bytes --]

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 <dpapavas@gmail.com>.  */
+/* { dg-do compile } */
+/* { dg-additional-options "-Wno-shadow-ivar" } */
+#include <objc/objc.h>
+
+@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 <dpapavas@gmail.com>.  */
+/* { dg-do compile } */
+#include <objc/objc.h>
+
+@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 <dpapavas@gmail.com>.  */
+/* { dg-do compile } */
+/* { dg-additional-options "-fno-local-ivars" } */
+#include <objc/objc.h>
+
+@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 <dpapavas@gmail.com>.  */
+/* { dg-do run } */
+/* { dg-additional-options "-Wno-shadow-ivar" } */
+#include "../objc-obj-c++-shared/TestsuiteObject.m"
+#include <objc/objc.h>
+
+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 <dpapavas@gmail.com>.  */
+/* { dg-do run } */
+/* { dg-additional-options "-Wno-shadow-ivar -fno-local-ivars" } */
+#include "../objc-obj-c++-shared/runtime.h"
+#include <objc/objc.h>
+
+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 <dpapavas@gmail.com>.  */
+/* { dg-do compile } */
+#include <objc/objc.h>
+
+@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 <dpapavas@gmail.com>.  */
+/* { dg-do compile } */
+/* { dg-additional-options "-fivar-visibility=protected" } */
+#include <objc/objc.h>
+
+@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 <dpapavas@gmail.com>.  */
+/* { dg-do compile } */
+/* { dg-additional-options "-fivar-visibility=private" } */
+#include <objc/objc.h>
+
+@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 <dpapavas@gmail.com>.  */
+/* { dg-do compile } */
+/* { dg-additional-options "-fivar-visibility=public" } */
+#include <objc/objc.h>
+
+@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

Re: [PATCH] Add a couple of dialect and warning options regarding Objective-C instance variable scope

[Mike Stump]

On Apr 24, 2014, at 4:09 PM, Dimitris Papavasiliou <dpapavas@gmail.com> wrote:
> On 04/24/2014 07:00 PM, Mike Stump wrote:
>> On Feb 6, 2014, at 1:25 AM, Dimitris Papavasiliou <dpapavas@gmail.com> 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.

Re: [PATCH] Add a couple of dialect and warning options regarding Objective-C instance variable scope

[Dimitris Papavasiliou]

On 04/24/2014 07:00 PM, Mike Stump wrote:
> On Feb 6, 2014, at 1:25 AM, Dimitris Papavasiliou <dpapavas@gmail.com> 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.  This patch was made 
quite a few months ago but since your were able to apply it without 
problems I suppose it was still up-to-date.  I took a look at the 
applied changes and everything seems to be ok.

I'll add documentation and test cases as soon as I figure out how.

Dimitris

Re: [PATCH] Add a couple of dialect and warning options regarding Objective-C instance variable scope

[Mike Stump]

On Feb 6, 2014, at 1:25 AM, Dimitris Papavasiliou <dpapavas@gmail.com> 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.

[PATCH] Add a couple of dialect and warning options regarding Objective-C instance variable scope

[Dimitris Papavasiliou]

[-- Attachment #1: Type: text/plain, Size: 3447 bytes --]

Hello,

This is a patch regarding a couple of Objective-C related dialect 
options and warning switches.  I have already submitted it a while ago 
but gave up after pinging a couple of times.  I am now informed that 
should have kept pinging until I got someone's attention so I'm 
resending it.

The patch is now against an old revision and as I stated originally it's 
probably not in a state that can be adopted as is.  I'm sending it as is 
so that the implemented features can be assesed in terms of their 
usefulness and if they're welcome I'd be happy to make any necessary 
changes to bring it up-to-date, split it into smaller patches, add 
test-cases and anything else that is deemed necessary.

Here's the relevant text from my initial message:

Two of these switches are related to a feature request I submitted a 
while ago, Bug 56044 
(http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56044).  I won't reproduce 
the entire argument here since it is available in the feature request. 
The relevant functionality in the patch comes in the form of two switches:

-Wshadow-ivars which controls the "local declaration of ‘somevar’ hides 
instance variable" warning which curiously is enabled by default instead 
of being controlled at least by -Wshadow.  The patch changes it so that 
this warning can be enabled and disabled specifically through 
-Wshadow-ivars as well as with all other shadowing-related warnings 
through -Wshadow.

The reason for the extra switch is that, while searching through the 
Internet for a solution to this problem I have found out that other 
people are inconvenienced by this particular warning as well so it might 
be useful to be able to turn it off while keeping all the other 
shadowing-related warnings enabled.

-flocal-ivars which when true, as it is by default, treats instance 
variables as having local scope.  If false (-fno-local-ivars) instance 
variables must always be referred to as self->ivarname and references of 
ivarname resolve to the local or global scope as usual.

I've also taken the opportunity of adding another switch unrelated to 
the above but related to instance variables:

-fivar-visibility which can be set to either private, protected (the 
default), public and package.  This sets the default instance variable 
visibility which normally is implicitly protected.  My use-case for it 
is basically to be able to set it to public and thus effectively disable 
this visibility mechanism altogether which I find no use for and 
therefore have to circumvent.  I'm not sure if anyone else feels the 
same way towards this but I figured it was worth a try.

I'm attaching a preliminary patch against the current revision in case 
anyone wants to have a look.  The changes are very small and any blatant 
mistakes should be immediately obvious.  I have to admit to having 
virtually no knowledge of the internals of GCC but I have tried to keep 
in line with formatting guidelines and general style as well as looking 
up the particulars of the way options are handled in the available 
documentation to avoid blind copy-pasting.  I have also tried to test 
the functionality both in my own (relatively large, or at least not too 
small) project and with small test programs and everything works as 
expected.  Finallly, I tried running the tests too but these fail to 
complete both in the patched and unpatched version, possibly due to the 
way I've configured GCC.

Dimitris

[-- Attachment #2: objc_ivar_scope.diff --]
[-- Type: text/x-patch, Size: 4828 bytes --]

Index: gcc/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt	(revision 199867)
+++ gcc/c-family/c.opt	(working copy)
@@ -661,6 +661,10 @@ Wselector
 ObjC ObjC++ Var(warn_selector) Warning
 Warn if a selector has multiple methods
 
+Wshadow-ivar
+ObjC ObjC++ Var(warn_shadow_ivar) Init(-1) Warning
+Warn if a local declaration hides an instance variable
+
 Wsequence-point
 C ObjC C++ ObjC++ Var(warn_sequence_point) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
 Warn about possible violations of sequence point rules
@@ -1018,6 +1022,29 @@ fnil-receivers
 ObjC ObjC++ Var(flag_nil_receivers) Init(1)
 Assume that receivers of Objective-C messages may be nil
 
+flocal-ivars
+ObjC ObjC++ Var(flag_local_ivars) Init(1)
+Allow access to instance variables as if they were local declarations within instance method implementations.
+
+fivar-visibility=
+ObjC ObjC++ Joined RejectNegative Enum(ivar_visibility) Var(default_ivar_visibility) Init(IVAR_VISIBILITY_PROTECTED)
+-fvisibility=[private|protected|public|package]	Set the default symbol visibility
+
+Enum
+Name(ivar_visibility) Type(enum ivar_visibility) UnknownError(unrecognized ivar visibility value %qs)
+
+EnumValue
+Enum(ivar_visibility) String(private) Value(IVAR_VISIBILITY_PRIVATE)
+
+EnumValue
+Enum(ivar_visibility) String(protected) Value(IVAR_VISIBILITY_PROTECTED)
+
+EnumValue
+Enum(ivar_visibility) String(public) Value(IVAR_VISIBILITY_PUBLIC)
+
+EnumValue
+Enum(ivar_visibility) String(package) Value(IVAR_VISIBILITY_PACKAGE)
+
 fnonansi-builtins
 C++ ObjC++ Var(flag_no_nonansi_builtin, 0)
 
Index: gcc/flag-types.h
===================================================================
--- gcc/flag-types.h	(revision 199867)
+++ gcc/flag-types.h	(working copy)
@@ -104,6 +104,16 @@ enum symbol_visibility
 };
 #endif
 
+/* Enumerate Objective-c instance variable visibility settings. */
+
+enum ivar_visibility
+{
+  IVAR_VISIBILITY_PRIVATE,
+  IVAR_VISIBILITY_PROTECTED,
+  IVAR_VISIBILITY_PUBLIC,
+  IVAR_VISIBILITY_PACKAGE
+};
+
 /* The stack reuse level.  */
 enum stack_reuse_level
 {
Index: gcc/objc/objc-act.c
===================================================================
--- gcc/objc/objc-act.c	(revision 199867)
+++ gcc/objc/objc-act.c	(working copy)
@@ -223,7 +223,7 @@ struct imp_entry *imp_list = 0;
 int imp_count = 0;	/* `@implementation' */
 int cat_count = 0;	/* `@category' */
 
-objc_ivar_visibility_kind objc_ivar_visibility;
+objc_ivar_visibility_kind objc_ivar_visibility, objc_default_ivar_visibility;
 
 /* Use to generate method labels.  */
 static int method_slot = 0;
@@ -391,6 +391,22 @@ objc_init (void)
   if (!ok)
     return false;
 
+  /* Determine the default visibility for instance variables. */
+  switch (default_ivar_visibility)
+    {
+    case IVAR_VISIBILITY_PRIVATE:
+        objc_default_ivar_visibility = OBJC_IVAR_VIS_PRIVATE;
+        break;
+    case IVAR_VISIBILITY_PUBLIC:
+        objc_default_ivar_visibility = OBJC_IVAR_VIS_PUBLIC;
+        break;
+    case IVAR_VISIBILITY_PACKAGE:
+        objc_default_ivar_visibility = OBJC_IVAR_VIS_PACKAGE;
+        break;
+    default:
+        objc_default_ivar_visibility = OBJC_IVAR_VIS_PROTECTED;
+    }
+      
   /* Generate general types and push runtime-specific decls to file scope.  */
   synth_module_prologue ();
 
@@ -565,7 +581,7 @@ objc_start_class_interface (tree klass,
   objc_interface_context
     = objc_ivar_context
     = start_class (CLASS_INTERFACE_TYPE, klass, super_class, protos, attributes);
-  objc_ivar_visibility = OBJC_IVAR_VIS_PROTECTED;
+  objc_ivar_visibility = objc_default_ivar_visibility;
 }
 
 void
@@ -643,7 +659,7 @@ objc_start_class_implementation (tree kl
     = objc_ivar_context
     = start_class (CLASS_IMPLEMENTATION_TYPE, klass, super_class, NULL_TREE,
 		   NULL_TREE);
-  objc_ivar_visibility = OBJC_IVAR_VIS_PROTECTED;
+  objc_ivar_visibility = objc_default_ivar_visibility;
 }
 
 void
@@ -9360,6 +9376,12 @@ objc_lookup_ivar (tree other, tree id)
       && other && other != error_mark_node)
     return other;
 
+  /* Don't look up the ivar if the user has explicitly advised against
+   * it with -fno-local-ivars. */
+
+  if (!flag_local_ivars)
+    return other;
+
   /* Look up the ivar, but do not use it if it is not accessible.  */
   ivar = is_ivar (objc_ivar_chain, id);
 
@@ -9376,8 +9398,11 @@ objc_lookup_ivar (tree other, tree id)
       && !DECL_FILE_SCOPE_P (other))
 #endif
     {
-      warning (0, "local declaration of %qE hides instance variable", id);
-
+      if (warn_shadow_ivar == 1 || (warn_shadow && warn_shadow_ivar != 0)) {
+          warning (warn_shadow_ivar ? OPT_Wshadow_ivar : OPT_Wshadow,
+                   "local declaration of %qE hides instance variable", id);
+      }
+        
       return other;
     }