r342925 - P0962R1: only use the member form of 'begin' and 'end' in a range-based

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 24 16:17:44 PDT 2018


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;




More information about the cfe-commits mailing list