ObjC/ObjC++: extend fix of PR objc/47076 ("Protocol referenced in @interface declarations should be defined")

On Fri, Dec 31, 2010 at 3:15 AM, Nicola Pero
<nicola.pero@meta-innovation.com> wrote:
> This patch is a sophistication of the fix to PR objc/47076, and fixes a
> slightly more complicated case.
>
> In objc/47076, we fixed the detection that a protocol was only
> forward-declared, ie, the testcase
>
> @protocol A;
>
> @interface MyClass <A>
> @end
>
> this testcase should cause a warning, because protocol A is
> forward-declared, ie, it has been declared
> that A is a protocol without providing the list of methods required/optional
> for it; but when A is used in the
> declaration of MyClass, the compiler needs the list of methods to properly
> process MyClass to do all the
> checks and things required.  So, the compiler does (now) correctly warn in
> this case.
>
> What we didn't cover was the complication
>
> @protocol A;
>
> @protocol B <A>
> - (void)method;
> @end
>
> @interface MyClass <B>
> @end
>
> In this case, protocol B, which is directly referenced by MyClass, is a
> normal, declared protocol with
> a list of methods.  But, it is also specified that B conforms to A, which is
> another protocol, this time
> forward-declared.  So, again, the compiler won't have the full list of
> methods required/optional for the
> protocols that MyClass is declared to implement, and won't be able to
> perform all the expected checks
> and stuff on MyClass.  It should, again, warn.  But our fix to objc/47076
> didn't cover this case; it only
> covered protocols directly referenced by the declaration.
>
> This patch extends the fix to these cases by doing the recursive check
> required, so we now warn
> in this case too.  The warning/checks actually become much more useful in
> practice too :-).  Testcases
> included.
>
> Ok to commit ?
>
> Thanks
>
> Index: objc/objc-act.c
> ===================================================================
> --- objc/objc-act.c     (revision 168362)
> +++ objc/objc-act.c     (working copy)
> @@ -10929,11 +10929,30 @@ add_protocol (tree protocol)
>   return protocol_chain;
>  }
>
> +/* Check that a protocol is defined, and, recursively, that all
> +   protocols that this protocol conforms to are defined too.  */
> +static void
> +check_that_protocol_is_defined (tree protocol)
> +{
> +  if (!PROTOCOL_DEFINED (protocol))
> +    warning (0, "definition of protocol %qE not found",
> +            PROTOCOL_NAME (protocol));
> +
> +  /* If the protocol itself conforms to other protocols, check them
> +     too, recursively.  */
> +  if (PROTOCOL_LIST (protocol))
> +    {
> +      tree p;
> +
> +      for (p = PROTOCOL_LIST (p); p; p = TREE_CHAIN (p))
> +       check_that_protocol_is_defined (TREE_VALUE (p));
> +    }
> +}
> +

This caused:

../../src-trunk/gcc/objc/objc-act.c: In function
'check_that_protocol_is_defined':
../../src-trunk/gcc/objc/objc-act.c:10947:16: error: 'p' may be used
uninitialized in this function [-Werror=uninitialized]
/export/gnu/import/svn/gcc-test-intel64/bld/./prev-gcc/xgcc
-B/export/gnu/import/svn/gcc-test-intel64/bld/./prev-gcc/
-B/usr/local/x86_64-unknown-linux-gnu/bin/
-B/usr/local/x86_64-unknown-linux-gnu/bin/
-B/usr/local/x86_64-unknown-linux-gnu/lib/ -isystem
/usr/local/x86_64-unknown-linux-gnu/include -isystem
/usr/local/x86_64-unknown-linux-gnu/sys-include    -c   -g -O2
-gtoggle -DIN_GCC   -W -Wall -Wwrite-strings -Wcast-qual
-Wstrict-prototypes -Wmissing-prototypes -Wmissing-format-attribute
-pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings
-Werror -Wold-style-definition -Wc++-compat -fno-common
-DHAVE_CONFIG_H -I. -I. -I../../src-trunk/gcc -I../../src-trunk/gcc/.
-I../../src-trunk/gcc/../include
-I../../src-trunk/gcc/../libcpp/include
-I../../src-trunk/gcc/../libdecnumber
-I../../src-trunk/gcc/../libdecnumber/bid -I../libdecnumber
insn-output.c -o insn-output.o
/export/gnu/import/svn/gcc-test-intel64/bld/./prev-gcc/xgcc
-B/export/gnu/import/svn/gcc-test-intel64/bld/./prev-gcc/
-B/usr/local/x86_64-unknown-linux-gnu/bin/
-B/usr/local/x86_64-unknown-linux-gnu/bin/
-B/usr/local/x86_64-unknown-linux-gnu/lib/ -isystem
/usr/local/x86_64-unknown-linux-gnu/include -isystem
/usr/local/x86_64-unknown-linux-gnu/sys-include    -c   -g -O2
-gtoggle -DIN_GCC   -W -Wall -Wwrite-strings -Wcast-qual
-Wstrict-prototypes -Wmissing-prototypes -Wmissing-format-attribute
-pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings
-Werror -Wold-style-definition -Wc++-compat -fno-common
-DHAVE_CONFIG_H -I. -I. -I../../src-trunk/gcc -I../../src-trunk/gcc/.
-I../../src-trunk/gcc/../include
-I../../src-trunk/gcc/../libcpp/include
-I../../src-trunk/gcc/../libdecnumber
-I../../src-trunk/gcc/../libdecnumber/bid -I../libdecnumber
insn-recog.c -o insn-recog.o
cc1: all warnings being treated as errors

make[6]: *** [objc/objc-act.o] Error 1

I checked in this as an obvious fix.

On Jan 2, 2011, at 8:28 AM, "H.J. Lu" <hjl.tools@gmail.com> wrote:
> On Fri, Dec 31, 2010 at 3:15 AM, Nicola Pero
> <nicola.pero@meta-innovation.com> wrote:
>> This patch is a sophistication of the fix to PR objc/47076, and fixes a
>> slightly more complicated case.

>> 
> This caused:
> 
> objc-act.c:10947:16: error: 'p' may be used uninitialized

Thanks for the fix.

Nicola, please investigate why you didn't see the build error...  I suspect you need a bootstrap tree with optimization on and ensure you have werror on.  Thanks.

> Nicola, please investigate why you didn't see the build error...  I suspect
> you need a bootstrap tree with optimization on and ensure you have werror
> on.

The default bootstrap settings are sufficient.