Author: rsmith Date: Mon Sep 24 16:17:44 2018 New Revision: 342925 URL: http://llvm.org/viewvc/llvm-project?rev=342925&view=rev Log: P0962R1: only use the member form of 'begin' and 'end' in a range-based for loop if both members exist.
This resolves a DR whereby an errant 'begin' or 'end' member in a base class could result in a derived class not being usable as a range with non-member 'begin' and 'end'. Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/Sema/SemaStmt.cpp cfe/trunk/test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp cfe/trunk/test/SemaCXX/for-range-dereference.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=342925&r1=342924&r2=342925&view=diff ============================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Sep 24 16:17:44 2018 @@ -2254,8 +2254,6 @@ def err_for_range_incomplete_type : Erro "cannot use incomplete type %0 as a range">; def err_for_range_iter_deduction_failure : Error< "cannot use type %0 as an iterator">; -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 ext_for_range_begin_end_types_differ : ExtWarn< "'begin' and 'end' returning different types (%0 and %1) is a C++17 extension">, InGroup<CXX17>; @@ -2268,6 +2266,8 @@ def note_in_for_range: Note< def err_for_range_invalid: Error< "invalid range expression of type %0; no viable '%select{begin|end}1' " "function available">; +def note_for_range_member_begin_end_ignored : Note< + "member is not a candidate because range type %0 has no '%select{end|begin}1' member">; def err_range_on_array_parameter : Error< "cannot build range expression with array function parameter %0 since " "parameter with array type %1 is treated as pointer type %2">; Modified: cfe/trunk/lib/Sema/SemaStmt.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=342925&r1=342924&r2=342925&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaStmt.cpp (original) +++ cfe/trunk/lib/Sema/SemaStmt.cpp Mon Sep 24 16:17:44 2018 @@ -2149,6 +2149,56 @@ BuildNonArrayForRange(Sema &SemaRef, Exp Sema::LookupMemberName); LookupResult EndMemberLookup(SemaRef, EndNameInfo, Sema::LookupMemberName); + auto BuildBegin = [&] { + *BEF = BEF_begin; + Sema::ForRangeStatus RangeStatus = + SemaRef.BuildForRangeBeginEndCall(ColonLoc, ColonLoc, BeginNameInfo, + BeginMemberLookup, CandidateSet, + BeginRange, BeginExpr); + + if (RangeStatus != Sema::FRS_Success) { + if (RangeStatus == Sema::FRS_DiagnosticIssued) + SemaRef.Diag(BeginRange->getBeginLoc(), diag::note_in_for_range) + << ColonLoc << BEF_begin << BeginRange->getType(); + return RangeStatus; + } + if (!CoawaitLoc.isInvalid()) { + // FIXME: getCurScope() should not be used during template instantiation. + // We should pick up the set of unqualified lookup results for operator + // co_await during the initial parse. + *BeginExpr = SemaRef.ActOnCoawaitExpr(SemaRef.getCurScope(), ColonLoc, + BeginExpr->get()); + if (BeginExpr->isInvalid()) + return Sema::FRS_DiagnosticIssued; + } + if (FinishForRangeVarDecl(SemaRef, BeginVar, BeginExpr->get(), ColonLoc, + diag::err_for_range_iter_deduction_failure)) { + NoteForRangeBeginEndFunction(SemaRef, BeginExpr->get(), *BEF); + return Sema::FRS_DiagnosticIssued; + } + return Sema::FRS_Success; + }; + + auto BuildEnd = [&] { + *BEF = BEF_end; + Sema::ForRangeStatus RangeStatus = + SemaRef.BuildForRangeBeginEndCall(ColonLoc, ColonLoc, EndNameInfo, + EndMemberLookup, CandidateSet, + EndRange, EndExpr); + if (RangeStatus != Sema::FRS_Success) { + if (RangeStatus == Sema::FRS_DiagnosticIssued) + SemaRef.Diag(EndRange->getBeginLoc(), diag::note_in_for_range) + << ColonLoc << BEF_end << EndRange->getType(); + return RangeStatus; + } + if (FinishForRangeVarDecl(SemaRef, EndVar, EndExpr->get(), ColonLoc, + diag::err_for_range_iter_deduction_failure)) { + NoteForRangeBeginEndFunction(SemaRef, EndExpr->get(), *BEF); + return Sema::FRS_DiagnosticIssued; + } + return Sema::FRS_Success; + }; + 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 @@ -2156,68 +2206,62 @@ BuildNonArrayForRange(Sema &SemaRef, Exp // declaration, begin-expr and end-expr are __range.begin() and // __range.end(), respectively; SemaRef.LookupQualifiedName(BeginMemberLookup, D); + if (BeginMemberLookup.isAmbiguous()) + return Sema::FRS_DiagnosticIssued; + SemaRef.LookupQualifiedName(EndMemberLookup, D); + if (EndMemberLookup.isAmbiguous()) + return Sema::FRS_DiagnosticIssued; if (BeginMemberLookup.empty() != EndMemberLookup.empty()) { - SourceLocation RangeLoc = BeginVar->getLocation(); - *BEF = BeginMemberLookup.empty() ? BEF_end : BEF_begin; - - SemaRef.Diag(RangeLoc, diag::err_for_range_member_begin_end_mismatch) - << RangeLoc << BeginRange->getType() << *BEF; - return Sema::FRS_DiagnosticIssued; + // Look up the non-member form of the member we didn't find, first. + // This way we prefer a "no viable 'end'" diagnostic over a "i found + // a 'begin' but ignored it because there was no member 'end'" + // diagnostic. + auto BuildNonmember = [&]( + BeginEndFunction BEFFound, LookupResult &Found, + llvm::function_ref<Sema::ForRangeStatus()> BuildFound, + llvm::function_ref<Sema::ForRangeStatus()> BuildNotFound) { + LookupResult OldFound = std::move(Found); + Found.clear(); + + if (Sema::ForRangeStatus Result = BuildNotFound()) + return Result; + + switch (BuildFound()) { + case Sema::FRS_Success: + return Sema::FRS_Success; + + case Sema::FRS_NoViableFunction: + SemaRef.Diag(BeginRange->getBeginLoc(), diag::err_for_range_invalid) + << BeginRange->getType() << BEFFound; + CandidateSet->NoteCandidates(SemaRef, OCD_AllCandidates, BeginRange); + LLVM_FALLTHROUGH; + + case Sema::FRS_DiagnosticIssued: + for (NamedDecl *D : OldFound) { + SemaRef.Diag(D->getLocation(), + diag::note_for_range_member_begin_end_ignored) + << BeginRange->getType() << BEFFound; + } + return Sema::FRS_DiagnosticIssued; + } + llvm_unreachable("unexpected ForRangeStatus"); + }; + if (BeginMemberLookup.empty()) + return BuildNonmember(BEF_end, EndMemberLookup, BuildEnd, BuildBegin); + return BuildNonmember(BEF_begin, BeginMemberLookup, BuildBegin, BuildEnd); } } 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; - Sema::ForRangeStatus RangeStatus = - SemaRef.BuildForRangeBeginEndCall(ColonLoc, ColonLoc, BeginNameInfo, - BeginMemberLookup, CandidateSet, - BeginRange, BeginExpr); - - if (RangeStatus != Sema::FRS_Success) { - if (RangeStatus == Sema::FRS_DiagnosticIssued) - SemaRef.Diag(BeginRange->getBeginLoc(), diag::note_in_for_range) - << ColonLoc << BEF_begin << BeginRange->getType(); - return RangeStatus; - } - if (!CoawaitLoc.isInvalid()) { - // FIXME: getCurScope() should not be used during template instantiation. - // We should pick up the set of unqualified lookup results for operator - // co_await during the initial parse. - *BeginExpr = SemaRef.ActOnCoawaitExpr(SemaRef.getCurScope(), ColonLoc, - BeginExpr->get()); - if (BeginExpr->isInvalid()) - return Sema::FRS_DiagnosticIssued; - } - if (FinishForRangeVarDecl(SemaRef, BeginVar, BeginExpr->get(), ColonLoc, - diag::err_for_range_iter_deduction_failure)) { - NoteForRangeBeginEndFunction(SemaRef, BeginExpr->get(), *BEF); - return Sema::FRS_DiagnosticIssued; - } - - *BEF = BEF_end; - RangeStatus = - SemaRef.BuildForRangeBeginEndCall(ColonLoc, ColonLoc, EndNameInfo, - EndMemberLookup, CandidateSet, - EndRange, EndExpr); - if (RangeStatus != Sema::FRS_Success) { - if (RangeStatus == Sema::FRS_DiagnosticIssued) - SemaRef.Diag(EndRange->getBeginLoc(), diag::note_in_for_range) - << ColonLoc << BEF_end << EndRange->getType(); - return RangeStatus; - } - if (FinishForRangeVarDecl(SemaRef, EndVar, EndExpr->get(), ColonLoc, - diag::err_for_range_iter_deduction_failure)) { - NoteForRangeBeginEndFunction(SemaRef, EndExpr->get(), *BEF); - return Sema::FRS_DiagnosticIssued; - } - return Sema::FRS_Success; + if (Sema::ForRangeStatus Result = BuildBegin()) + return Result; + return BuildEnd(); } /// Speculatively attempt to dereference an invalid range expression. Modified: cfe/trunk/test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp?rev=342925&r1=342924&r2=342925&view=diff ============================================================================== --- cfe/trunk/test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp (original) +++ cfe/trunk/test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp Mon Sep 24 16:17:44 2018 @@ -150,9 +150,9 @@ void g() { struct NoEnd { null_t begin(); }; - for (auto u : NoBegin()) { // expected-error {{range type 'NoBegin' has 'end' member but no 'begin' member}} + for (auto u : NoBegin()) { // expected-error {{no viable 'begin' function available}} } - for (auto u : NoEnd()) { // expected-error {{range type 'NoEnd' has 'begin' member but no 'end' member}} + for (auto u : NoEnd()) { // expected-error {{no viable 'end' function available}} } struct NoIncr { @@ -271,3 +271,58 @@ namespace rdar13712739 { template void foo(const int&); // expected-note{{in instantiation of function template specialization}} } + +namespace p0962r1 { + namespace NA { + struct A { + void begin(); + }; + int *begin(A); + int *end(A); + } + + namespace NB { + struct B { + void end(); + }; + int *begin(B); + int *end(B); + } + + namespace NC { + struct C { + void begin(); + }; + int *begin(C); + } + + namespace ND { + struct D { + void end(); + }; + int *end(D); + } + + namespace NE { + struct E { + void begin(); // expected-note {{member is not a candidate because range type 'p0962r1::NE::E' has no 'end' member}} + }; + int *end(E); + } + + namespace NF { + struct F { + void end(); // expected-note {{member is not a candidate because range type 'p0962r1::NF::F' has no 'begin' member}} + }; + int *begin(F); + } + + void use(NA::A a, NB::B b, NC::C c, ND::D d, NE::E e, NF::F f) { + for (auto x : a) {} + for (auto x : b) {} + for (auto x : c) {} // expected-error {{no viable 'end' function}} + for (auto x : d) {} // expected-error {{no viable 'begin' function}} + for (auto x : e) {} // expected-error {{no viable 'begin' function}} + for (auto x : f) {} // expected-error {{no viable 'end' function}} + } +} Modified: cfe/trunk/test/SemaCXX/for-range-dereference.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/for-range-dereference.cpp?rev=342925&r1=342924&r2=342925&view=diff ============================================================================== --- cfe/trunk/test/SemaCXX/for-range-dereference.cpp (original) +++ cfe/trunk/test/SemaCXX/for-range-dereference.cpp Mon Sep 24 16:17:44 2018 @@ -17,7 +17,7 @@ struct DeletedEnd : public T { struct DeletedADLBegin { }; int* begin(DeletedADLBegin) = delete; //expected-note {{candidate function has been explicitly deleted}} \ - expected-note 5 {{candidate function not viable: no known conversion}} + expected-note 6 {{candidate function not viable: no known conversion}} struct PrivateEnd { Data *begin(); @@ -27,7 +27,7 @@ struct PrivateEnd { }; struct ADLNoEnd { }; -Data * begin(ADLNoEnd); // expected-note 6 {{candidate function not viable: no known conversion}} +Data * begin(ADLNoEnd); // expected-note 7 {{candidate function not viable: no known conversion}} struct OverloadedStar { T operator*(); @@ -45,7 +45,7 @@ void f() { for (auto i : parr) { }// expected-error{{invalid range expression of type 'int (*)[10]'; did you mean to dereference it with '*'?}} NoBegin NB; - for (auto i : NB) { }// expected-error{{range type 'NoBegin' has 'end' member but no 'begin' member}} + for (auto i : NB) { }// expected-error{{invalid range expression of type 'NoBegin'; no viable 'begin' function available}} NoBegin *pNB; for (auto i : pNB) { }// expected-error{{invalid range expression of type 'NoBegin *'; no viable 'begin' function available}} NoBegin **ppNB; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits