george.burgess.iv added inline comments.
================
Comment at: lib/Sema/SemaExpr.cpp:5047-5098
@@ -5046,57 +5046,54 @@
+CallExpr *Sema::buildDependentCallExpr(Expr *ExecConfig, Expr *Fn,
+ MultiExprArg ArgExprs,
+ SourceLocation RParenLoc) {
+ if (ExecConfig)
+ return new (Context)
+ CUDAKernelCallExpr(Context, Fn, cast<CallExpr>(ExecConfig), ArgExprs,
+ Context.DependentTy, VK_RValue, RParenLoc);
+ return new (Context) CallExpr(Context, Fn, ArgExprs, Context.DependentTy,
+ VK_RValue, RParenLoc);
+}
+
/// ActOnCallExpr - Handle a call to Fn with the specified array of arguments.
/// This provides the location of the left/right parens and a list of comma
/// locations.
ExprResult
Sema::ActOnCallExpr(Scope *S, Expr *Fn, SourceLocation LParenLoc,
MultiExprArg ArgExprs, SourceLocation RParenLoc,
Expr *ExecConfig, bool IsExecConfig) {
// Since this might be a postfix expression, get rid of ParenListExprs.
ExprResult Result = MaybeConvertParenListExprToParenExpr(S, Fn);
if (Result.isInvalid()) return ExprError();
Fn = Result.get();
if (checkArgsForPlaceholders(*this, ArgExprs))
return ExprError();
if (getLangOpts().CPlusPlus) {
// If this is a pseudo-destructor expression, build the call immediately.
if (isa<CXXPseudoDestructorExpr>(Fn)) {
if (!ArgExprs.empty()) {
// Pseudo-destructor calls should not have any arguments.
Diag(Fn->getLocStart(), diag::err_pseudo_dtor_call_with_args)
<< FixItHint::CreateRemoval(
SourceRange(ArgExprs.front()->getLocStart(),
ArgExprs.back()->getLocEnd()));
}
return new (Context)
CallExpr(Context, Fn, None, Context.VoidTy, VK_RValue, RParenLoc);
}
if (Fn->getType() == Context.PseudoObjectTy) {
ExprResult result = CheckPlaceholderExpr(Fn);
if (result.isInvalid()) return ExprError();
Fn = result.get();
}
// Determine whether this is a dependent call inside a C++ template,
// in which case we won't do any semantic analysis now.
// FIXME: Will need to cache the results of name lookup (including ADL) in
// Fn.
- bool Dependent = false;
- if (Fn->isTypeDependent())
- Dependent = true;
- else if (Expr::hasAnyTypeDependentArguments(ArgExprs))
- Dependent = true;
-
- if (Dependent) {
- if (ExecConfig) {
- return new (Context) CUDAKernelCallExpr(
- Context, Fn, cast<CallExpr>(ExecConfig), ArgExprs,
- Context.DependentTy, VK_RValue, RParenLoc);
- } else {
- return new (Context) CallExpr(
- Context, Fn, ArgExprs, Context.DependentTy, VK_RValue, RParenLoc);
- }
- }
+ if (Fn->isTypeDependent() || Expr::hasAnyTypeDependentArguments(ArgExprs))
+ return buildDependentCallExpr(ExecConfig, Fn, ArgExprs, RParenLoc);
----------------
rsmith wrote:
> You don't seem to make use of this refactoring in the functional change
> below. Either revert or commit separately, please (and if you commit it, fix
> the comment in Sema.h to reflect what this is used for).
SG; will just revert.
================
Comment at: lib/Sema/SemaExpr.cpp:5095-5096
@@ -5083,3 +5094,4 @@
// in which case we won't do any semantic analysis now.
// FIXME: Will need to cache the results of name lookup (including ADL) in
// Fn.
+ if (Fn->isTypeDependent() || Expr::hasAnyTypeDependentArguments(ArgExprs))
----------------
rsmith wrote:
> Can you delete this FIXME? I'm pretty sure we've got that right for years :)
r265341
================
Comment at: lib/Sema/SemaExpr.cpp:5202-5205
@@ +5201,6 @@
+ Fn, NDecl, LParenLoc, ArgExprs, RParenLoc, ExecConfig, IsExecConfig);
+ if (MakeCallValueDependent) {
+ BuiltCall.get()->setValueDependent(true);
+ BuiltCall.get()->setInstantiationDependent(true);
+ }
+
----------------
rsmith wrote:
> Does this need to be value-dependent? The only possible outcomes here are
> either that we call the designated function or that instantiation fails,
> which seems like instantiation-dependence is enough. (In practice, the
> `setValueDependent` call will be a no-op, because the only way the flag gets
> set is if one of the `ArgExprs` is value-dependent, which results in the call
> being value-dependent regardless, so there should be no functional change in
> removing the `setValueDependent` call.)
If instantiation dependence is enough to be sure that we re-evaluate the
`enable_if` condition(s) when we have all of the values we need, then not
marking this as value-dependent seems like a better approach, yeah. :)
> In practice, the setValueDependent call will be a no-op, because the only way
> the flag gets set is if one of the ArgExprs is value-dependent, which results
> in the call being value-dependent regardless, so there should be no
> functional change in removing the setValueDependent call
Given that we no longer care about value-dependence, it doesn't matter, but
will this also happen when the `enable_if` condition itself is value-dependent?
================
Comment at: lib/Sema/SemaOverload.cpp:5872
@@ -5845,2 +5871,3 @@
+ if (void *Failed = packedCheckEnableIf(*this, Function, Args)) {
Candidate.Viable = false;
Candidate.FailureKind = ovl_fail_enable_if;
----------------
rsmith wrote:
> I'm nervous about setting `Viable` to `false` for a function that *might be*
> viable; I expect a lot of code to "misinterpret" that. If we go ahead with
> this direction, I'd prefer for the `Viable` flag to become an enum of
> `NonViable`, `Viable`, `Dependent`, have overload resolution treat
> dependently-viable functions like viable functions, and add an
> `OverloadingResult` of `OR_Dependent` for the case where there was a unique
> best result that was dependently viable. All 31 callers of
> `BestViableFunction` would then need to check for and handle the new case,
> and build a dependent call expression in that case.
>
> However, this seems like a lot of complexity to deal with this case. Let's
> talk offline and see if we can come up with something better.
Works for me; will come bug you soonish.
http://reviews.llvm.org/D18425
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
