Merge pull request #71739 from tshortli/module-serialization-remarks

Serialization: Skip invalid decls during module serialization

Author: tshortli

include/swift/AST/DiagnosticsSema.def

@@ -808,6 +808,19 @@ WARNING(sema_import_current_module_with_file,none,
 ERROR(sema_opening_import,Fatal,
       "opening import file for module %0: %1", (Identifier, StringRef))
 
+REMARK(serialization_skipped_invalid_decl,none,
+       "serialization skipped invalid %kind0",
+       (const Decl *))
+REMARK(serialization_skipped_invalid_type,none,
+       "serialization skipped for invalid type %0",
+       (TypeRepr *))
+REMARK(serialization_skipped_invalid_type_unknown_name,none,
+       "serialization skipped for invalid type",
+       ())
+ERROR(serialization_failed,none,
+      "serialization of module %0 failed due to the errors above",
+      (const ModuleDecl *))
+
 ERROR(serialization_load_failed,Fatal,
       "failed to load module '%0'", (StringRef))
 ERROR(module_interface_build_failed,Fatal,

include/swift/Basic/LangOptions.h

@@ -243,6 +243,9 @@ namespace swift {
     /// Emit remarks about contextual inconsistencies in loaded modules.
     bool EnableModuleRecoveryRemarks = false;
 
+    /// Emit remarks for unexpected conditions when serializing a module.
+    bool EnableModuleSerializationRemarks = false;
+
     /// Emit remarks about the source of each element exposed by the module API.
     bool EnableModuleApiImportRemarks = false;
 

include/swift/Option/Options.td

@@ -417,6 +417,10 @@ def remark_skip_explicit_interface_build : Flag<["-"], "Rskip-explicit-interface
   Flags<[FrontendOption, DoesNotAffectIncrementalBuild]>,
   HelpText<"Emit a remark if an explicit module interface invocation has an early exit because the expected output is up-to-date">;
 
+def remark_module_serialization : Flag<["-"], "Rmodule-serialization">,
+  Flags<[FrontendOption, DoesNotAffectIncrementalBuild]>,
+  HelpText<"Emit remarks about module serialization">;
+
 def emit_tbd : Flag<["-"], "emit-tbd">,
   HelpText<"Emit a TBD file">,
   Flags<[FrontendOption, NoInteractiveOption, SupplementaryOutput]>;

include/swift/Serialization/SerializationOptions.h

@@ -159,6 +159,7 @@ namespace swift {
     bool IsOSSA = false;
     bool SkipNonExportableDecls = false;
     bool ExplicitModuleBuild = false;
+    bool EnableSerializationRemarks = false;
   };
 
 } // end namespace swift

lib/Frontend/CompilerInvocation.cpp

@@ -1088,6 +1088,8 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
 
   Opts.EnableModuleLoadingRemarks = Args.hasArg(OPT_remark_loading_module);
   Opts.EnableModuleRecoveryRemarks = Args.hasArg(OPT_remark_module_recovery);
+  Opts.EnableModuleSerializationRemarks =
+      Args.hasArg(OPT_remark_module_serialization);
   Opts.EnableModuleApiImportRemarks = Args.hasArg(OPT_remark_module_api_import);
   Opts.EnableMacroLoadingRemarks = Args.hasArg(OPT_remark_macro_loading);
   Opts.EnableIndexingSystemModuleRemarks = Args.hasArg(OPT_remark_indexing_system_module);

lib/Frontend/Frontend.cpp

@@ -253,6 +253,9 @@ SerializationOptions CompilerInvocation::computeSerializationOptions(
 
   serializationOpts.ExplicitModuleBuild = FrontendOpts.DisableImplicitModules;
 
+  serializationOpts.EnableSerializationRemarks =
+      getLangOptions().EnableModuleSerializationRemarks;
+
   return serializationOpts;
 }
 

lib/Serialization/Serialization.cpp

@@ -18,6 +18,7 @@
 #include "swift/AST/ASTVisitor.h"
 #include "swift/AST/AutoDiff.h"
 #include "swift/AST/DiagnosticsCommon.h"
+#include "swift/AST/DiagnosticsSema.h"
 #include "swift/AST/Expr.h"
 #include "swift/AST/FileSystem.h"
 #include "swift/AST/ForeignAsyncConvention.h"
@@ -3045,12 +3046,12 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
       if (D->getResolvedMacro(const_cast<CustomAttr *>(theAttr)))
         return;
 
-      auto typeID = S.addTypeRef(
-          D->getResolvedCustomAttrType(const_cast<CustomAttr *>(theAttr)));
-      if (!typeID && !S.allowCompilerErrors()) {
-        llvm::PrettyStackTraceString message("CustomAttr has no type");
-        abort();
-      }
+      auto attrType =
+          D->getResolvedCustomAttrType(const_cast<CustomAttr *>(theAttr));
+      if (S.skipTypeIfInvalid(attrType, theAttr->getTypeRepr()))
+        return;
+
+      auto typeID = S.addTypeRef(attrType);
       CustomDeclAttrLayout::emitRecord(S.Out, S.ScratchRecord, abbrCode,
                                        theAttr->isImplicit(),
                                        typeID, theAttr->isArgUnsafe());
@@ -3656,27 +3657,37 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
     }
     case PatternKind::Named: {
       auto named = cast<NamedPattern>(pattern);
+      auto ty = getPatternType();
+      if (S.skipTypeIfInvalid(ty, named->getLoc()))
+        break;
 
       unsigned abbrCode = S.DeclTypeAbbrCodes[NamedPatternLayout::Code];
       NamedPatternLayout::emitRecord(S.Out, S.ScratchRecord, abbrCode,
                                      S.addDeclRef(named->getDecl()),
-                                     S.addTypeRef(getPatternType()));
+                                     S.addTypeRef(ty));
       break;
     }
     case PatternKind::Any: {
+      auto ty = getPatternType();
+      if (S.skipTypeIfInvalid(ty, pattern->getLoc()))
+        break;
+
       unsigned abbrCode = S.DeclTypeAbbrCodes[AnyPatternLayout::Code];
       auto anyPattern = cast<AnyPattern>(pattern);
       AnyPatternLayout::emitRecord(S.Out, S.ScratchRecord, abbrCode,
-                                   S.addTypeRef(getPatternType()),
+                                   S.addTypeRef(ty),
                                    anyPattern->isAsyncLet());
       break;
     }
     case PatternKind::Typed: {
       auto typed = cast<TypedPattern>(pattern);
+      auto ty = getPatternType();
+      if (S.skipTypeIfInvalid(ty, typed->getTypeRepr()))
+        break;
 
       unsigned abbrCode = S.DeclTypeAbbrCodes[TypedPatternLayout::Code];
       TypedPatternLayout::emitRecord(S.Out, S.ScratchRecord, abbrCode,
-                                     S.addTypeRef(getPatternType()));
+                                     S.addTypeRef(ty));
       writePattern(typed->getSubPattern());
       break;
     }
@@ -4996,13 +5007,8 @@ void Serializer::writeASTBlockEntity(const Decl *D) {
   PrettyStackTraceDecl trace("serializing", D);
   assert(DeclsToSerialize.hasRef(D));
 
-  if (D->isInvalid()) {
-    assert(allowCompilerErrors() &&
-           "cannot create a module with an invalid decl");
-
-    if (canSkipWhenInvalid(D))
-      return;
-  }
+  if (skipDeclIfInvalid(D))
+    return;
 
   BitOffset initialOffset = Out.GetCurrentBitNo();
   SWIFT_DEFER {
@@ -6780,6 +6786,11 @@ void Serializer::writeToStream(
     S.writeInputBlock();
     S.writeSIL(SILMod, options.SerializeAllSIL);
     S.writeAST(DC);
+
+    if (S.hadError)
+      S.getASTContext().Diags.diagnose(SourceLoc(), diag::serialization_failed,
+                                       S.M);
+
     if (!options.DisableCrossModuleIncrementalInfo && DepGraph) {
       fine_grained_dependencies::writeFineGrainedDependencyGraph(
           S.Out, *DepGraph, fine_grained_dependencies::Purpose::ForSwiftModule);
@@ -6793,6 +6804,48 @@ bool Serializer::allowCompilerErrors() const {
   return getASTContext().LangOpts.AllowModuleWithCompilerErrors;
 }
 
+bool Serializer::skipDeclIfInvalid(const Decl *decl) {
+  if (!decl->isInvalid())
+    return false;
+
+  if (allowCompilerErrors())
+    return canSkipWhenInvalid(decl);
+
+  if (Options.EnableSerializationRemarks) {
+    getASTContext().Diags.diagnose(
+        decl->getLoc(), diag::serialization_skipped_invalid_decl, decl);
+  }
+
+  hadError = true;
+  return true;
+}
+
+bool Serializer::skipTypeIfInvalid(Type ty, TypeRepr *tyRepr) {
+  if ((ty && !ty->hasError()) || allowCompilerErrors())
+    return false;
+
+  if (Options.EnableSerializationRemarks) {
+    getASTContext().Diags.diagnose(
+        tyRepr->getLoc(), diag::serialization_skipped_invalid_type, tyRepr);
+  }
+
+  hadError = true;
+  return true;
+}
+
+bool Serializer::skipTypeIfInvalid(Type ty, SourceLoc loc) {
+  if ((ty && !ty->hasError()) || allowCompilerErrors())
+    return false;
+
+  if (Options.EnableSerializationRemarks) {
+    getASTContext().Diags.diagnose(
+        loc, diag::serialization_skipped_invalid_type_unknown_name);
+  }
+
+  hadError = true;
+  return true;
+}
+
 void serialization::writeToStream(
     raw_ostream &os, ModuleOrSourceFile DC, const SILModule *M,
     const SerializationOptions &options,

lib/Serialization/Serialization.h

@@ -112,6 +112,10 @@ class Serializer : public SerializerBase {
 
   SmallVector<DeclID, 16> exportedPrespecializationDecls;
 
+  /// Will be set to true if any serialization step failed, for example due to
+  /// an error in the AST.
+  bool hadError = false;
+
   /// Helper for serializing entities in the AST block object graph.
   ///
   /// Keeps track of assigning IDs to newly-seen entities, and collecting
@@ -570,6 +574,19 @@ class Serializer : public SerializerBase {
   void writePrimaryAssociatedTypes(ArrayRef<AssociatedTypeDecl *> assocTypes);
 
   bool allowCompilerErrors() const;
+
+private:
+  /// If the declaration is invalid, records that an error occurred and returns
+  /// true if the decl should be skipped.
+  bool skipDeclIfInvalid(const Decl *decl);
+
+  /// If the type is invalid, records that an error occurred and returns
+  /// true if the type should be skipped.
+  bool skipTypeIfInvalid(Type ty, TypeRepr *tyRepr);
+
+  /// If the type is invalid, records that an error occurred and returns
+  /// true if the type should be skipped.
+  bool skipTypeIfInvalid(Type ty, SourceLoc loc);
 };
 
 } // end namespace serialization

test/Serialization/lazy-typecheck-errors.swift

@@ -0,0 +1,34 @@
+// Verify the -emit-module job fails with a broken AST.
+// RUN: not %target-swift-frontend -emit-module %s -emit-module-path /dev/null -module-name lazy_typecheck -swift-version 5 -enable-library-evolution -parse-as-library -experimental-lazy-typecheck -experimental-skip-all-function-bodies -experimental-skip-non-exportable-decls 2>&1 | %FileCheck %s
+
+// CHECK: <unknown>:0: error: serialization of module 'lazy_typecheck' failed due to the errors above
+
+// Verify typechecking errors are emitted.
+// RUN: %target-swift-frontend -emit-module %s -emit-module-path /dev/null -module-name lazy_typecheck -swift-version 5 -enable-library-evolution -parse-as-library -experimental-lazy-typecheck -experimental-skip-all-function-bodies -experimental-skip-non-exportable-decls -verify -verify-ignore-unknown
+
+// Verify output with -Rmodule-serialization.
+// RUN: %target-swift-frontend -emit-module %s -emit-module-path /dev/null -module-name lazy_typecheck -swift-version 5 -enable-library-evolution -parse-as-library -experimental-lazy-typecheck -experimental-skip-all-function-bodies -experimental-skip-non-exportable-decls -verify -verify-ignore-unknown -Rmodule-serialization -verify-additional-prefix serialization-
+
+public func returnsInvalidType() -> InvalidType { fatalError() }
+// expected-error@-1 {{cannot find type 'InvalidType' in scope}}
+// expected-serialization-remark@-2 {{serialization skipped invalid global function 'returnsInvalidType()'}}
+
+public func takesInvalidType(_ x: InvalidType) {}
+// expected-error@-1 {{cannot find type 'InvalidType' in scope}}
+// expected-serialization-remark@-2 {{serialization skipped invalid global function 'takesInvalidType'}}
+
+@InvalidCustomAttr public struct HasInvalidCustomAttr {}
+// FIXME: An error should be emitted for the invalid attribute
+// expected-serialization-remark@-2 {{serialization skipped for invalid type 'InvalidCustomAttr'}}
+
+public var varWithExplicitInvalidType: InvalidType
+// expected-error@-1 {{cannot find type 'InvalidType' in scope}}
+// expected-serialization-remark@-2 {{serialization skipped invalid var 'varWithExplicitInvalidType'}}
+// expected-serialization-remark@-3 {{serialization skipped for invalid type 'InvalidType'}}
+
+public var varWithImplicitInvalidType = (1 as InvalidType)
+// expected-error@-1 {{cannot find type 'InvalidType' in scope}}
+// expected-serialization-remark@-2 {{serialization skipped invalid var 'varWithImplicitInvalidType'}}
+// expected-serialization-remark@-3 {{serialization skipped for invalid type}}
+
+public var _: InvalidType