[cfe-dev] [cfe-commits] r100942 - /cfe/trunk/lib/CodeGen/CGObjC.cpp

[Chris Lattner]

On Apr 11, 2010, at 5:25 AM, David Chisnall wrote:

> On 11 Apr 2010, at 03:33, Chris Lattner wrote:
> 
>> Uh, we've compiled a *lot* of objective-c code, and Apple has been shipping clang as a production quality c/objc compiler since last summer.
> 
> Perhaps this is not an idiom used inside Apple.  XCode does not default to clang, and most projects that I have tried compiling only worked without issues in the last few months.

Arguing about whether people are using clang or not is a moot point.  If they aren't already, they will at some point.  We intentionally have decided to reject some constructs that GCC supports (for a number of reasons, forcing people to clean up their code is one).  Doing this when transitioning from GCC -> Clang is a much easier thing for developers to understand than to find out they upgraded a minor version of their compiler and suddenly their code doesn't build.

>  ABI support has been a big issue for non-Darwin users, with various random seeming crashes that have only been addressed (and reported - no implied criticism here) recently.

I'm not sure what you mean.

>> There are lots of ways, remove the parens from the macro or don't use a macro at all.  Amazing.
> 
> Neither of these are solutions.  Removing the parentheses from the macro is not an option, because they are required when something that is an expression is used.  Don't use the macro at all is not really a solution either - macros of this form are already in use and suggesting that someone goes through their existing code and manually expands every instance of the macro (including wrapping it in #ifdefs) is more of a problem than a solution.
> 
> Macros of this form are included with the standard GNUstep headers, and have been for the past 15 years.  Our next release will be the first one to officially support clang as a compiler, although the fact that you broke the address of label extension just before 1.1 means that we have to require clang from svn to build our Foundation implementation.  
> 
> This is the first time that we are encouraging downstream developers to use clang, and you are intentionally breaking the compilation of their code, if they are using these macros.  

Why not include a second version of the macro that works with super?  Alternatively, why not just drop the parens from the macro definition?

I'm not sure how address of label is broken, PR#?

>>> Simply declaring code as broken when it has always worked in GCC and solves a specific problem is not a constructive approach.
>> 
>> David, I respect you a lot, but I feel like I've given you several explanations of why this is wrong.  I can't help that you don't want to hear it :)
> 
> I agree that treating super as an expression is wrong in the general case, but I disagree that it is wrong for super to be allowed to be in brackets.  Putting something in brackets does not change its semantics, is an idiom used in the wild, and does not require it to be treated as an expression.

I agree that putting an expression in parentheses does not change it semantics, but super isn't an expression.  You can't do:

x (+) y

for example.  You also can't do:

 for (id x (in) ...

etc.  Why should you be able to parenthesize super?  It doesn't make any sense, and arguing about language semantics from a convenience standpoint doesn't fly well, there are lots of things that don't make sense that would be convenient.

>> Have you looked at all the places and weird ad-hoc things that clang is doing w.r.t. super?  It is a real mess right now, it isn't a keyword, it is special cased in a few bizarre places.  I want to make it so the parser *just* handles this in the message dispatch parser as a context sensitive keyword.  This is the semantic behavior of it, it should not be parsed as a general expression because it doesn't make sense.
> 
> With regard to your latest change, I really dislike this warning:
> 
> 'super' not valid when not in a method
> 
> This is misleading, because 'super' is valid when not used in a method, as long as it is declared as a variable.

Sure, this is the typical problem with diagnostics: you have to guess what the user meant and/or what the most useful message is to help people fix their code.  I inferred that a likely reason to get this message is refactoring code from a method out to a C function.  ObjC programers consider super to be a magic incantation, not something they're likely to name variables (despite it not actually being a keyword).

I don't think it would be particularly useful to emit "'super' not valid when not in a method, but you could name a variable 'super' if you really wanted to".

> I often use super as a variable name referring to a superclass in Objective-C code outside of methods, especially in free-standing methods that will be attached to a class at run time.  And please don't tell me that this is a bug - Objective-C is a pure superset of C and, by design, does not declare any keywords that are valid C identifiers.

I agree, that is not a bug.

>  You also seem to have broken some valid code:
> 
> $ cat sup.m && clang -c sup.m
> #import <objc/Object.h>
> 
> int main(void)
> {
> 	id super = nil;
> 	[(Object*)super new];
> 	int *s1 = (int*)super;
> 	return 0;
> }
> sup.m:6:12: error: cannot cast 'super' (it isn't an expression)
>        [(Object*)super new];
>         ~~~~~~~~~^
> sup.m:7:18: error: cannot cast 'super' (it isn't an expression)
>        int *s1 = (int*)super;
>                  ~~~~~~^
> 2 errors generated.
> 
> super is an expression here.  It is a local variable.  This is entirely valid, especially in the second case where there is no Objective-C at all.  Pure C code should always compile correctly in Objective-C mode.  

I'll take a look thanks!

-Chris