Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions include/swift/AST/ClangModuleLoader.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ namespace swift {

class ConcreteDeclRef;
class Decl;
class VarDecl;
class DeclContext;
class EffectiveClangContext;
class SwiftLookupTable;
Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions include/swift/ClangImporter/ClangImporter.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
42 changes: 42 additions & 0 deletions lib/ClangImporter/ClangImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5236,6 +5236,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) {
Expand Down
43 changes: 0 additions & 43 deletions lib/ClangImporter/ImportDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -3390,7 +3379,6 @@ namespace {
}

Decl *VisitVarDecl(const clang::VarDecl *decl) {

// Variables are imported as... variables.
ImportedName importedName;
Optional<ImportedName> correctSwiftName;
Expand All @@ -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;
Expand All @@ -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())
Expand Down
8 changes: 8 additions & 0 deletions lib/Sema/TypeCheckDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2365,6 +2365,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);
Expand Down
8 changes: 8 additions & 0 deletions test/Interop/Cxx/static/Inputs/invalid-static-member-var.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
template<class T>
struct Bad {
typename T::doesnotexist x;
};

struct X {
static Bad<int> b;
};
5 changes: 5 additions & 0 deletions test/Interop/Cxx/static/Inputs/module.modulemap
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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}}