brunodf added inline comments.
================
Comment at: clang/lib/Sema/TreeTransform.h:14583
if (Second && Second->getObjectKind() == OK_ObjCProperty) {
ExprResult Result = SemaRef.CheckPlaceholderExpr(Second);
----------------
rjmccall wrote:
> rnk wrote:
> > This is also pseudo object handling code
> Hmm. Am I wrong to be concerned about folding overload placeholders too
> early in these clauses? Surely overloads can be resolved by the operator
> call in some cases.
>
> I agree with Reid that it would be really nice if we could make this share
> the normal paths for C++ operator resolution instead of duplicating so much
> of them.
> Hmm. Am I wrong to be concerned about folding overload placeholders too
> early in these clauses? Surely overloads can be resolved by the operator
> call in some cases.
This would have to be reviewed against the paths in `Sema::BuildBinOp`? There
are some cases for a RHS placeholder that does not end in calling
`CheckPlaceholderExpr`:
```
// Handle pseudo-objects in the RHS.
if (const BuiltinType *pty = RHSExpr->getType()->getAsPlaceholderType()) {
// An overload in the RHS can potentially be resolved by the type
// being assigned to.
if (Opc == BO_Assign && pty->getKind() == BuiltinType::Overload) {
if (getLangOpts().CPlusPlus &&
(LHSExpr->isTypeDependent() || RHSExpr->isTypeDependent() ||
LHSExpr->getType()->isOverloadableType()))
return BuildOverloadedBinOp(*this, S, OpLoc, Opc, LHSExpr, RHSExpr);
return CreateBuiltinBinOp(OpLoc, Opc, LHSExpr, RHSExpr);
}
// Don't resolve overloads if the other type is overloadable.
if (getLangOpts().CPlusPlus && pty->getKind() == BuiltinType::Overload &&
LHSExpr->getType()->isOverloadableType())
return BuildOverloadedBinOp(*this, S, OpLoc, Opc, LHSExpr, RHSExpr);
ExprResult resolvedRHS = CheckPlaceholderExpr(RHSExpr);
if (!resolvedRHS.isUsable()) return ExprError();
RHSExpr = resolvedRHS.get();
}
```
> I agree with Reid that it would be really nice if we could make this share
> the normal paths for C++ operator resolution instead of duplicating so much
> of them.
I think extracting the common placeholder logic from Build(Unary|Bin)Op and
this function is beyond the scope of this patch (and probably outside my
capabilities, I'm afraid). Still I hope to get things in a better shape with
this patch than it was before (see PR51855). A follow up that would attempt to
remove the duplication would indeed be nice.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111639/new/
https://reviews.llvm.org/D111639
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
