Merge pull request #61026 from zoecarver/fix-invalid-vars

[cxx-interop] Lazily instanciate var types and compute their type.

Author: zoecarver

include/swift/AST/ClangModuleLoader.h

@@ -33,6 +33,7 @@ namespace swift {
 
 class ConcreteDeclRef;
 class Decl;
+class VarDecl;
 class DeclContext;
 class EffectiveClangContext;
 class SwiftLookupTable;
@@ -267,6 +268,10 @@ class ClangModuleLoader : public ModuleLoader {
   virtual Type importFunctionReturnType(const clang::FunctionDecl *clangDecl,
                                         DeclContext *dc) = 0;
 
+  virtual Type importVarDeclType(const clang::VarDecl *clangDecl,
+                                 VarDecl *swiftDecl,
+                                 DeclContext *dc) = 0;
+
   /// Find the lookup table that corresponds to the given Clang module.
   ///
   /// \param clangModule The module, or null to indicate that we're talking

include/swift/ClangImporter/ClangImporter.h

@@ -487,6 +487,10 @@ class ClangImporter final : public ClangModuleLoader {
   Type importFunctionReturnType(const clang::FunctionDecl *clangDecl,
                                  DeclContext *dc) override;
 
+  Type importVarDeclType(const clang::VarDecl *clangDecl,
+                         VarDecl *swiftDecl,
+                         DeclContext *dc) override;
+
   Optional<std::string>
   getOrCreatePCH(const ClangImporterOptions &ImporterOptions,
                  StringRef SwiftPCHHash);

lib/ClangImporter/ClangImporter.cpp

@@ -5237,6 +5237,48 @@ Type ClangImporter::importFunctionReturnType(
   return dc->getASTContext().getNeverType();
 }
 
+Type ClangImporter::importVarDeclType(
+    const clang::VarDecl *decl, VarDecl *swiftDecl, DeclContext *dc) {
+  if (decl->getTemplateInstantiationPattern())
+    Impl.getClangSema().InstantiateVariableDefinition(
+        decl->getLocation(),
+        const_cast<clang::VarDecl *>(decl));
+
+  // If the declaration is const, consider it audited.
+  // We can assume that loading a const global variable doesn't
+  // involve an ownership transfer.
+  bool isAudited = decl->getType().isConstQualified();
+
+  auto declType = decl->getType();
+
+  // Special case: NS Notifications
+  if (isNSNotificationGlobal(decl))
+    if (auto newtypeDecl = findSwiftNewtype(decl, Impl.getClangSema(),
+                                            Impl.CurrentVersion))
+      declType = Impl.getClangASTContext().getTypedefType(newtypeDecl);
+
+  bool isInSystemModule =
+      cast<ClangModuleUnit>(dc->getModuleScopeContext())->isSystemModule();
+
+  // Note that we deliberately don't bridge most globals because we want to
+  // preserve pointer identity.
+  auto importedType =
+      Impl.importType(declType,
+                      (isAudited ? ImportTypeKind::AuditedVariable
+                                 : ImportTypeKind::Variable),
+                      ImportDiagnosticAdder(Impl, decl, decl->getLocation()),
+                      isInSystemModule, Bridgeability::None,
+                      getImportTypeAttrs(decl));
+
+  if (!importedType)
+    return nullptr;
+
+  if (importedType.isImplicitlyUnwrapped())
+    swiftDecl->setImplicitlyUnwrappedOptional(true);
+
+  return importedType.getType();
+}
+
 bool ClangImporter::isInOverlayModuleForImportedModule(
                                                const DeclContext *overlayDC,
                                                const DeclContext *importedDC) {

lib/ClangImporter/ImportDecl.cpp

@@ -2684,17 +2684,6 @@ namespace {
       if (isSpecializationDepthGreaterThan(def, 8))
         return nullptr;
 
-      // If we have an inline data member, it won't get eagerly instantiated
-      // when we instantiate the class. So, make sure we do that now to catch
-      // any instantiation errors.
-      for (auto member : decl->decls()) {
-        if (auto varDecl = dyn_cast<clang::VarDecl>(member)) {
-          if (varDecl->getTemplateInstantiationPattern())
-            clangSema.InstantiateVariableDefinition(varDecl->getLocation(),
-                                                    varDecl);
-        }
-      }
-
       return VisitCXXRecordDecl(def);
     }
 
@@ -3390,7 +3379,6 @@ namespace {
     }
 
     Decl *VisitVarDecl(const clang::VarDecl *decl) {
-
       // Variables are imported as... variables.
       ImportedName importedName;
       Optional<ImportedName> correctSwiftName;
@@ -3403,34 +3391,6 @@ namespace {
       if (!dc)
         return nullptr;
 
-      // If the declaration is const, consider it audited.
-      // We can assume that loading a const global variable doesn't
-      // involve an ownership transfer.
-      bool isAudited = decl->getType().isConstQualified();
-
-      auto declType = decl->getType();
-
-      // Special case: NS Notifications
-      if (isNSNotificationGlobal(decl))
-        if (auto newtypeDecl = findSwiftNewtype(decl, Impl.getClangSema(),
-                                                Impl.CurrentVersion))
-          declType = Impl.getClangASTContext().getTypedefType(newtypeDecl);
-
-      // Note that we deliberately don't bridge most globals because we want to
-      // preserve pointer identity.
-      auto importedType =
-          Impl.importType(declType,
-                          (isAudited ? ImportTypeKind::AuditedVariable
-                                     : ImportTypeKind::Variable),
-                          ImportDiagnosticAdder(Impl, decl, decl->getLocation()),
-                          isInSystemModule(dc), Bridgeability::None,
-                          getImportTypeAttrs(decl));
-
-      if (!importedType)
-        return nullptr;
-
-      auto type = importedType.getType();
-
       // If we've imported this variable as a member, it's a static
       // member.
       bool isStatic = false;
@@ -3451,9 +3411,6 @@ namespace {
                        name, dc);
       result->setIsObjC(false);
       result->setIsDynamic(false);
-      result->setInterfaceType(type);
-      Impl.recordImplicitUnwrapForDecl(result,
-                                       importedType.isImplicitlyUnwrapped());
 
       // If imported as member, the member should be final.
       if (dc->getSelfClassDecl())

lib/Sema/TypeCheckDecl.cpp

@@ -2366,6 +2366,14 @@ InterfaceTypeRequest::evaluate(Evaluator &eval, ValueDecl *D) const {
 
   case DeclKind::Var: {
     auto *VD = cast<VarDecl>(D);
+
+    if (auto clangDecl = VD->getClangDecl()) {
+      auto clangVarDecl = cast<clang::VarDecl>(clangDecl);
+
+      return VD->getASTContext().getClangModuleLoader()->importVarDeclType(
+          clangVarDecl, VD, VD->getDeclContext());
+    }
+
     auto *namingPattern = VD->getNamingPattern();
     if (!namingPattern) {
       return ErrorType::get(Context);

test/Interop/Cxx/static/Inputs/invalid-static-member-var.h

@@ -0,0 +1,8 @@
+template<class T>
+struct Bad {
+  typename T::doesnotexist x;
+};
+
+struct X {
+  static Bad<int> b;
+};

test/Interop/Cxx/static/Inputs/module.modulemap

@@ -27,3 +27,8 @@ module ConstexprStaticMemberVarErrors {
   header "constexpr-static-member-var-errors.h"
   requires cplusplus
 }
+
+module InvalidStaticMemberVar {
+  header "invalid-static-member-var.h"
+  requires cplusplus
+}

test/Interop/Cxx/static/constexpr-invalid-static-member-var.swift

@@ -0,0 +1,7 @@
+// RUN: %target-swift-emit-ir %s -I %S/Inputs -enable-experimental-cxx-interop | %FileCheck %s
+
+import InvalidStaticMemberVar
+
+func test(i: X) { }
+
+// CHECK-NOT: error

test/Interop/Cxx/static/constexpr-static-member-var-errors.swift

@@ -3,8 +3,8 @@
 // Check that we properly report the error and don't crash when importing an
 // invalid decl.
 
-// CHECK: error: type 'int' cannot be used prior to '::' because it has no members
-// CHECK: {{note: in instantiation of template class 'GetTypeValue<int>' requested here|note: in instantiation of static data member 'GetTypeValue<int>::value' requested here}}
+// Windows doesn't fail at all here which seems ok (and probably should be the case for other platforms too).
+// XFAIL: windows
 
 // CHECK: error: type 'int' cannot be used prior to '::' because it has no members
-// CHECK: note: in instantiation of static data member 'GetTypeValueInline<int>::value' requested here
+// CHECK: {{note: in instantiation of template class 'GetTypeValue<int>' requested here|note: in instantiation of static data member 'GetTypeValue<int>::value' requested here}}