AST: Record a HadError bit in DelayedConformanceDiags

Author: slavapestov

lib/AST/ASTContext.cpp

@@ -192,6 +192,28 @@ template <> struct DenseMapInfo<OverrideSignatureKey> {
 };
 } // namespace llvm
 
+namespace {
+
+/// If the conformance is in a primary file, we might diagnose some failures
+/// early via request evaluation, with all remaining failures diagnosed when
+/// we completely force the conformance from typeCheckDecl(). To emit the
+/// diagnostics together, we batch them up in the Diags vector.
+///
+/// If the conformance is in a secondary file, we instead just diagnose a
+/// generic "T does not conform to P" error the first time we hit an error
+/// via request evaluation. The detailed delayed conformance diagnostics
+/// are discarded, since we'll emit them again when we compile the file as
+/// a primary file.
+struct DelayedConformanceDiags {
+  /// We set this if we've ever seen an error diagnostic here.
+  bool HadError = false;
+
+  /// The delayed conformance diagnostics that have not been emitted yet.
+  /// Never actually emitted for a secondary file.
+  std::vector<ASTContext::DelayedConformanceDiag> Diags;
+};
+
+}
 struct ASTContext::Implementation {
   Implementation();
   ~Implementation();
@@ -354,8 +376,7 @@ struct ASTContext::Implementation {
 
   /// Map from normal protocol conformances to diagnostics that have
   /// been delayed until the conformance is fully checked.
-  llvm::DenseMap<NormalProtocolConformance *,
-                 std::vector<ASTContext::DelayedConformanceDiag>>
+  llvm::DenseMap<NormalProtocolConformance *, ::DelayedConformanceDiags>
     DelayedConformanceDiags;
 
   /// Map from normal protocol conformances to missing witnesses that have
@@ -2733,25 +2754,18 @@ LazyIterableDeclContextData *ASTContext::getOrCreateLazyIterableContextData(
 bool ASTContext::hasDelayedConformanceErrors(
                           NormalProtocolConformance const* conformance) const {
 
-  auto hasDelayedErrors = [](std::vector<DelayedConformanceDiag> const& diags) {
-    return std::any_of(diags.begin(), diags.end(),
-                    [](ASTContext::DelayedConformanceDiag const& diag) {
-                      return diag.IsError;
-                    });
-  };
-
   if (conformance) {
     auto entry = getImpl().DelayedConformanceDiags.find(conformance);
     if (entry != getImpl().DelayedConformanceDiags.end())
-      return hasDelayedErrors(entry->second);
+      return entry->second.HadError;
 
     return false; // unknown conformance, so no delayed diags either.
   }
 
   // check all conformances for any delayed errors
   for (const auto &entry : getImpl().DelayedConformanceDiags) {
     auto const& diagnostics = entry.getSecond();
-    if (hasDelayedErrors(diagnostics))
+    if (diagnostics.HadError)
       return true;
   }
 
@@ -2763,7 +2777,44 @@ MissingWitnessesBase::~MissingWitnessesBase() { }
 void ASTContext::addDelayedConformanceDiag(
        NormalProtocolConformance *conformance,
        DelayedConformanceDiag fn) {
-  getImpl().DelayedConformanceDiags[conformance].push_back(std::move(fn));
+  auto &diagnostics = getImpl().DelayedConformanceDiags[conformance];
+
+  if (fn.IsError && !diagnostics.HadError) {
+    diagnostics.HadError = true;
+
+    auto *proto = conformance->getProtocol();
+    auto *dc = conformance->getDeclContext();
+    auto *sf = dc->getParentSourceFile();
+    auto *mod = sf->getParentModule();
+    assert(mod->isMainModule());
+
+    // If we have at least one primary file and the conformance is declared in a
+    // non-primary file, emit a fallback diagnostic.
+    if ((!sf->isPrimary() && !mod->getPrimarySourceFiles().empty()) ||
+        TypeCheckerOpts.EnableLazyTypecheck) {
+      auto complainLoc = evaluator.getInnermostSourceLoc([&](SourceLoc loc) {
+        if (loc.isInvalid())
+          return false;
+
+        auto *otherSF = mod->getSourceFileContainingLocation(loc);
+        if (otherSF == nullptr)
+          return false;
+
+        return otherSF->isPrimary();
+      });
+
+      if (complainLoc.isInvalid()) {
+        complainLoc = conformance->getLoc();
+      }
+
+      Diags.diagnose(complainLoc,
+                     diag::type_does_not_conform,
+                     dc->getSelfInterfaceType(),
+                     proto->getDeclaredInterfaceType());
+    }
+  }
+
+  diagnostics.Diags.push_back(std::move(fn));
 }
 
 void ASTContext::addDelayedMissingWitnesses(
@@ -2789,8 +2840,7 @@ ASTContext::takeDelayedConformanceDiags(NormalProtocolConformance const* cnfrm){
   std::vector<ASTContext::DelayedConformanceDiag> result;
   auto known = getImpl().DelayedConformanceDiags.find(cnfrm);
   if (known != getImpl().DelayedConformanceDiags.end()) {
-    result = std::move(known->second);
-    getImpl().DelayedConformanceDiags.erase(known);
+    std::swap(result, known->second.Diags);
   }
   return result;
 }

lib/Sema/TypeCheckProtocol.cpp

@@ -2916,12 +2916,7 @@ ConformanceChecker::ConformanceChecker(
       LocalMissingWitnessesStartIndex(GlobalMissingWitnesses.size()),
       SuppressDiagnostics(suppressDiagnostics) {}
 
-ConformanceChecker::~ConformanceChecker() {
-  // its not OK to forget about error diagnostics, unless if we have already
-  // complained or are suppose to suppress diagnostics.
-  assert(AlreadyComplained || SuppressDiagnostics ||
-            !getASTContext().hasDelayedConformanceErrors(Conformance));
-}
+ConformanceChecker::~ConformanceChecker() {}
 
 
 TinyPtrVector<AssociatedTypeDecl *>

test/SILGen/Inputs/dynamic_other.swift

@@ -15,8 +15,8 @@ class FromOtherFile: Proto {
   @objc init(objc: Int) {}
   @objc func objcMethod() {}
   @objc var objcProp: Int = 0
-  @objc subscript(objc objc: Int) -> Int {
-    get { return objc }
+  @objc subscript(objc objc: AnyObject) -> Int {
+    get { return 0 }
     set {}
   }
 
@@ -29,6 +29,11 @@ class FromOtherFile: Proto {
     set {}
   }
 
+  static subscript(nativeType nativeType: Int) -> Int {
+    get { return nativeType }
+    set {}
+  }
+
   func overriddenByDynamic() {}
 
   @NSManaged var managedProp: Int

test/SILGen/dynamic.swift

@@ -409,9 +409,9 @@ func objcMethodDispatchFromOtherFile() {
   // CHECK: class_method {{%.*}} : $FromOtherFile, #FromOtherFile.objcProp!setter :
   c.objcProp = x
   // CHECK: class_method {{%.*}} : $FromOtherFile, #FromOtherFile.subscript!getter :
-  let y = c[objc: 0]
+  let y = c[objc: 0 as AnyObject]
   // CHECK: class_method {{%.*}} : $FromOtherFile, #FromOtherFile.subscript!setter :
-  c[objc: 0] = y
+  c[objc: 0 as AnyObject] = y
 }
 
 // CHECK-LABEL: sil hidden [ossa] @$s7dynamic0A27MethodDispatchFromOtherFileyyF : $@convention(thin) () -> ()

test/TBD/lazy-typecheck-bad-conformance.swift

@@ -1,10 +1,10 @@
 // RUN: %empty-directory(%t)
-// RUN: %target-swift-frontend -typecheck -experimental-lazy-typecheck -emit-tbd -emit-tbd-path %t/lazy.tbd %s -enable-library-evolution -parse-as-library -tbd-install_name lazy
+// RUN: %target-swift-frontend -typecheck -verify -experimental-lazy-typecheck -emit-tbd -emit-tbd-path %t/lazy.tbd %s -enable-library-evolution -parse-as-library -tbd-install_name lazy
 
 public protocol P {
   func req()
 }
 
-// FIXME: This malformed conformance should probably be diagnosed.
+// expected-error@+1 {{type 'S' does not conform to protocol 'P'}}
 public struct S: P {
 }

test/multifile/protocol-conformance-broken-2.swift

@@ -0,0 +1,25 @@
+// RUN: %empty-directory(%t)
+// RUN: split-file %s %t
+// RUN: %target-swift-frontend -typecheck -verify -primary-file %t/Usage.swift %t/Conformance.swift
+
+//--- Conformance.swift
+protocol P {
+  associatedtype A: Equatable
+  func f() -> A
+}
+
+extension P {
+  func f() -> A { fatalError() }
+}
+
+struct S: P {}
+
+//--- Usage.swift
+func callee(_: some Equatable) {}
+
+// FIXME: This source location will become more accurate once TypeCheckExpr
+// is a request! For now, pointing at the function is still better than crashing.
+
+func caller() { // expected-error {{type 'S' does not conform to protocol 'P'}}
+  callee(S().f())
+}

test/multifile/protocol-conformance-issue-53408.swift

@@ -5,3 +5,4 @@
 
 func reproducer() -> Float { return Struct().func1(1.0) }
 // expected-error@-1 {{cannot convert value of type 'Double' to expected argument type 'Struct.Input'}}
+// expected-error@-2 {{type 'Struct' does not conform to protocol 'Proto1'}}

validation-test/compiler_crashers_2_fixed/0127-issue-48118.swift

@@ -1,5 +1,4 @@
-// RUN: not %target-swift-frontend %s -typecheck
-// REQUIRES: asserts
+// RUN: %target-typecheck-verify-swift
 
 // https://github.com/apple/swift/issues/48118
 
@@ -18,7 +17,7 @@ public protocol P {
     func f4(_ x: Context)
 }
 
-public extension P {
+extension P {
     public func f1(_ x: Context, _ y: PA) {
     }
     public func f2(_ x: Context, _ y: PB) {
@@ -35,6 +34,6 @@ public struct S: P {
 
     public func f1(_ x: Context, _ y: PA) {
     }
-    public func f2(_ x: Context, _ y: PB) {
+    public func f2(_ x: Context, _ y: PB) { // expected-error {{reference to invalid type alias 'Context' of type 'S'}}
     }
 }