Ignore bad @_hasStorage in module interfaces

An objcImpl bug previously caused `@_hasStorage` to be emitted inside some extensions in module interfaces. An earlier commit in this PR created an error for this, but for backwards compatibility, it would actually be better to simply ignore the attribute in module interfaces. Modify TypeCheckStorage to emit a warning, not an error, in this situation.

Additionally, modify the module interface loader to show warnings when you verify a module interface, but not for other module interface uses (like compiling or importing one). The assumption here is that if you’re verifying a module interface, you’re either the author of the module that created it or you’re investigating a problem with it, and in either case you’d like to be told about minor defects in case they’re related.

Fixes rdar://144811653 thoroughly.

Author: beccadax

include/swift/Frontend/ModuleInterfaceLoader.h

@@ -672,7 +672,8 @@ struct InterfaceSubContextDelegateImpl : InterfaceSubContextDelegate {
     return InterfaceSubContextDelegateImpl::diagnose(interfacePath, diagnosticLoc, SM, Diags, ID, std::move(Args)...);
   }
   void
-  inheritOptionsForBuildingInterface(const SearchPathOptions &SearchPathOpts,
+  inheritOptionsForBuildingInterface(FrontendOptions::ActionType requestedAction,
+                                     const SearchPathOptions &SearchPathOpts,
                                      const LangOptions &LangOpts,
                                      const ClangImporterOptions &clangImporterOpts,
                                      const CASOptions &casOpts,

lib/Frontend/ModuleInterfaceLoader.cpp

@@ -1654,6 +1654,7 @@ void ModuleInterfaceLoader::collectVisibleTopLevelModuleNames(
 }
 
 void InterfaceSubContextDelegateImpl::inheritOptionsForBuildingInterface(
+    FrontendOptions::ActionType requestedAction,
     const SearchPathOptions &SearchPathOpts, const LangOptions &LangOpts,
     const ClangImporterOptions &clangImporterOpts, const CASOptions &casOpts,
     bool suppressRemarks, RequireOSSAModules_t RequireOSSAModules) {
@@ -1741,9 +1742,12 @@ void InterfaceSubContextDelegateImpl::inheritOptionsForBuildingInterface(
   }
 
   // Inhibit warnings from the genericSubInvocation since we are assuming the user
-  // is not in a position to address them.
-  genericSubInvocation.getDiagnosticOptions().SuppressWarnings = true;
-  GenericArgs.push_back("-suppress-warnings");
+  // is not in a position to address them. (Unless we're verifying, in which
+  // case they might.)
+  if (requestedAction != FrontendOptions::ActionType::Typecheck) {
+    genericSubInvocation.getDiagnosticOptions().SuppressWarnings = true;
+    GenericArgs.push_back("-suppress-warnings");
+  }
   // Inherit the parent invocation's setting on whether to suppress remarks
   if (suppressRemarks) {
     genericSubInvocation.getDiagnosticOptions().SuppressRemarks = true;
@@ -1871,8 +1875,8 @@ InterfaceSubContextDelegateImpl::InterfaceSubContextDelegateImpl(
     RequireOSSAModules_t requireOSSAModules)
     : SM(SM), Diags(Diags), ArgSaver(Allocator) {
   genericSubInvocation.setMainExecutablePath(LoaderOpts.mainExecutablePath);
-  inheritOptionsForBuildingInterface(searchPathOpts, langOpts,
-                                     clangImporterOpts, casOpts,
+  inheritOptionsForBuildingInterface(LoaderOpts.requestedAction, searchPathOpts,
+                                     langOpts, clangImporterOpts, casOpts,
                                      Diags->getSuppressRemarks(),
                                      requireOSSAModules);
   // Configure front-end input.

lib/Sema/TypeCheckStorage.cpp

@@ -3748,8 +3748,12 @@ bool HasStorageRequest::evaluate(Evaluator &evaluator,
     return false;
 
   // @_hasStorage implies that it... has storage.
-  if (var->getAttrs().hasAttribute<HasStorageAttr>())
-    return true;
+  if (var->getAttrs().hasAttribute<HasStorageAttr>()) {
+    // Except in contexts where it would be invalid. This is diagnosed in
+    // StorageImplInfoRequest.
+    return !isa<ProtocolDecl, ExtensionDecl, EnumDecl>(
+               storage->getDeclContext()->getImplementedObjCContext());
+  }
 
   // Protocol requirements never have storage.
   if (isa<ProtocolDecl>(storage->getDeclContext()))
@@ -3852,7 +3856,8 @@ StorageImplInfoRequest::evaluate(Evaluator &evaluator,
     // and ignore it.
     if (isa<ExtensionDecl, EnumDecl, ProtocolDecl>(DC)) {
       storage->diagnose(diag::attr_invalid_in_context, attr,
-                        DC->getAsDecl()->getDescriptiveKind());
+                        DC->getAsDecl()->getDescriptiveKind())
+        .warnInSwiftInterface(storage->getDeclContext());
 
     // Allow the @_hasStorage attribute to override all the accessors we parsed
     // when making the final classification.

test/ModuleInterface/has_storage_attr_bad.swiftinterface

@@ -1,10 +1,9 @@
 // swift-interface-format-version: 1.0
 // swift-module-flags:
-// expected-error@-2 {{failed to verify module interface of 'Test' due to the errors above}}
 
 // RUN: %target-swift-typecheck-module-from-interface(%s) -module-name Test -verify -verify-ignore-unknown
 // REQUIRES: OS=macosx
 
 extension Array {
-  @_hasStorage public var foo: Int { get set } // expected-error {{'@_hasStorage' attribute cannot be applied to declaration in extension}}
+  @_hasStorage public var foo: Int { get set } // expected-warning {{'@_hasStorage' attribute cannot be applied to declaration in extension}}
 }