[cfe-commits] [patch] Better diagnostics in for-range expressions.
Richard Smith
richard at metafoo.co.uk
Thu Jul 26 14:01:56 PDT 2012
Hi Sam,
Sorry for the massive delay reviewing this...
> diff --git a/include/clang/Basic/DiagnosticSemaKinds.td
b/include/clang/Basic/DiagnosticSemaKinds.td
> index 52641b8..97b0469 100644
> --- a/include/clang/Basic/DiagnosticSemaKinds.td
> +++ b/include/clang/Basic/DiagnosticSemaKinds.td
> @@ -1391,7 +1391,14 @@ def err_for_range_member_begin_end_mismatch :
Error<
> "range type %0 has '%select{begin|end}1' member but no
'%select{end|begin}1' member">;
> def err_for_range_begin_end_types_differ : Error<
> "'begin' and 'end' must return the same type (got %0 and %1)">;
> +def err_for_range_invalid: Error<
> + "invalid range expression of type %0">;
> +def note_for_range_adl_failure: Note<
> + "no viable '%select{begin|end}0' function for range of type %1">;
Please merge these into a single diagnostic (use the text of
note_for_range_adl_failure as the error message).
> def note_for_range_type : Note<"range has type %0">;
> +def err_for_range_dereference : Error<
> + "invalid range expression of type %0; did you mean to dereference it "
> + "with '*'?">;
> def note_for_range_begin_end : Note<
> "selected '%select{begin|end}0' %select{function|template }1%2 with
iterator type %3">;
>
> diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
> index a48cde0..29fbded 100644
> --- a/include/clang/Sema/Sema.h
> +++ b/include/clang/Sema/Sema.h
> @@ -1923,7 +1923,24 @@ public:
> Expr **Args, unsigned NumArgs,
> SourceLocation RParenLoc,
> Expr *ExecConfig,
> - bool AllowTypoCorrection=true);
> + OverloadCandidateSet *CandidateSet,
> + bool AllowTypoCorrection,
> + bool InRangeExpr);
> +
> + ExprResult BuildOverloadedCallExpr(Scope *S, Expr *Fn,
> + UnresolvedLookupExpr *ULE,
> + SourceLocation LParenLoc,
> + Expr **Args, unsigned NumArgs,
> + SourceLocation RParenLoc,
> + Expr *ExecConfig,
> + bool AllowTypoCorrection=true,
> + bool InRangeExpr=false);
I don't like passing InRangeExpr in here. It's too specific. It's also
unnecessary (more on that later).
> +
> + bool buildOverloadedCallSet(Scope *S, Expr *Fn, UnresolvedLookupExpr
*ULE,
> + Expr **Args, unsigned NumArgs,
> + SourceLocation RParenLoc,
> + OverloadCandidateSet *CandidateSet,
> + ExprResult *Result);
>
> ExprResult CreateOverloadedUnaryOp(SourceLocation OpLoc,
> unsigned Opc,
> diff --git a/lib/Sema/SemaOverload.cpp b/lib/Sema/SemaOverload.cpp
> index f045a2b..0a71dc7 100644
> --- a/lib/Sema/SemaOverload.cpp
> +++ b/lib/Sema/SemaOverload.cpp
> @@ -735,9 +735,12 @@
OverloadCandidate::DeductionFailureInfo::getSecondArg() {
> }
>
> void OverloadCandidateSet::clear() {
> - for (iterator i = begin(), e = end(); i != e; ++i)
> + for (iterator i = begin(), e = end(); i != e; ++i) {
> for (unsigned ii = 0, ie = i->NumConversions; ii != ie; ++ii)
> i->Conversions[ii].~ImplicitConversionSequence();
> + if (i->FailureKind == ovl_fail_bad_deduction)
> + i->DeductionFailure.Destroy();
> + }
I've applied this fix separately, thanks!
> NumInlineSequences = 0;
> Candidates.clear();
> Functions.clear();
> @@ -9688,55 +9691,30 @@ BuildRecoveryCallExpr(Sema &SemaRef, Scope *S,
Expr *Fn,
> RParenLoc);
> }
>
> -/// ResolveOverloadedCallFn - Given the call expression that calls Fn
> -/// (which eventually refers to the declaration Func) and the call
> -/// arguments Args/NumArgs, attempt to resolve the function call down
> -/// to a specific function. If overload resolution succeeds, returns
> -/// the function declaration produced by overload
> -/// resolution. Otherwise, emits diagnostics, deletes all of the
> -/// arguments and Fn, and returns NULL.
> -ExprResult
> -Sema::BuildOverloadedCallExpr(Scope *S, Expr *Fn, UnresolvedLookupExpr
*ULE,
> - SourceLocation LParenLoc,
> - Expr **Args, unsigned NumArgs,
> - SourceLocation RParenLoc,
> - Expr *ExecConfig,
> - bool AllowTypoCorrection) {
> -#ifndef NDEBUG
> - if (ULE->requiresADL()) {
> - // To do ADL, we must have found an unqualified name.
> - assert(!ULE->getQualifier() && "qualified name with ADL");
> -
> - // We don't perform ADL for implicit declarations of builtins.
> - // Verify that this was correctly set up.
> - FunctionDecl *F;
> - if (ULE->decls_begin() + 1 == ULE->decls_end() &&
> - (F = dyn_cast<FunctionDecl>(*ULE->decls_begin())) &&
> - F->getBuiltinID() && F->isImplicit())
> - llvm_unreachable("performing ADL for builtin");
> -
> - // We don't perform ADL in C.
> - assert(getLangOpts().CPlusPlus && "ADL enabled in C");
> - } else
> - assert(!ULE->isStdAssociatedNamespace() &&
> - "std is associated namespace but not doing ADL");
> -#endif
> -
> +/// \brief Constructs and populates an OverloadedCandidateSet from
> +/// the given function.
> +/// \returns true when an the ExprResult output parameter has been set.
> +bool Sema::buildOverloadedCallSet(Scope *S, Expr *Fn,
> + UnresolvedLookupExpr *ULE,
> + Expr **Args, unsigned NumArgs,
> + SourceLocation RParenLoc,
> + OverloadCandidateSet *CandidateSet,
> + ExprResult *Result) {
> UnbridgedCastsSet UnbridgedCasts;
> - if (checkArgPlaceholdersForOverload(*this, Args, NumArgs,
UnbridgedCasts))
> - return ExprError();
> -
> - OverloadCandidateSet CandidateSet(Fn->getExprLoc());
> + if (checkArgPlaceholdersForOverload(*this, Args, NumArgs,
UnbridgedCasts)) {
> + *Result = ExprError();
> + return true;
> + }
>
> // Add the functions denoted by the callee to the set of candidate
> // functions, including those from argument-dependent lookup.
> AddOverloadedCallCandidates(ULE, llvm::makeArrayRef(Args, NumArgs),
> - CandidateSet);
> + *CandidateSet);
>
> // If we found nothing, try to recover.
> // BuildRecoveryCallExpr diagnoses the error itself, so we just bail
> // out if it fails.
> - if (CandidateSet.empty()) {
> + if (CandidateSet->empty()) {
> // In Microsoft mode, if we are inside a template class member
function then
> // create a type dependent CallExpr. The goal is to postpone name
lookup
> // to instantiation time to be able to search into type dependent
base
> @@ -9747,18 +9725,43 @@ Sema::BuildOverloadedCallExpr(Scope *S, Expr *Fn,
UnresolvedLookupExpr *ULE,
> Context.DependentTy, VK_RValue,
> RParenLoc);
> CE->setTypeDependent(true);
> - return Owned(CE);
> + *Result = Owned(CE);
> + return true;
> }
> + return false;
> + }
> +
> + UnbridgedCasts.restore();
> + return false;
> +}
> +
> +
> +// FIXME: Update this stale comment
Please do :)
> +/// ResolveOverloadedCallFn - Given the call expression that calls Fn
> +/// (which eventually refers to the declaration Func) and the call
> +/// arguments Args/NumArgs, attempt to resolve the function call down
> +/// to a specific function. If overload resolution succeeds, returns
> +/// the function declaration produced by overload
> +/// resolution. Otherwise, emits diagnostics, deletes all of the
> +/// arguments and Fn, and returns NULL.
> +
> +ExprResult Sema::BuildOverloadedCallExpr(Scope *S, Expr *Fn,
> + UnresolvedLookupExpr *ULE,
> + SourceLocation LParenLoc,
> + Expr **Args, unsigned NumArgs,
> + SourceLocation RParenLoc,
> + Expr *ExecConfig,
> + OverloadCandidateSet
*CandidateSet,
> + bool AllowTypoCorrection,
> + bool InRangeExpr) {
> + if (CandidateSet->empty())
> return BuildRecoveryCallExpr(*this, S, Fn, ULE, LParenLoc,
> llvm::MutableArrayRef<Expr *>(Args,
NumArgs),
> RParenLoc, /*EmptyLookup=*/true,
> AllowTypoCorrection);
> - }
> -
> - UnbridgedCasts.restore();
>
> OverloadCandidateSet::iterator Best;
> - switch (CandidateSet.BestViableFunction(*this, Fn->getLocStart(),
Best)) {
> + switch (CandidateSet->BestViableFunction(*this, Fn->getLocStart(),
Best)) {
> case OR_Success: {
> FunctionDecl *FDecl = Best->Function;
> MarkFunctionReferenced(Fn->getExprLoc(), FDecl);
> @@ -9770,6 +9773,10 @@ Sema::BuildOverloadedCallExpr(Scope *S, Expr *Fn,
UnresolvedLookupExpr *ULE,
> }
>
> case OR_No_Viable_Function: {
> + // In case we're in a range expression, let the range handling code
take
> + // care of it.
> + if (InRangeExpr)
> + break;
This is the only use of InRangeExpr in this function, and we don't call
this function in the case where BestViableFunction returns
OR_No_Viable_Function. Is this parameter really needed?
> // Try to recover by looking for viable functions which the user
might
> // have meant to call.
> ExprResult Recovery = BuildRecoveryCallExpr(*this, S, Fn, ULE,
LParenLoc,
> @@ -9783,27 +9790,26 @@ Sema::BuildOverloadedCallExpr(Scope *S, Expr *Fn,
UnresolvedLookupExpr *ULE,
> Diag(Fn->getLocStart(),
> diag::err_ovl_no_viable_function_in_call)
> << ULE->getName() << Fn->getSourceRange();
> - CandidateSet.NoteCandidates(*this, OCD_AllCandidates,
> - llvm::makeArrayRef(Args, NumArgs));
> + CandidateSet->NoteCandidates(*this, OCD_AllCandidates,
> + llvm::makeArrayRef(Args, NumArgs));
> break;
> }
>
> case OR_Ambiguous:
> Diag(Fn->getLocStart(), diag::err_ovl_ambiguous_call)
> << ULE->getName() << Fn->getSourceRange();
> - CandidateSet.NoteCandidates(*this, OCD_ViableCandidates,
> - llvm::makeArrayRef(Args, NumArgs));
> + CandidateSet->NoteCandidates(*this, OCD_ViableCandidates,
> + llvm::makeArrayRef(Args, NumArgs));
> break;
>
> - case OR_Deleted:
> - {
> + case OR_Deleted: {
> Diag(Fn->getLocStart(), diag::err_ovl_deleted_call)
> << Best->Function->isDeleted()
> << ULE->getName()
> << getDeletedOrUnavailableSuffix(Best->Function)
> << Fn->getSourceRange();
> - CandidateSet.NoteCandidates(*this, OCD_AllCandidates,
> - llvm::makeArrayRef(Args, NumArgs));
> + CandidateSet->NoteCandidates(*this, OCD_AllCandidates,
> + llvm::makeArrayRef(Args, NumArgs));
>
> // We emitted an error for the unvailable/deleted function call
but keep
> // the call in the AST.
> @@ -9816,6 +9822,49 @@ Sema::BuildOverloadedCallExpr(Scope *S, Expr *Fn,
UnresolvedLookupExpr *ULE,
>
> // Overload resolution failed.
> return ExprError();
> +
> +}
> +
> +/// This version of BuildOverloadedCallExpr creates an
OverloadCandidateSet
> +/// first.
> +ExprResult Sema::BuildOverloadedCallExpr(Scope *S, Expr *Fn,
> + UnresolvedLookupExpr *ULE,
> + SourceLocation LParenLoc,
> + Expr **Args, unsigned NumArgs,
> + SourceLocation RParenLoc,
> + Expr *ExecConfig,
> + bool AllowTypoCorrection,
> + bool InRangeExpr) {
> +#ifndef NDEBUG
> + if (ULE->requiresADL()) {
> + // To do ADL, we must have found an unqualified name.
> + assert(!ULE->getQualifier() && "qualified name with ADL");
> +
> + // We don't perform ADL for implicit declarations of builtins.
> + // Verify that this was correctly set up.
> + FunctionDecl *F;
> + if (ULE->decls_begin() + 1 == ULE->decls_end() &&
> + (F = dyn_cast<FunctionDecl>(*ULE->decls_begin())) &&
> + F->getBuiltinID() && F->isImplicit())
> + llvm_unreachable("performing ADL for builtin");
> +
> + // We don't perform ADL in C.
> + assert(getLangOpts().CPlusPlus && "ADL enabled in C");
> + } else
> + assert(!ULE->isStdAssociatedNamespace() &&
> + "std is associated namespace but not doing ADL");
> +#endif
This checking belongs in buildOverloadedCallSet. We want to perform these
checks for range-based for calls too.
> +
> + OverloadCandidateSet CandidateSet(Fn->getExprLoc());
> + ExprResult result;
> + if (buildOverloadedCallSet(S, Fn, ULE, Args, NumArgs, LParenLoc,
> + &CandidateSet, &result))
> + return result;
> +
> + return BuildOverloadedCallExpr(S, Fn, ULE, LParenLoc, Args, NumArgs,
> + RParenLoc, ExecConfig, &CandidateSet,
> + AllowTypoCorrection, InRangeExpr);
> +
> }
>
> static bool IsOverloaded(const UnresolvedSetImpl &Functions) {
> diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp
> index 9be1d34..18bcf89 100644
> --- a/lib/Sema/SemaStmt.cpp
> +++ b/lib/Sema/SemaStmt.cpp
> @@ -1578,18 +1578,35 @@ void NoteForRangeBeginEndFunction(Sema &SemaRef,
Expr *E,
> << BEF << IsTemplate << Description << E->getType();
> }
>
> +
> +enum ForRangeStatus {
> + FRS_Success,
> + FRS_InvalidMemberFunction,
> + FRS_NoOverload,
> + FRS_DeletedOrAmbiguous,
> + FRS_NoViableFunction,
> + FRS_InvalidIteratorType,
> + FRS_BeginEndMismatch
> +};
> +
> /// Build a call to 'begin' or 'end' for a C++0x for-range statement. If
the
> /// given LookupResult is non-empty, it is assumed to describe a member
which
> /// will be invoked. Otherwise, the function will be found via argument
> /// dependent lookup.
> -static ExprResult BuildForRangeBeginEndCall(Sema &SemaRef, Scope *S,
> - SourceLocation Loc,
> - VarDecl *Decl,
> - BeginEndFunction BEF,
> - const DeclarationNameInfo
&NameInfo,
> - LookupResult &MemberLookup,
> - Expr *Range) {
> - ExprResult CallExpr;
> +/// CallExpr is set and FRS_Success returned on success, otherwise some
> +/// non-success value is returned.
> +static ForRangeStatus
> +BuildForRangeBeginEndCall(Sema &SemaRef, Scope *S,
> + SourceLocation Loc,
> + SourceLocation RangeLoc,
> + VarDecl *Decl,
> + BeginEndFunction BEF,
> + const DeclarationNameInfo &NameInfo,
> + LookupResult &MemberLookup,
> + OverloadCandidateSet *CandidateSet,
> + Expr *Range,
> + ExprResult *CallExpr) {
Now this is performing the call to BestViableFunction itself, this really
belongs in SemaOverload.cpp.
> + CandidateSet->clear();
> if (!MemberLookup.empty()) {
> ExprResult MemberRef =
> SemaRef.BuildMemberReferenceExpr(Range, Range->getType(), Loc,
> @@ -1598,12 +1615,16 @@ static ExprResult BuildForRangeBeginEndCall(Sema
&SemaRef, Scope *S,
> /*FirstQualifierInScope=*/0,
> MemberLookup,
> /*TemplateArgs=*/0);
> - if (MemberRef.isInvalid())
> - return ExprError();
> - CallExpr = SemaRef.ActOnCallExpr(S, MemberRef.get(), Loc,
MultiExprArg(),
> + if (MemberRef.isInvalid()) {
> + *CallExpr = ExprError();
> + return FRS_InvalidMemberFunction;
> + }
> + *CallExpr = SemaRef.ActOnCallExpr(S, MemberRef.get(), Loc,
MultiExprArg(),
> Loc, 0);
> - if (CallExpr.isInvalid())
> - return ExprError();
> + if (CallExpr->isInvalid()) {
> + *CallExpr = ExprError();
> + return FRS_InvalidMemberFunction;
> + }
> } else {
> UnresolvedSet<0> FoundNames;
> // C++0x [stmt.ranged]p1: For the purposes of this name lookup,
namespace
> @@ -1614,20 +1635,49 @@ static ExprResult BuildForRangeBeginEndCall(Sema
&SemaRef, Scope *S,
> /*NeedsADL=*/true,
/*Overloaded=*/false,
> FoundNames.begin(), FoundNames.end(),
> /*LookInStdNamespace=*/true);
> - CallExpr = SemaRef.BuildOverloadedCallExpr(S, Fn, Fn, Loc, &Range,
1, Loc,
> - 0,
/*AllowTypoCorrection=*/false);
> - if (CallExpr.isInvalid()) {
> - SemaRef.Diag(Range->getLocStart(), diag::note_for_range_type)
> - << Range->getType();
> - return ExprError();
> +
> + SemaRef.buildOverloadedCallSet(S, Fn, Fn, &Range, 1, Loc,
CandidateSet,
> + CallExpr);
> +
> + OverloadCandidateSet::iterator Best;
> + if (CandidateSet->empty()) {
> + *CallExpr = ExprError();
> + return FRS_NoOverload;
> + } else {
> + switch (CandidateSet->BestViableFunction(SemaRef,
Fn->getLocStart(),
> + Best)) {
> + case OR_Success:
> + *CallExpr =
> + SemaRef.BuildOverloadedCallExpr(S, Fn, Fn, Loc, &Range, 1,
Loc, 0,
> + CandidateSet,
> +
/*AllowTypoCorrection=*/false,
> + /*InRangeExpr=*/true);
In the common case of a success, we're calling BestViableFunction twice;
once here and once in BuildOverloadedCallExpr. Can you factor out the
'OR_Success' case from there and just call it directly?
> + if (CallExpr->isInvalid())
> + return FRS_NoOverload;
> + break;
> +
> + case OR_No_Viable_Function:
> + *CallExpr = ExprError();
> + return FRS_NoViableFunction;
> +
> + case OR_Deleted:
> + case OR_Ambiguous:
> + // Here we want to defer to the diagnostics in
BuildOverloadedCallExpr.
> + // The return value doesn't matter, since we're returning an
ExprError()
> + // anyway.
> + SemaRef.BuildOverloadedCallExpr(S, Fn, Fn, Loc, &Range, 1, Loc,
0,
> + /*AllowTypoCorrection=*/false,
> + /*InRangeExpr=*/true);
> + *CallExpr = ExprError();
> + return FRS_DeletedOrAmbiguous;
This case is a bit awkward: we're calling BuildOverloadedCallExpr in order
to generate diagnostics which we don't actually want (the diagnostics we do
want are produced later by ForRangeDiagnostics). How about producing the
diagnostics directly here, instead of calling BuildOverloadedCallExpr?
> + }
> }
> +
> }
> - if (FinishForRangeVarDecl(SemaRef, Decl, CallExpr.get(), Loc,
> - diag::err_for_range_iter_deduction_failure))
{
> - NoteForRangeBeginEndFunction(SemaRef, CallExpr.get(), BEF);
> - return ExprError();
> - }
> - return CallExpr;
> + if (FinishForRangeVarDecl(SemaRef, Decl, CallExpr->get(), Loc,
> + diag::err_for_range_iter_deduction_failure))
> + return FRS_InvalidIteratorType;
> + return FRS_Success;
> }
>
> }
> @@ -1700,6 +1750,129 @@ Sema::ActOnCXXForRangeStmt(SourceLocation ForLoc,
SourceLocation LParenLoc,
> RParenLoc);
> }
>
> +// Emit the diagnostic appropriate to the given ForRangeStatus. The other
> +// parameters are used for supplementary information.
> +static void ForRangeDiagnostics(Sema &SemaRef, ForRangeStatus st, Expr
*Range,
> + const ExprResult &CallExpr,
> + SourceLocation RangeLoc,
> + OverloadCandidateSet *CandidateSet,
> + BeginEndFunction BEF) {
> + switch (st) {
> + case FRS_Success:
> + // It worked; nothing to do.
> + break;
> +
> + case FRS_InvalidMemberFunction:
> + SemaRef.Diag(Range->getLocStart(), diag::err_for_range_invalid)
> + << RangeLoc << Range->getType();
> + SemaRef.Diag(Range->getLocStart(), diag::note_for_range_adl_failure)
> + << BEF << Range->getType();
> + break;
> +
> + case FRS_NoOverload:
> + SemaRef.Diag(Range->getLocStart(), diag::err_for_range_invalid)
> + << RangeLoc << Range->getType();
> + SemaRef.Diag(Range->getLocStart(), diag::note_for_range_adl_failure)
> + << BEF << Range->getType();
> + break;
> +
> + case FRS_DeletedOrAmbiguous:
> + // Some of the diagnostics are emitted in BuildOverloadedCallExpr,
so we
> + // just add these.
> + SemaRef.Diag(Range->getLocStart(), diag::err_for_range_invalid)
> + << RangeLoc << Range->getType();
> + SemaRef.Diag(Range->getLocStart(), diag::note_for_range_adl_failure)
> + << BEF << Range->getType();
> + break;
> +
> + case FRS_NoViableFunction:
> + SemaRef.Diag(Range->getLocStart(), diag::err_for_range_invalid)
> + << RangeLoc << Range->getType();
> + CandidateSet->NoteCandidates(SemaRef, OCD_AllCandidates,
> + llvm::makeArrayRef(&Range,
/*NumArgs=*/1));
> + SemaRef.Diag(Range->getLocStart(), diag::note_for_range_adl_failure)
> + << BEF << Range->getType();
> + break;
> +
> + case FRS_InvalidIteratorType:
> + NoteForRangeBeginEndFunction(SemaRef, CallExpr.get(), BEF);
> + break;
> +
> + case FRS_BeginEndMismatch:
> + SemaRef.Diag(RangeLoc, diag::err_for_range_member_begin_end_mismatch)
> + << Range->getType() << BEF;
> + break;
> + }
> +}
I don't like that we sometimes produce notes here which attach to
diagnostics produced in BuildForRangeBeginEndCall. I'd like to generally
see this restructured as follows:
BuildForRangeBeginEndCall's return value indicates either 'everything was
OK', 'this isn't a valid range because there's no viable begin/end
function', or 'something else went wrong and I produced a diagnostic'.
Then, produce a diagnostic here only in the second case.
> +
> +/// \brief Create the initialization, compare, and increment steps for
> +/// the range-based for loop expression.
> +/// This function does not handle array-based for loops,
> +/// which are created in Sema::BuildCXXForRangeStmt.
> +///
> +/// \returns a ForRangeStatus indicating success or what kind of error
occurred.
> +/// BeginExpr and EndExpr are set and FRS_Success is returned on success;
> +/// CandidateSet and BEF are set and some non-success value is returned
on
> +/// failure.
> +static ForRangeStatus BuildNonArrayForRange(Sema &SemaRef, Scope *S,
> + Expr *BeginRange, Expr
*EndRange,
> + QualType RangeType,
> + VarDecl *BeginVar,
> + VarDecl *EndVar,
> + SourceLocation ColonLoc,
> + OverloadCandidateSet
*CandidateSet,
> + ExprResult *BeginExpr,
> + ExprResult *EndExpr,
> + BeginEndFunction *BEF) {
> + DeclarationNameInfo BeginNameInfo(
> + &SemaRef.PP.getIdentifierTable().get("begin"), ColonLoc);
> + DeclarationNameInfo
EndNameInfo(&SemaRef.PP.getIdentifierTable().get("end"),
> + ColonLoc);
> +
> + LookupResult BeginMemberLookup(SemaRef, BeginNameInfo,
> + Sema::LookupMemberName);
> + LookupResult EndMemberLookup(SemaRef, EndNameInfo,
Sema::LookupMemberName);
> +
> + if (CXXRecordDecl *D = RangeType->getAsCXXRecordDecl()) {
> + // - if _RangeT is a class type, the unqualified-ids begin and end
are
> + // looked up in the scope of class _RangeT as if by class member
access
> + // lookup (3.4.5), and if either (or both) finds at least one
> + // declaration, begin-expr and end-expr are __range.begin() and
> + // __range.end(), respectively;
> + SemaRef.LookupQualifiedName(BeginMemberLookup, D);
> + SemaRef.LookupQualifiedName(EndMemberLookup, D);
> +
> + if (BeginMemberLookup.empty() != EndMemberLookup.empty()) {
> + *BEF = BeginMemberLookup.empty() ? BEF_end : BEF_begin;
> + return FRS_BeginEndMismatch;
> + }
> + } else {
> + // - otherwise, begin-expr and end-expr are begin(__range) and
> + // end(__range), respectively, where begin and end are looked up
with
> + // argument-dependent lookup (3.4.2). For the purposes of this name
> + // lookup, namespace std is an associated namespace.
> +
> + }
> +
> + *BEF = BEF_begin;
> + ForRangeStatus
> + RangeStatus = BuildForRangeBeginEndCall(SemaRef, S, ColonLoc,
> + ColonLoc, BeginVar,
> + BEF_begin, BeginNameInfo,
> + BeginMemberLookup,
> + CandidateSet,
> + BeginRange, BeginExpr);
> + if (RangeStatus != FRS_Success)
> + return st;
> +
> + *BEF = BEF_end;
> + RangeStatus = BuildForRangeBeginEndCall(SemaRef, S, ColonLoc, ColonLoc,
> + EndVar, BEF_end, EndNameInfo,
> + EndMemberLookup, CandidateSet,
> + EndRange, EndExpr);
> + return RangeStatus;
> +}
> +
> /// BuildCXXForRangeStmt - Build or instantiate a C++0x for-range
statement.
> StmtResult
> Sema::BuildCXXForRangeStmt(SourceLocation ForLoc, SourceLocation
ColonLoc,
> @@ -1791,49 +1964,58 @@ Sema::BuildCXXForRangeStmt(SourceLocation ForLoc,
SourceLocation ColonLoc,
> return StmtError();
> }
> } else {
> - DeclarationNameInfo
BeginNameInfo(&PP.getIdentifierTable().get("begin"),
> - ColonLoc);
> - DeclarationNameInfo
EndNameInfo(&PP.getIdentifierTable().get("end"),
> - ColonLoc);
> -
> - LookupResult BeginMemberLookup(*this, BeginNameInfo,
LookupMemberName);
> - LookupResult EndMemberLookup(*this, EndNameInfo, LookupMemberName);
> -
> - if (CXXRecordDecl *D = RangeType->getAsCXXRecordDecl()) {
> - // - if _RangeT is a class type, the unqualified-ids begin and
end are
> - // looked up in the scope of class _RangeT as if by class
member access
> - // lookup (3.4.5), and if either (or both) finds at least one
> - // declaration, begin-expr and end-expr are __range.begin() and
> - // __range.end(), respectively;
> - LookupQualifiedName(BeginMemberLookup, D);
> - LookupQualifiedName(EndMemberLookup, D);
> -
> - if (BeginMemberLookup.empty() != EndMemberLookup.empty()) {
> - Diag(ColonLoc, diag::err_for_range_member_begin_end_mismatch)
> - << RangeType << BeginMemberLookup.empty();
> + Expr *BeginRange = BeginRangeRef.get();
> + Expr *EndRange = EndRangeRef.get();
> + OverloadCandidateSet CandidateSet(RangeLoc);
> + BeginEndFunction BEFFailure;
> + ForRangeStatus RangeStatus =
> + BuildNonArrayForRange(*this, S, BeginRange, EndRange,
RangeType,
> + BeginVar, EndVar, ColonLoc,
&CandidateSet,
> + &BeginExpr, &EndExpr, &BEFFailure);
> +
> + // Attempt to dereference pointers and try again.
> + if (RangeStatus != FRS_Success) {
> + if (!RangeType->isPointerType()) {
> + const ExprResult & TheCall = BEFFailure ? EndExpr: BeginExpr;
> + ForRangeDiagnostics(*this, RangeStatus,
> + BEFFailure ? EndRange : BeginRange,
> + TheCall, RangeLoc, &CandidateSet,
BEFFailure);
> return StmtError();
> }
> - } else {
> - // - otherwise, begin-expr and end-expr are begin(__range) and
> - // end(__range), respectively, where begin and end are looked
up with
> - // argument-dependent lookup (3.4.2). For the purposes of this
name
> - // lookup, namespace std is an associated namespace.
> + BeginRange =
> + new (Context) UnaryOperator(BeginRange, UO_Deref,
> + RangeType->getPointeeType(),
> + VK_LValue, OK_Ordinary,
RangeLoc);
> + EndRange =
> + new (Context) UnaryOperator(EndRange, UO_Deref,
> + RangeType->getPointeeType(),
> + VK_LValue, OK_Ordinary,
RangeLoc);
> +
> + BeginEndFunction DerefBEF;
> + ForRangeStatus DerefStatus
> + = BuildNonArrayForRange(*this, S, BeginRange, EndRange,
> + BeginRange->getType(),
> + BeginVar, EndVar,
> + ColonLoc, &CandidateSet,
> + &BeginExpr, &EndExpr, &DerefBEF);
> + if (DerefStatus != FRS_Success) {
> + const ExprResult & TheCall = BEFFailure ? EndExpr: BeginExpr;
> + ForRangeDiagnostics(*this, RangeStatus,
> + DerefBEF ? EndRangeRef.get():
BeginRangeRef.get(),
> + TheCall, RangeLoc, &CandidateSet,
BEFFailure);
> + return StmtError();
> + } else {
> + // The attempt to dereference would likely succeed.
> + Diag(RangeLoc, diag::err_for_range_dereference)
> + << RangeLoc << RangeType
> + << FixItHint::CreateInsertion(RangeLoc, "*");
Can you make this comment a bit more precise? The bar for producing a fixit
on an error is that the fix would succeed and would produce the same AST.
Here, we're taking some liberties, because we're applying the '*' within
the __begin and __end expressions instead of to the __range initializer.
That's equivalent, but doesn't provide an identical AST, so you should
either justify it or rebuild __range with the right expression here.
> + }
> }
> -
> - BeginExpr = BuildForRangeBeginEndCall(*this, S, ColonLoc, BeginVar,
> - BEF_begin, BeginNameInfo,
> - BeginMemberLookup,
> - BeginRangeRef.get());
> - if (BeginExpr.isInvalid())
> - return StmtError();
> -
> - EndExpr = BuildForRangeBeginEndCall(*this, S, ColonLoc, EndVar,
> - BEF_end, EndNameInfo,
> - EndMemberLookup,
EndRangeRef.get());
> - if (EndExpr.isInvalid())
> - return StmtError();
> }
>
> + assert(!BeginExpr.isInvalid() && !EndExpr.isInvalid() &&
> + "invalid range expression in for loop");
> +
> // C++0x [decl.spec.auto]p6: BeginType and EndType must be the same.
> QualType BeginType = BeginVar->getType(), EndType =
EndVar->getType();
> if (!Context.hasSameType(BeginType, EndType)) {
> diff --git a/test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp
b/test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp
> index 96bb472..e5809f0 100644
> --- a/test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp
> +++ b/test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp
> @@ -3,20 +3,21 @@
> struct pr12960 {
> int begin;
> void foo(int x) {
> - for (int& it : x) { // expected-error {{use of undeclared identifier
'begin'}} expected-note {{range has type 'int'}}
> + for (int& it : x) { //expected-error{{invalid range expression}}\
> + expected-note{{no viable 'begin' function for range of type 'int'}}
> }
> }
> };
>
> namespace std {
> template<typename T>
> - auto begin(T &&t) -> decltype(t.begin()) { return t.begin(); } //
expected-note 4{{ignored: substitution failure}}
> + auto begin(T &&t) -> decltype(t.begin()) { return t.begin(); } //
expected-note 3{{ignored: substitution failure}}
> template<typename T>
> auto end(T &&t) -> decltype(t.end()) { return t.end(); } //
expected-note {{candidate template ignored: substitution failure [with T =
}}
>
> template<typename T>
> auto begin(T &&t) -> decltype(t.alt_begin()) { return t.alt_begin();
} // expected-note {{selected 'begin' template [with T = }} \
> -
expected-note 4{{candidate template ignored: substitution failure
[with T = }}
> +
expected-note 3{{candidate template ignored: substitution failure
[with T = }}
> template<typename T>
> auto end(T &&t) -> decltype(t.alt_end()) { return t.alt_end(); } //
expected-note {{candidate template ignored: substitution failure [with T =
}}
>
> @@ -116,9 +117,11 @@ void g() {
> struct NoEndADL {
> null_t alt_begin();
> };
> - for (auto u : NoBeginADL()) { // expected-error {{no matching function
for call to 'begin'}} expected-note {{range has type 'NoBeginADL'}}
> + for (auto u : NoBeginADL()) {// expected-error{{invalid range
expression of type 'NoBeginADL'}} \
> + expected-note{{no viable 'begin' function for range of type
'NoBeginADL'}}
> }
> - for (auto u : NoEndADL()) { // expected-error {{no matching function
for call to 'end'}} expected-note {{range has type 'NoEndADL'}}
> + for (auto u : NoEndADL()) { // expected-error{{invalid range
expression of type 'NoEndADL'}} \
> + expected-note{{no viable 'end' function for range of type
'NoEndADL'}}
> }
>
> struct NoBegin {
> @@ -156,8 +159,8 @@ void g() {
> for (int n : NoCopy()) { // ok
> }
>
> - for (int n : 42) { // expected-error {{no matching function for call
to 'begin'}} \
> - expected-note {{range has type 'int'}}
> + for (int n : 42) { // expected-error{{invalid range expression of type
'int'}} \
> + expected-note{{no viable 'begin' function for range of type 'int'}}
> }
>
> for (auto a : *also_incomplete) { // expected-error {{cannot use
incomplete type 'struct Incomplete' as a range}}
> @@ -179,9 +182,10 @@ template void h<A(&)[13], int>(A(&)[13]); //
expected-note {{requested here}}
>
> template<typename T>
> void i(T t) {
> - for (auto u : t) { // expected-error {{no matching function for call
to 'begin'}} \
> - expected-error {{member function 'begin' not
viable}} \
> - expected-note {{range has type}}
> + for (auto u : t) { // expected-error {{invalid range expression of
type 'A *'; did you mean to dereference it with '*'?}} \
> + expected-error {{member function 'begin' not
viable}}\
> + expected-error{{invalid range expression of type
'const A'}}\
> + expected-note{{no viable 'begin' function for
range of type 'const A'}}
> }
> }
> template void i<A[13]>(A*); // expected-note {{requested here}}
> @@ -204,7 +208,8 @@ void end(VoidBeginADL);
> void j() {
> for (auto u : NS::ADL()) {
> }
> - for (auto u : NS::NoADL()) { // expected-error {{no matching function
for call to 'begin'}} expected-note {{range has type}}
> + for (auto u : NS::NoADL()) { // expected-error{{invalid range
expression of type 'NS::NoADL'}}\
> + expected-note {{no viable 'begin' function for range of type
'NS::NoADL'}}
> }
> for (auto a : VoidBeginADL()) { // expected-error {{cannot use type
'void' as an iterator}}
> }
> @@ -215,4 +220,3 @@ void example() {
> for (int &x : array)
> x *= 2;
> }
> -
> diff --git a/test/SemaCXX/for-range-no-std.cpp
b/test/SemaCXX/for-range-no-std.cpp
> index fa42ca4..8f3213b 100644
> --- a/test/SemaCXX/for-range-no-std.cpp
> +++ b/test/SemaCXX/for-range-no-std.cpp
> @@ -31,10 +31,11 @@ NS::iter end(NS::NoADL);
> void f() {
> int a[] = {1, 2, 3};
> for (auto b : S()) {} // ok
> - for (auto b : T()) {} // expected-error {{no matching function for
call to 'begin'}} expected-note {{range has type}}
> + for (auto b : T()) {} // expected-error {{invalid range expression of
type 'T'}} expected-note {{no viable 'begin' function for range of type
'T'}}
> for (auto b : a) {} // ok
> for (int b : NS::ADL()) {} // ok
> - for (int b : NS::NoADL()) {} // expected-error {{no matching function
for call to 'begin'}} expected-note {{range has type}}
> + for (int b : NS::NoADL()) {} // expected-error {{invalid range
expression of type 'NS::NoADL'}} \
> + expected-note {{no viable 'begin' function for range of type
'NS::NoADL'}}
> }
>
> void PR11601() {
> diff --git a/test/SemaCXX/typo-correction.cpp
b/test/SemaCXX/typo-correction.cpp
> index 893f08a..f8bb8d0 100644
> --- a/test/SemaCXX/typo-correction.cpp
> +++ b/test/SemaCXX/typo-correction.cpp
> @@ -155,7 +155,8 @@ void Test3() {
> struct R {};
> bool begun(R);
> void RangeTest() {
> - for (auto b : R()) {} // expected-error {{use of undeclared identifier
'begin'}} expected-note {{range has type}}
> + for (auto b : R()) {} // expected-error {{invalid range expression of
type 'R'}}\
> + expected-note {{no viable 'begin' function for range of type 'R'}}
> }
>
> // PR 12019 - Avoid infinite mutual recursion in
DiagnoseInvalidRedeclaration
> --
> 1.7.7.3
>
On Mon, Jul 23, 2012 at 3:41 PM, Sam Panzer <panzer at google.com> wrote:
> Ping.
>
>
> On Wed, Jul 11, 2012 at 4:29 PM, Sam Panzer <panzer at google.com> wrote:
>
>> Ping, and a slightly updated version which changed a few comments to make
>> doxygen happy and renamed a few variables to fit the conventions better.
>>
>>
>> On Mon, Jun 18, 2012 at 9:40 AM, Sam Panzer <panzer at google.com> wrote:
>>
>>> *sigh*
>>> I ran format-patch from the correct branch this time.
>>>
>>> On Sun, Jun 17, 2012 at 4:49 PM, Richard Smith <richard at metafoo.co.uk>wrote:
>>>
>>>> On Thu, Jun 14, 2012 at 10:32 AM, Sam Panzer <panzer at google.com> wrote:
>>>> > I will now be including the word 'attach' in these emails as Gmail's
>>>> > equivalent of -Wmissing-patch.
>>>>
>>>> Sadly that won't save you from running 'diff' in the wrong checkout.
>>>> This is the printf / c_str patch.
>>>>
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120726/d2d8e260/attachment.html>
More information about the cfe-commits
mailing list