[PATCH] Implement the __builtin_call_with_static_chain GNU extension.

[Peter Collingbourne]

================
Comment at: lib/Sema/SemaChecking.cpp:187
@@ +186,3 @@
+  BuiltinCall->setType(CE->getType());
+  BuiltinCall->getCallee()->setType(CE->getCallee()->getType());
+  BuiltinCall->setValueKind(CE->getValueKind());
----------------
rsmith wrote:
> pcc wrote:
> > rsmith wrote:
> > > This can't be right: multiple calls to the builtin will share the same callee and will want different types here. Just remove this line?
> > I found this to be necessary in cases where the function returns an lvalue; without it, I was getting assertion failures.
> > 
> > I thought this might be an issue as well, until I looked at the AST representation of builtin calls:
> > 
> > ```
> > |     |-CallExpr 0x4310ae0 <col:3, col:42> 'int' lvalue
> > |     | |-ImplicitCastExpr 0x4310ac8 <col:3> 'int &(*)(void)' <BuiltinFnToFnPtr>
> > |     | | `-DeclRefExpr 0x4310960 <col:3> '<builtin fn type>' Function 0x43108c0 '__builtin_call_with_static_chain' 'void ()'
> > |     | |-CallExpr 0x4310a50 <col:36, col:38> 'int' lvalue
> > |     | | `-ImplicitCastExpr 0x4310a38 <col:36> 'int &(*)(void)' <FunctionToPointerDecay>
> > |     | |   `-DeclRefExpr 0x43109e0 <col:36> 'int &(void)' lvalue Function 0x42ca430 'f' 'int &(void)'
> > |     | `-ImplicitCastExpr 0x4310b18 <col:41> 'int &(*)(void)' <FunctionToPointerDecay>
> > |     |   `-DeclRefExpr 0x4310a78 <col:41> 'int &(void)' lvalue Function 0x42ca430 'f' 'int &(void)'
> > ```
> > 
> > So we are only overwriting the type of the `BuiltinFnToFnPtr` cast. This seems to be not too bad (except that the argument types are wrong; I should fix that, but it didn't seem to cause any issues).
> That's still not correct. Practically, we could share the same `ImplicitCastExpr` between multiple calls to the builtin (through template instantiation, for instance), and theoretically, it's wrong to have a `BuiltinFnToFnPtr` cast that performs some conversion other than function-to-pointer decay. One reasonable thing to do here would be to create a new `ImplicitCastExpr` node to represent the conversion between function pointer types. (Another would be to teach whatever code was asserting that it shouldn't assume the call return type matches that of the callee for a call to a builtin -- I guess this is probably the ExprClassification.cpp code?)
> Practically, we could share the same ImplicitCastExpr between multiple calls to the builtin (through template instantiation, for instance)

Fair point. I couldn't get this to trigger during template instantiation, but I suppose it could happen in other cases or if we change how template instantiation works.

> theoretically, it's wrong to have a BuiltinFnToFnPtr cast that performs some conversion other than function-to-pointer decay.

I don't understand why. We get to decide what the semantics of the various implicit casts are, and it seems reasonable to me for `BuiltinFnToFnPtr` to also represent the conversion of a generic builtin to a specific type. I also couldn't find any existing code that depends on `BuiltinFnToFnPtr` being a strict function-to-pointer decay conversion.

> One reasonable thing to do here would be to create a new ImplicitCastExpr node to represent the conversion between function pointer types.

Agreed that the code should create a new `ImplicitCastExpr` instead of trying to change the existing one, but I think you also meant that we should introduce a new cast kind for this case, which I don't think is necessary.

> Another would be to teach whatever code was asserting that it shouldn't assume the call return type matches that of the callee for a call to a builtin

I'd feel more comfortable representing this with a cast, as I'd rather not carve out an exception to a reasonable AST invariant.

> I guess this is probably the ExprClassification.cpp code?

Yes.

http://reviews.llvm.org/D6332

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/