building dbuskit fails with llvm/clang

[Sebastian Reitenbach]

Hi,

I extended the list of libs/apps to be compiled and what I want to add
to the gnustep ports a little bit, and found dbuskit from svn does
compile with gcc, but not with llvm/clang.

The first error is, and is easy to fix, see attached patch:
clang DKArgument.m -c \
-MMD -MP -DGNUSTEP -DGNUSTEP_BASE_LIBRARY=1
-DGNU_GUI_LIBRARY=1 -DGNU_RUNTIME=1 -DGNUSTEP_BASE_LIBRARY=1
-fno-strict-aliasing -fPIC -DDEBUG -fno-omit-frame-pointer -Wall
-DGSWARN -DGSDIAGNOSE -Wno-import -O2 -pipe -g -fgnu-runtime
-I/usr/local/include/dbus-1.0 -I/usr/local/lib/dbus-1.0/include -Wall
-Wdeclaration-after-statement -Werror
-fconstant-string-class=NSConstantString -I./derived_src -I.
-I/usr/local/include
-I/gnustep-dbuskit-0.1_writes_to_HOME/GNUstep/Library/Headers
-I/usr/local/include \
-o obj/DBusKit.obj/DKArgument.m.o
DKArgument.m:495:14: error: equality comparison with extraneous
parentheses [-Werror,-Wparentheses]
if ((type == DKDBusTypeForObjCType([sig methodReturnType])))
~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
DKArgument.m:495:14: note: use '=' to turn this equality comparison into
an assignment
if ((type == DKDBusTypeForObjCType([sig methodReturnType])))
^~
=
DKArgument.m:495:14: note: remove extraneous parentheses around the
comparison to silence this warning
if ((type == DKDBusTypeForObjCType([sig methodReturnType])))
~ ^ ~
1 error generated.

the second error then later is:

clang DKEndpointManager.m -c \
-MMD -MP -DGNUSTEP -DGNUSTEP_BASE_LIBRARY=1
-DGNU_GUI_LIBRARY=1 -DGNU_RUNTIME=1 -DGNUSTEP_BASE_LIBRARY=1
-fno-strict-aliasing -fPIC -DDEBUG -fno-omit-frame-pointer -Wall
-DGSWARN -DGSDIAGNOSE -Wno-import -O2 -pipe -g -fgnu-runtime
-I/usr/local/include/dbus-1.0 -I/usr/local/lib/dbus-1.0/include -Wall
-Wdeclaration-after-statement -Werror
-fconstant-string-class=NSConstantString -I./derived_src -I.
-I/usr/local/include
-I/gnustep-dbuskit-0.1_writes_to_HOME/GNUstep/Library/Headers
-I/usr/local/include \
-o obj/DBusKit.obj/DKEndpointManager.m.o
DKEndpointManager.m:187:6: error: receiver 'GSStackTrace' is a forward
class and corresponding @interface may not exist [-Werror]
[[[GSStackTrace alloc] init] release];
^
In file included from DKEndpointManager.m:23:
In file included from ./DKArgument.h:25:
In file included from ./DKIntrospectionNode.h:25:
/usr/local/include/Foundation/NSObject.h:186:1: note: method 'alloc' is
used for the forward class
+ (id) alloc;
^
DKEndpointManager.m:190:6: error: receiver 'DKWatcher' is a forward
class and corresponding @interface may not exist [-Werror]
[[[DKWatcher alloc] init] release];
^
In file included from DKEndpointManager.m:23:
In file included from ./DKArgument.h:25:
In file included from ./DKIntrospectionNode.h:25:
/usr/local/include/Foundation/NSObject.h:186:1: note: method 'alloc' is
used for the forward class
+ (id) alloc;
^
2 errors generated.

both GSStackTrace and DKWatcher are added with @class statement. They
are both defined within a .m file, so not possible to include/import a
.h file, which I'd have guessed would make the error go away.

Sebastian

[David Chisnall]

On 9 Apr 2011, at 10:29, Sebastian Reitenbach wrote:

> DKArgument.m:495:14: error: equality comparison with extraneous
> parentheses [-Werror,-Wparentheses]
> if ((type == DKDBusTypeForObjCType([sig methodReturnType])))
> ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

We've been through this one before. The added complication here is that Niels has picked up my habit of using -Werror to make it harder to ignore warnings, so a small style point is now stopping it building.

> DKEndpointManager.m:187:6: error: receiver 'GSStackTrace' is a forward
> class and corresponding @interface may not exist [-Werror]
> [[[GSStackTrace alloc] init] release];
> ^

Clang is a bit more aggressive about checking for messages that the receiver doesn't advertise understanding than gcc. If you forward-declare a class with @class, then there is no guarantee that it responds to any messages.

I'm not sure why this code is using GSStackTrace, since the useful functionality of this class is exposed in NSException via public interfaces.

> both GSStackTrace and DKWatcher are added with @class statement. They
> are both defined within a .m file, so not possible to include/import a
> .h file, which I'd have guessed would make the error go away.

The simplest solution is to add a private declaration of the class as:

@interface GSStackTrace : NSObject @end

I think the correct solution is to allow something like this:

@class<NSObject> GSStackTrace;

This would forward-declare the class, and also advertise that it conforms to the NSObject protocol.

David

-- Sent from my Difference Engine

[Sebastian Reitenbach]

with this I get the error:
DKEndpointManager.m:51:8: error: expected identifier
@class <NSObject> GSStackTrace;

but with attached patch, picking up your simplest solution above, it works.

But then afterwards I get:
clang DKInterface.m -c \

-MMD -MP -DGNUSTEP -DGNUSTEP_BASE_LIBRARY=1
-DGNU_GUI_LIBRARY=1 -DGNU_RUNTIME=1 -DGNUSTEP_BASE_LIBRARY=1
-fno-strict-aliasing -fPIC -DDEBUG -fno-omit-frame-pointer -Wall
-DGSWARN -DGSDIAGNOSE -Wno-import -O2 -pipe -g -fgnu-runtime
-I/usr/local/include/dbus-1.0 -I/usr/local/lib/dbus-1.0/include -Wall
-Wdeclaration-after-statement -Werror
-fconstant-string-class=NSConstantString -I./derived_src -I.
-I/usr/local/include
-I/gnustep-dbuskit-0.1_writes_to_HOME/GNUstep/Library/Headers
-I/usr/local/include \

-o obj/DBusKit.obj/DKInterface.m.o
DKInterface.m:153:65: error: if statement has empty body
[-Werror,-Wempty-body]
&& (NO == [method isKindOfClass: [DKPropertyMethod class]]));
^
1 error generated.

the other patch attached fixes this.

Sebastian

[David Chisnall]

On 9 Apr 2011, at 11:01, Sebastian Reitenbach wrote:

>> I think the correct solution is to allow something like this:
>>
>> @class<NSObject> GSStackTrace;
> with this I get the error:
> DKEndpointManager.m:51:8: error: expected identifier
> @class <NSObject> GSStackTrace;

Sorry, I meant that the correct solution is to modify the language to make the above allowable - it's not valid Objective-C at present (although it may be soon...).

> && (NO == [method isKindOfClass: [DKPropertyMethod class]]));
> ^
> 1 error generated.

Semicolon on the end of this line is probably not intended.

[Nicola Pero]

> I think the correct solution is to allow something like this:
>
> @class<NSObject> GSStackTrace;
>

> This would forward-declare the class, and also advertise that
> it conforms to the NSObject protocol.

It's an interesting suggestion, and has good merit. :-)

I would personally not like it that much because:

* well, I like a simple language and I'm worried about feature-creep. :-)

* @class only exists to allow you to deal with circular references
when declaring classes. So, really, every @class should later
be followed by a corresponding @interface.

* I would agree that compilers very lax behaviour in that respect (where
'@class A' makes anything of class A practically equivalent to 'id') is
counter-productive as it doesn't enforce or recommend that at all.
If you message an object of a class which is only declared using @class,
you should get a warning. Both GCC and clang don't warn about this.
GCC also allows class methods to the class again with no warnings;
clang doesn't (in fact it produces a hard error, which is the "problem"
reported above). I like the clang behaviour a lot, and I submitted a bug
report to GCC requesting the warning (maybe not an error) for GCC too.
I'm not entirely sure why clang makes a difference between class and
instance methods. I would like the warning on instance methods too! :-)

Having said that, the idea is creative and nice.

Thanks

[David Chisnall]

On 10 Apr 2011, at 12:30, Nicola Pero wrote:

> If you message an object of a class which is only declared using @class,
> you should get a warning. Both GCC and clang don't warn about this.

I think clang does. The problem is that, in this case, it never sees that the instances are of forward-declared classes. They are created with alloc / init, so they are treated as id. This is a separate issue, and there was a discussion on the clang list a while ago about adding a pair of __attribute__s, one indicating that the returned object is of the same type as the receiver, and another indicating that it's an instance of the receiver, so that +alloc and -init (for example) can be annotated like this and the compiler can correctly type check the returned objects.

David

-- Sent from my Cray X1

[Nicola Pero]

>> If you message an object of a class which is only declared using @class,
>> you should get a warning. Both GCC and clang don't warn about this.
>
> I think clang does.

On my machine, all compilers (both GCC and clang from trunk, as far as I can see)
compile the following testcase with no warnings --

#include <objc/objc.h>

@class A;

@interface B
{
id isa;
}
- (void) doSomething;
@end

void test (A *x)
{
[x doSomething];
}

Now, this is what all Objective-C compilers since GCC 2.95 have done, but is it
a good behaviour ? Shouldn't we get a warning at the '[x doSomething]' line ? :-)

I think the correct behaviour should be to warn on that line, as the compiler
does not have enough information to decide if the method invocation makes sense
or not. If the programmer really knows it makes sense (and for whatever mysterious
reason can't add a proper @interface A), they can always use "[(id)x doSomething]"
to turn the warnings off. ;-)

Thanks

[Niels Grewe]

Hello Sebastian!

Thanks for catching these errors, they should now be resolved in svn.

On Sat, Apr 09, 2011 at 12:01:38PM +0200, Sebastian Reitenbach wrote:
> On 04/09/11 11:39, David Chisnall wrote:
> > I'm not sure why this code is using GSStackTrace, since the useful functionality of this class is exposed in NSException via public interfaces.

This is part of an rather ugly hack to work around the fact that we
don't have per-class +initialize locks in the runtime. The subsequent
code sets up the worker thread for D-Bus interaction and I needed to
make sure that all classes needed by that thread are initialized before
it's being started.

> DKInterface.m:153:65: error: if statement has empty body
> [-Werror,-Wempty-body]

> && (NO == [method isKindOfClass: [DKPropertyMethod class]]));
> ^
> 1 error generated.

This was a rather stupid but still actual bug and catching stuff like
this is one of the reaons for building with -Werror. I'm turning that
flag of in release versions, though.

But thanks again for the patches!

Cheers,

Niels