Merge pull request #18999 from hamishknight/location-by-association

 [GSB] Avoid emitting associated type diagnostics on the protocol decl

Author: slavapestov

include/swift/AST/Decl.h

@@ -1151,14 +1151,6 @@ class RequirementRepr {
     return SecondLayout;
   }
 
-  /// \brief Retrieve the location of the ':' in an explicitly-written
-  /// conformance requirement.
-  SourceLoc getColonLoc() const {
-    assert(getKind() == RequirementReprKind::TypeConstraint ||
-           getKind() == RequirementReprKind::LayoutConstraint);
-    return SeparatorLoc;
-  }
-
   /// \brief Retrieve the first type of a same-type requirement.
   Type getFirstType() const {
     assert(getKind() == RequirementReprKind::SameType);
@@ -1201,10 +1193,9 @@ class RequirementRepr {
     return SecondType;
   }
 
-  /// \brief Retrieve the location of the '==' in an explicitly-written
-  /// same-type requirement.
-  SourceLoc getEqualLoc() const {
-    assert(getKind() == RequirementReprKind::SameType);
+  /// \brief Retrieve the location of the ':' or '==' in an explicitly-written
+  /// conformance or same-type requirement respectively.
+  SourceLoc getSeparatorLoc() const {
     return SeparatorLoc;
   }
 

include/swift/AST/DiagnosticsSema.def

@@ -1911,11 +1911,11 @@ ERROR(protocol_composition_one_class,none,
 
 ERROR(requires_conformance_nonprotocol,none,
       "type %0 constrained to non-protocol, non-class type %1",
-      (TypeLoc, TypeLoc))
+      (Type, Type))
 ERROR(requires_not_suitable_archetype,none,
       "type %0 in conformance requirement does not refer to a "
       "generic parameter or associated type",
-      (TypeLoc))
+      (Type))
 WARNING(requires_no_same_type_archetype,none,
         "neither type in same-type constraint (%0 or %1) refers to a "
         "generic parameter or associated type",

lib/AST/Decl.cpp

@@ -621,7 +621,7 @@ GenericParamList::clone(DeclContext *dc) const {
       auto second = reqt.getConstraintLoc();
       reqt = RequirementRepr::getTypeConstraint(
           first.clone(ctx),
-          reqt.getColonLoc(),
+          reqt.getSeparatorLoc(),
           second.clone(ctx));
       break;
     }
@@ -630,7 +630,7 @@ GenericParamList::clone(DeclContext *dc) const {
       auto second = reqt.getSecondTypeLoc();
       reqt = RequirementRepr::getSameType(
           first.clone(ctx),
-          reqt.getEqualLoc(),
+          reqt.getSeparatorLoc(),
           second.clone(ctx));
       break;
     }
@@ -639,7 +639,7 @@ GenericParamList::clone(DeclContext *dc) const {
       auto layout = reqt.getLayoutConstraintLoc();
       reqt = RequirementRepr::getLayoutConstraint(
           first.clone(ctx),
-          reqt.getColonLoc(),
+          reqt.getSeparatorLoc(),
           layout);
       break;
     }

lib/AST/GenericSignatureBuilder.cpp

@@ -1454,16 +1454,9 @@ SourceLoc RequirementSource::getLoc() const {
   if (auto typeRepr = getTypeRepr())
     return typeRepr->getStartLoc();
 
-  if (auto requirementRepr = getRequirementRepr()) {
-    switch (requirementRepr->getKind()) {
-    case RequirementReprKind::LayoutConstraint:
-    case RequirementReprKind::TypeConstraint:
-      return requirementRepr->getColonLoc();
+  if (auto requirementRepr = getRequirementRepr())
+    return requirementRepr->getSeparatorLoc();
 
-    case RequirementReprKind::SameType:
-      return requirementRepr->getEqualLoc();
-    }
-  }
   if (parent)
     return parent->getLoc();
 
@@ -1681,22 +1674,24 @@ const RequirementSource *FloatingRequirementSource::getSource(
 }
 
 SourceLoc FloatingRequirementSource::getLoc() const {
+  // For an explicit abstract protocol source, we can get a more accurate source
+  // location from the written protocol requirement.
+  if (kind == Kind::AbstractProtocol && isExplicit()) {
+    auto written = protocolReq.written;
+    if (auto typeRepr = written.dyn_cast<const TypeRepr *>())
+      return typeRepr->getLoc();
+    if (auto requirementRepr = written.dyn_cast<const RequirementRepr *>())
+      return requirementRepr->getSeparatorLoc();
+  }
+
   if (auto source = storage.dyn_cast<const RequirementSource *>())
     return source->getLoc();
 
   if (auto typeRepr = storage.dyn_cast<const TypeRepr *>())
     return typeRepr->getLoc();
 
-  if (auto requirementRepr = storage.dyn_cast<const RequirementRepr *>()) {
-    switch (requirementRepr->getKind()) {
-    case RequirementReprKind::LayoutConstraint:
-    case RequirementReprKind::TypeConstraint:
-      return requirementRepr->getColonLoc();
-
-    case RequirementReprKind::SameType:
-      return requirementRepr->getEqualLoc();
-    }
-  }
+  if (auto requirementRepr = storage.dyn_cast<const RequirementRepr *>())
+    return requirementRepr->getSeparatorLoc();
 
   return SourceLoc();
 }
@@ -4586,7 +4581,7 @@ ConstraintResult GenericSignatureBuilder::addLayoutRequirement(
       Impl->HadAnyError = true;
 
       Diags.diagnose(source.getLoc(), diag::requires_not_suitable_archetype,
-                     TypeLoc::withoutLoc(concreteType));
+                     concreteType);
       return ConstraintResult::Concrete;
     }
 
@@ -4719,8 +4714,7 @@ ConstraintResult GenericSignatureBuilder::addTypeRequirement(
 
       Impl->HadAnyError = true;
       Diags.diagnose(source.getLoc(), diag::requires_conformance_nonprotocol,
-                     TypeLoc::withoutLoc(subjectType),
-                     TypeLoc::withoutLoc(constraintType));
+                     subjectType, constraintType);
     }
 
     return ConstraintResult::Conflicting;
@@ -4768,7 +4762,7 @@ ConstraintResult GenericSignatureBuilder::addTypeRequirement(
       if (source.getLoc().isValid()) {
         Impl->HadAnyError = true;
         Diags.diagnose(source.getLoc(), diag::requires_not_suitable_archetype,
-                       TypeLoc::withoutLoc(subjectType));
+                       subjectType);
       }
 
       return ConstraintResult::Concrete;
@@ -5356,7 +5350,7 @@ GenericSignatureBuilder::addRequirement(const Requirement &req,
         !req.getSecondType()->hasTypeParameter() &&
         !req.getFirstType()->hasError() &&
         !req.getSecondType()->hasError()) {
-      Diags.diagnose(reqRepr->getEqualLoc(),
+      Diags.diagnose(reqRepr->getSeparatorLoc(),
                      diag::requires_no_same_type_archetype,
                      req.getFirstType(), req.getSecondType())
         .highlight(reqRepr->getFirstTypeLoc().getSourceRange())

lib/Sema/TypeCheckProtocol.cpp

@@ -2719,11 +2719,11 @@ void ConformanceChecker::checkNonFinalClassWitness(ValueDecl *requirement,
       emitDeclaredHereIfNeeded(diags, diagLoc, witness);
 
       if (auto requirementRepr = *constraint) {
-        diags.diagnose(requirementRepr->getEqualLoc(),
+        diags.diagnose(requirementRepr->getSeparatorLoc(),
                        diag::witness_self_weaken_same_type,
                        requirementRepr->getFirstType(),
                        requirementRepr->getSecondType())
-          .fixItReplace(requirementRepr->getEqualLoc(), ":");
+          .fixItReplace(requirementRepr->getSeparatorLoc(), ":");
       }
     }
   }

test/NameBinding/scope_map_lookup.swift

@@ -23,9 +23,10 @@ protocol P1 {
 }
 
 // Protocols involving associated types.
-protocol AProtocol { // expected-error{{type 'Self.e' constrained to non-protocol, non-class type 'Self.e'}}
+protocol AProtocol {
   associatedtype e : e
   // expected-error@-1 {{inheritance from non-protocol, non-class type 'Self.e'}}
+  // expected-error@-2 {{type 'Self.e' constrained to non-protocol, non-class type 'Self.e'}}
 }
 
 // Extensions.

test/Sema/circular_decl_checking.swift

@@ -43,10 +43,11 @@ func TopLevelGenericFunc2<T : TopLevelGenericFunc2>(x: T) -> T { return x} // ex
 var TopLevelVar: TopLevelVar? { return nil } // expected-error {{use of undeclared type 'TopLevelVar'}}
 
 
-// FIXME: The first error is redundant, isn't correct in what it states, and
-// also should be emitted on the inheritance clause.
-protocol AProtocol { // expected-error {{type 'Self.e' constrained to non-protocol, non-class type 'Self.e'}}
-  associatedtype e : e // expected-error {{inheritance from non-protocol, non-class type 'Self.e'}}
+// FIXME: The first error is redundant and isn't correct in what it states.
+protocol AProtocol {
+  associatedtype e : e
+  // expected-error@-1 {{type 'Self.e' constrained to non-protocol, non-class type 'Self.e'}}
+  // expected-error@-2 {{inheritance from non-protocol, non-class type 'Self.e'}}
 }
 
 

test/decl/protocol/req/unsatisfiable.swift

@@ -37,16 +37,30 @@ protocol Base {
   associatedtype Assoc
 }
 
-// FIXME: The first error is redundant, isn't correct in what it states, and
-// also should be emitted on the inheritance clause.
+// FIXME: The first error is redundant and isn't correct in what it states.
 // FIXME: This used to /not/ error in Swift 3. It didn't impose any statically-
 // enforced requirements, but the compiler crashed if you used anything but the
 // same type.
-protocol Sub1: Base { // expected-error {{type 'Self.SubAssoc' constrained to non-protocol, non-class type 'Self.Assoc'}}
-  associatedtype SubAssoc: Assoc // expected-error {{inheritance from non-protocol, non-class type 'Self.Assoc'}}
+protocol Sub1: Base {
+  associatedtype SubAssoc: Assoc
+  // expected-error@-1 {{type 'Self.SubAssoc' constrained to non-protocol, non-class type 'Self.Assoc'}}
+  // expected-error@-2 {{inheritance from non-protocol, non-class type 'Self.Assoc'}}
 }
-// FIXME: This error is incorrect in what it states and should be emitted on
-// the where-clause.
-protocol Sub2: Base { // expected-error {{type 'Self.SubAssoc' constrained to non-protocol, non-class type 'Self.Assoc'}}
-  associatedtype SubAssoc where SubAssoc: Assoc
+
+// FIXME: This error is incorrect in what it states.
+protocol Sub2: Base {
+  associatedtype SubAssoc where SubAssoc: Assoc // expected-error {{type 'Self.SubAssoc' constrained to non-protocol, non-class type 'Self.Assoc'}}
+}
+
+struct S {}
+
+// FIX-ME: One of these errors is redundant.
+protocol P4 {
+  associatedtype X : S
+  // expected-error@-1 {{type 'Self.X' constrained to non-protocol, non-class type 'S'}}
+  // expected-error@-2 {{inheritance from non-protocol, non-class type 'S'}}
+}
+
+protocol P5 {
+  associatedtype Y where Y : S // expected-error {{type 'Self.Y' constrained to non-protocol, non-class type 'S'}}
 }