Serialization: Skip invalid decls during module serialization.

When `-enable-lazy-typecheck` is specified, serialization may be expected to
run on an AST containing invalid declarations since type checking may happen
on-demand, during serialization, in this mode. If the declarations that are
invalid are not skipped, then the compiler is likely to crash when attempting
to serialize them. Now, invalid declarations are skipped and an error is
emitted at the end of serialization to note that serialization failed.

Additionally, a new `-Rmodule-serialization` flag can be specified to request
more detailed information about module serialization failures. This would be
useful in a situation where lazy typechecking does not produce any diagnostic
for some reason, but module serialization fails and more information is
therefore required to debug.

Resolves rdar://123260476

Author: tshortli

include/swift/AST/DiagnosticsSema.def

@@ -808,6 +808,13 @@ 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 *))
+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"
@@ -4998,13 +4999,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 {
@@ -6782,6 +6778,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);
@@ -6795,6 +6796,22 @@ 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;
+}
+
 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,11 @@ 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);
 };
 
 } // end namespace serialization

test/Serialization/lazy-typecheck-errors.swift

@@ -0,0 +1,18 @@
+// 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'}}