public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Add a couple of dialect and warning options regarding Objective-C instance variable scope
@ 2013-06-09 21:44 Dimitris Papavasiliou
  2013-06-24 12:09 ` Dimitris Papavasiliou
  0 siblings, 1 reply; 17+ messages in thread
From: Dimitris Papavasiliou @ 2013-06-09 21:44 UTC (permalink / raw)
  To: gcc-patches

[-- 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;
     }
 

^ permalink raw reply	[flat|nested] 17+ messages in thread
* [PATCH] Add a couple of dialect and warning options regarding Objective-C instance variable scope
@ 2014-02-06  9:22 Dimitris Papavasiliou
  2014-04-24 16:03 ` Mike Stump
  0 siblings, 1 reply; 17+ messages in thread
From: Dimitris Papavasiliou @ 2014-02-06  9:22 UTC (permalink / raw)
  To: gcc-patches; +Cc: mikestump, stanshebs

[-- 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;
     }
 

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2014-05-13 14:17 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-09 21:44 [PATCH] Add a couple of dialect and warning options regarding Objective-C instance variable scope Dimitris Papavasiliou
2013-06-24 12:09 ` Dimitris Papavasiliou
2013-07-02  8:05   ` Dimitris Papavasiliou
2014-02-06  9:22 Dimitris Papavasiliou
2014-04-24 16:03 ` Mike Stump
2014-04-24 23:16   ` Dimitris Papavasiliou
2014-04-25  3:20     ` Mike Stump
2014-04-25 16:50       ` Dimitris Papavasiliou
2014-04-25 16:53         ` Mike Stump
2014-04-28 10:37           ` Dimitris Papavasiliou
2014-05-05  7:31             ` Dimitris Papavasiliou
2014-05-12  7:49               ` Dimitris Papavasiliou
2014-05-12 16:25             ` Mike Stump
2014-05-12 18:57               ` Dimitris Papavasiliou
2014-05-12 19:14                 ` Mike Stump
2014-05-12 20:53             ` Mike Stump
2014-05-13 14:17               ` Dimitris Papavasiliou

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).