On 3 Feb 2010, at 23:29, David Chisnall wrote:
> Hi Daniel,
>
> How does this break the NeXT runtime? I ran the test suite and got no
> failures, and I tested a few extra @selector() cases with -fnext-runtime and
> got code that looked correct. If you can provide me with a test case that
> demonstrates how this breaks the NeXT runtime then I'd be happy to fix it.
>
> The code generation code paths for the NeXT runtime should be exactly the
> same before and after the patch and the only change in Sema produces the
> warning conditionally, suppressing it when the NeXT runtime is used.
Specifically, this is the only code modified in the Mac runtime:
> + virtual llvm::Constant *GetConstantSelector(Selector Sel) {
> + assert(0 && "Constant Selectors are not yet supported on the Mac
> runtimes");
> + return 0;
> + }
> + virtual llvm::Constant *GetConstantTypedSelector(
> + const ObjCMethodDecl *Method) {
> + return GetConstantSelector(Method->getSelector());
> + }
>
This is only called in one place, in CGExprConstant:
> + llvm::Constant *VisitObjCSelectorExpr(const ObjCSelectorExpr *E) {
> + ObjCMethodDecl *OMD = E->getMethodDecl();
> + if (OMD)
> + return CGM.getObjCRuntime().GetConstantTypedSelector(OMD);
> + else
> + return CGM.getObjCRuntime().GetConstantSelector(E->getSelector());
> + }
> +
This does change the behaviour on the Mac runtime. The change in this file
cause it fails with an assert that CGObjCMac doesn't yet support this behaviour
instead of 'can not codegen this constant expression'. This is now reached
because of this change in AST/Expr.cpp:
> + case ObjCSelectorExprClass:
> return true;
Which was incorrect before the change; @selector() expressions are constants
and GCC treats them as such. The following will compile quite happily in GCC:
static SEL foo = @selector(foo);
The only other change that touched Mac runtime code paths was this:
> + ObjCMethodDecl *OMD = E->getMethodDecl();
> + if (OMD)
> + return CGM.getObjCRuntime().GetSelector(Builder, OMD);
> + else
> + return CGM.getObjCRuntime().GetSelector(Builder, E->getSelector());
This calls a method that was already present in the GCObjCMac (calling
EmitSelector() with OMD->getSelector() as the argument, so this will generate
exactly the same code for both code paths on the Mac runtime).
The only other change was to add a ObjCMethodDecl to the ObjCSelectorExpr.
This change has no semantic effect on the Mac runtime - indeed, the test for
the Mac runtime here was added specifically because an earlier version of the
patch did fail a test because a warning that the Mac runtime didn't expect was
generated.
So, I am at a loss to see where this broke the Mac runtime aside from causing
it to fail in a different place when you try to compile (valid) code which uses
an @selector() expression as a static initialiser. I'm sorry if it broke the
Mac runtime, but I did test it with all of the tests in the current test suite
(and got no unexpected failures) and with a few extra programs using -S
-fnext-runtime -emit-llvm to see if it still generated the same code as before
the patch.
'Substantially breaks working code' is possibly the least useful bug report
that I have ever read. I did everything in my power to ensure that this did
not affect the Mac runtime, but if it did then I would appreciate it if you
would provide me with some hints as to how it broke the Mac runtime, rather
than just asserting that it did.
David
-- Sent from my Apple II
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
