rsmith added inline comments.
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2828-2829
+ "%0 attribute requires that the return value, all parameters, and any "
+ "temporaries created by the expression are trivially destructible and "
+ "do not require ARC">;
+def err_musttail_needs_call : Error<
----------------
Can we somehow avoid talking about ARC where it's not relevant? While it'd be
nice to be more precise here, my main concern is that we shouldn't be
mentioning ARC to people for whom it's not a meaningful term (eg, when not
compiling Objective-C or Objective-C++). Perhaps the simplest approach would be
to only mention ARC if `getLangOpts().ObjCAutoRefCount` is set?
================
Comment at: clang/lib/AST/AttrImpl.cpp:221-226
+ // Elide constructors even if this is disabled with -fno-elide-constructors
+ // because 'musttail' is a more local setting.
+ if (CCE && CCE->isElidable()) {
+ assert(CCE->getNumArgs() == 1); // Copy or move constructor.
+ Ex = CCE->getArg(0);
+ }
----------------
`IgnoreImplicitAsWritten` should already skip over implicit elidable
constructors, so I would imagine this is skipping over elidable //explicit//
constructor calls (eg, `[[musttail]] return T(make());` would perform a
tail-call to `make()`). Is that what we want?
================
Comment at: clang/lib/CodeGen/CGStmt.cpp:665
+ SaveAndRestore<const CallExpr *> save_musttail(MustTailCall, musttail);
EmitStmt(S.getSubStmt(), S.getAttrs());
}
----------------
In the case where we're forcibly eliding a constructor, we'll need to emit a
return statement that returns `musttail` call expression here rather than
emitting the original substatement. Otherwise the tail call we emit will be
initializing a local temporary rather than initializing our return slot. Eg,
given:
```
struct A {
A(const A&);
~A();
char data[32];
};
A f();
A g() {
[[clang::musttail]] return f();
}
```
under `-fno-elide-constructors` when targeting C++11, say, we'll normally lower
that into something like:
```
void f(A *return_slot);
void g(A *return_slot) {
A temporary; //uninitialized
f(&temporary); // call f
A::A(return_slot, temporary); // call copy constructor to copy into return
slot
}
```
... and with the current patch, it looks like we'll add a 'ret void' after the
call to `f`, leaving `g`'s return slot uninitialized and passing an address
into `f` that refers to a variable that will no longer exist once `f` is
called. We need to instead lower to:
```
void f(A *return_slot);
void g(A *return_slot) {
f(return_slot); // call f
}
```
Probably the easiest way to do this would be to change the return value on the
`ReturnStmt` to be the tail-called `CallExpr` when attaching the attribute.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99517/new/
https://reviews.llvm.org/D99517
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
