Skip to content

Commit bbf92fd

Browse files
j-huij-hui
authored
[cxx-interop] Import non-public members of SWIFT_PRIVATE_FILEID-annotated classes (swiftlang#80320)
1 parent e8d871c commit bbf92fd

8 files changed

+81
-56
lines changed

‎lib/ClangImporter/ClangImporter.cpp

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6272,6 +6272,15 @@ TinyPtrVector<ValueDecl *> ClangRecordMemberLookup::evaluate(
62726272
ClangInheritanceInfo inheritance = desc.inheritance;
62736273

62746274
auto &ctx = recordDecl->getASTContext();
6275+
6276+
// Whether to skip non-public members. Feature::ImportNonPublicCxxMembers says
6277+
// to import all non-public members by default; if that is disabled, we only
6278+
// import non-public members annotated with SWIFT_PRIVATE_FILEID (since those
6279+
// are the only classes that need non-public members.)
6280+
auto skipIfNonPublic =
6281+
!ctx.LangOpts.hasFeature(Feature::ImportNonPublicCxxMembers) &&
6282+
importer::getPrivateFileIDAttrs(inheritingDecl->getClangDecl()).empty();
6283+
62756284
auto directResults = evaluateOrDefault(
62766285
ctx.evaluator,
62776286
ClangDirectLookupRequest({recordDecl, recordDecl->getClangDecl(), name}),
@@ -6289,16 +6298,25 @@ TinyPtrVector<ValueDecl *> ClangRecordMemberLookup::evaluate(
62896298
recordDecl->getClangDecl())
62906299
continue;
62916300

6292-
if (!ctx.LangOpts.hasFeature(Feature::ImportNonPublicCxxMembers)) {
6293-
auto access = found->getAccess();
6294-
if ((access == clang::AS_private || access == clang::AS_protected) &&
6295-
(inheritance || !isa<clang::FieldDecl>(found)))
6296-
// 'found' is a non-public member and ImportNonPublicCxxMembers is not
6297-
// enabled. Don't import it unless it is a non-inherited field, which
6298-
// we must import because it may affect implicit conformances that
6299-
// iterate through all of a struct's fields, e.g., Sendable (#76892).
6300-
continue;
6301-
}
6301+
// We should not import 'found' if the following are all true:
6302+
//
6303+
// - Feature::ImportNonPublicCxxMembers is not enabled
6304+
// - 'found' is not a member of a SWIFT_PRIVATE_FILEID-annotated class
6305+
// - 'found' is a non-public member.
6306+
// - 'found' is not a non-inherited FieldDecl; we must import private
6307+
// fields because they may affect implicit conformances that iterate
6308+
// through all of a struct's fields, e.g., Sendable (#76892).
6309+
//
6310+
// Note that we can skip inherited FieldDecls because implicit conformances
6311+
// handle those separately.
6312+
//
6313+
// The first two conditions are captured by skipIfNonPublic. The next two
6314+
// are conveyed by the following:
6315+
auto nonPublic = found->getAccess() == clang::AS_private ||
6316+
found->getAccess() == clang::AS_protected;
6317+
auto noninheritedField = !inheritance && isa<clang::FieldDecl>(found);
6318+
if (skipIfNonPublic && nonPublic && !noninheritedField)
6319+
continue;
63026320

63036321
// Don't import constructors on foreign reference types.
63046322
if (isa<clang::CXXConstructorDecl>(found) && isa<ClassDecl>(recordDecl))
@@ -6353,8 +6371,7 @@ TinyPtrVector<ValueDecl *> ClangRecordMemberLookup::evaluate(
63536371
foundNameArities.insert(getArity(valueDecl));
63546372

63556373
for (auto base : cxxRecord->bases()) {
6356-
if (!ctx.LangOpts.hasFeature(Feature::ImportNonPublicCxxMembers) &&
6357-
base.getAccessSpecifier() != clang::AS_public)
6374+
if (skipIfNonPublic && base.getAccessSpecifier() != clang::AS_public)
63586375
continue;
63596376

63606377
clang::QualType baseType = base.getType();

‎lib/ClangImporter/ImportDecl.cpp

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10016,24 +10016,42 @@ static void loadAllMembersOfSuperclassIfNeeded(ClassDecl *CD) {
1001610016
void ClangImporter::Implementation::loadAllMembersOfRecordDecl(
1001710017
NominalTypeDecl *swiftDecl, const clang::RecordDecl *clangRecord,
1001810018
ClangInheritanceInfo inheritance) {
10019+
10020+
// Whether to skip non-public members. Feature::ImportNonPublicCxxMembers says
10021+
// to import all non-public members by default; if that is disabled, we only
10022+
// import non-public members annotated with SWIFT_PRIVATE_FILEID (since those
10023+
// are the only classes that need non-public members.)
10024+
auto skipIfNonPublic =
10025+
!swiftDecl->getASTContext().LangOpts.hasFeature(
10026+
Feature::ImportNonPublicCxxMembers) &&
10027+
importer::getPrivateFileIDAttrs(swiftDecl->getClangDecl()).empty();
10028+
1001910029
// Import all of the members.
1002010030
llvm::SmallVector<Decl *, 16> members;
1002110031
for (const clang::Decl *m : clangRecord->decls()) {
1002210032
auto nd = dyn_cast<clang::NamedDecl>(m);
1002310033
if (!nd)
1002410034
continue;
1002510035

10026-
if (!swiftDecl->getASTContext().LangOpts.hasFeature(
10027-
Feature::ImportNonPublicCxxMembers)) {
10028-
auto access = nd->getAccess();
10029-
if ((access == clang::AS_private || access == clang::AS_protected) &&
10030-
(inheritance || !isa<clang::FieldDecl>(nd)))
10031-
// 'nd' is a non-public member and ImportNonPublicCxxMembers is not
10032-
// enabled. Don't import it unless it is a non-inherited field, which
10033-
// we must import because it may affect implicit conformances that
10034-
// iterate through all of a struct's fields, e.g., Sendable (#76892).
10035-
continue;
10036-
}
10036+
// We should not import 'found' if the following are all true:
10037+
//
10038+
// - Feature::ImportNonPublicCxxMembers is not enabled
10039+
// - 'found' is not a member of a SWIFT_PRIVATE_FILEID-annotated class
10040+
// - 'found' is a non-public member.
10041+
// - 'found' is not a non-inherited FieldDecl; we must import private
10042+
// fields because they may affect implicit conformances that iterate
10043+
// through all of a struct's fields, e.g., Sendable (#76892).
10044+
//
10045+
// Note that we can skip inherited FieldDecls because implicit conformances
10046+
// handle those separately.
10047+
//
10048+
// The first two conditions are captured by skipIfNonPublic. The next two
10049+
// are conveyed by the following:
10050+
auto nonPublic = nd->getAccess() == clang::AS_private ||
10051+
nd->getAccess() == clang::AS_protected;
10052+
auto noninheritedField = !inheritance && isa<clang::FieldDecl>(nd);
10053+
if (skipIfNonPublic && nonPublic && !noninheritedField)
10054+
continue;
1003710055

1003810056
// Currently, we don't import unnamed bitfields.
1003910057
if (isa<clang::FieldDecl>(m) &&
@@ -10102,9 +10120,7 @@ void ClangImporter::Implementation::loadAllMembersOfRecordDecl(
1010210120
// If this is a C++ record, look through the base classes too.
1010310121
if (auto cxxRecord = dyn_cast<clang::CXXRecordDecl>(clangRecord)) {
1010410122
for (auto base : cxxRecord->bases()) {
10105-
if (!swiftDecl->getASTContext().LangOpts.hasFeature(
10106-
Feature::ImportNonPublicCxxMembers) &&
10107-
base.getAccessSpecifier() != clang::AS_public)
10123+
if (skipIfNonPublic && base.getAccessSpecifier() != clang::AS_public)
1010810124
continue;
1010910125

1011010126
clang::QualType baseType = base.getType();

‎test/Interop/Cxx/class/access/non-public-inheritance-executable.swift

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,11 @@
22
// Test that all accessible inherited methods can be called.
33
//
44
// RUN: split-file %s %t
5-
// RUN: %target-build-swift -module-name main %t/blessed.swift -I %S/Inputs -o %t/out -Xfrontend -cxx-interoperability-mode=default -enable-experimental-feature ImportNonPublicCxxMembers
5+
// RUN: %target-build-swift -module-name main %t/blessed.swift -I %S/Inputs -o %t/out -Xfrontend -cxx-interoperability-mode=default
66
// RUN: %target-codesign %t/out
77
// RUN: %target-run %t/out
88
//
99
// REQUIRES: executable_test
10-
// REQUIRES: swift_feature_ImportNonPublicCxxMembers
1110

1211
import StdlibUnittest
1312
import NonPublicInheritance

‎test/Interop/Cxx/class/access/non-public-inheritance-typecheck.swift

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
//--- blessed.swift
22
// RUN: split-file %s %t
3-
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -module-name main %t/blessed.swift -enable-experimental-feature ImportNonPublicCxxMembers
4-
// REQUIRES: swift_feature_ImportNonPublicCxxMembers
3+
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -module-name main %t/blessed.swift
54
import NonPublicInheritance
65

76
// Extensions of each class test whether we correctly modeled *which* members

‎test/Interop/Cxx/class/access/private-fileid-diagnostics.swift

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
// RUN: split-file %s %t
2-
// RUN: %target-swift-frontend -typecheck -verify -suppress-remarks %t/some/subdir/file1.swift -verify-additional-file %t/Cxx/include/cxx-header.h -I %t/Cxx/include -cxx-interoperability-mode=default -enable-experimental-feature ImportNonPublicCxxMembers -module-name main
3-
// REQUIRES: swift_feature_ImportNonPublicCxxMembers
2+
// RUN: %target-swift-frontend -typecheck -verify -suppress-remarks %t/some/subdir/file1.swift -verify-additional-file %t/Cxx/include/cxx-header.h -I %t/Cxx/include -cxx-interoperability-mode=default -module-name main
43

54
// This test uses -verify-additional-file, which do not work well on Windows:
65
// UNSUPPORTED: OS=windows-msvc

‎test/Interop/Cxx/class/access/private-fileid-nested-typecheck.swift

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,8 @@
22
// works as expected.
33
//
44
// RUN: split-file %s %t
5-
// RUN: %target-swift-frontend -typecheck -verify %t/file1.swift -I %t/include -cxx-interoperability-mode=default -enable-experimental-feature ImportNonPublicCxxMembers -module-name main
6-
// RUN: %target-swift-frontend -typecheck -verify %t/file2.swift -I %t/include -cxx-interoperability-mode=default -enable-experimental-feature ImportNonPublicCxxMembers -module-name main
7-
// REQUIRES: swift_feature_ImportNonPublicCxxMembers
5+
// RUN: %target-swift-frontend -typecheck -verify %t/file1.swift -I %t/include -cxx-interoperability-mode=default -module-name main
6+
// RUN: %target-swift-frontend -typecheck -verify %t/file2.swift -I %t/include -cxx-interoperability-mode=default -module-name main
87

98
//--- include/module.modulemap
109
module CxxModule {

‎test/Interop/Cxx/class/access/private-fileid-template-typecheck.swift

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,7 @@
1515
// non-public in different contexts, and variations in module and file names.
1616
//
1717
// RUN: split-file %s %t
18-
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -enable-experimental-feature ImportNonPublicCxxMembers -module-name main %t/blessed.swift
19-
// REQUIRES: swift_feature_ImportNonPublicCxxMembers
18+
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -module-name main %t/blessed.swift
2019

2120
//--- blessed.swift
2221

‎test/Interop/Cxx/class/access/private-fileid-typecheck.swift

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,6 @@
33
// in the contexts where they should be accessible (i.e., the files blessed by
44
// the SWIFT_PRIVATE_FILEID annotation).
55
//
6-
// For now, it requires the following feature to import private members:
7-
// REQUIRES: swift_feature_ImportNonPublicCxxMembers
8-
//
96
// The private_fileid mechanism relies on fileIDs, so we need some control over
107
// file names:
118
//
@@ -19,37 +16,37 @@
1916
// members are private (default) or protected. The result should be the same
2017
// no matter the configuration.
2118
//
22-
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -enable-experimental-feature ImportNonPublicCxxMembers -module-name main %t/blessed.swift
23-
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -enable-experimental-feature ImportNonPublicCxxMembers -module-name main %t/blessed.swift -Xcc -DTEST_CLASS=struct
24-
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -enable-experimental-feature ImportNonPublicCxxMembers -module-name main %t/blessed.swift -Xcc -DTEST_PRIVATE=protected
25-
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -enable-experimental-feature ImportNonPublicCxxMembers -module-name main %t/blessed.swift -Xcc -DTEST_CLASS=struct -Xcc -DTEST_PRIVATE=protected
19+
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -module-name main %t/blessed.swift
20+
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -module-name main %t/blessed.swift -Xcc -DTEST_CLASS=struct
21+
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -module-name main %t/blessed.swift -Xcc -DTEST_PRIVATE=protected
22+
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -module-name main %t/blessed.swift -Xcc -DTEST_CLASS=struct -Xcc -DTEST_PRIVATE=protected
2623
//
2724
// This test also includes a "cursed.swift", which expects to not have access to
2825
// non-public members:
2926
//
30-
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -enable-experimental-feature ImportNonPublicCxxMembers -module-name main %t/cursed.swift
31-
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -enable-experimental-feature ImportNonPublicCxxMembers -module-name main %t/cursed.swift -Xcc -DTEST_CLASS=struct
32-
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -enable-experimental-feature ImportNonPublicCxxMembers -module-name main %t/cursed.swift -Xcc -DTEST_PRIVATE=protected
33-
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -enable-experimental-feature ImportNonPublicCxxMembers -module-name main %t/cursed.swift -Xcc -DTEST_CLASS=struct -Xcc -DTEST_PRIVATE=protected
27+
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -module-name main %t/cursed.swift
28+
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -module-name main %t/cursed.swift -Xcc -DTEST_CLASS=struct
29+
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -module-name main %t/cursed.swift -Xcc -DTEST_PRIVATE=protected
30+
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -module-name main %t/cursed.swift -Xcc -DTEST_CLASS=struct -Xcc -DTEST_PRIVATE=protected
3431
//
3532
// To check that fileID is agnostic about directory structure within a module,
3633
// we move blessed.swift into a subdirectory (but keep its filename).
3734
//
3835
// RUN: mkdir -p %t/subdir/subsubdir
3936
// RUN: mv %t/blessed.swift %t/subdir/subsubdir/blessed.swift
40-
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -enable-experimental-feature ImportNonPublicCxxMembers -module-name main %t/subdir/subsubdir/blessed.swift
41-
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -enable-experimental-feature ImportNonPublicCxxMembers -module-name main %t/subdir/subsubdir/blessed.swift -Xcc -DTEST_CLASS=struct
42-
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -enable-experimental-feature ImportNonPublicCxxMembers -module-name main %t/subdir/subsubdir/blessed.swift -Xcc -DTEST_PRIVATE=protected
43-
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -enable-experimental-feature ImportNonPublicCxxMembers -module-name main %t/subdir/subsubdir/blessed.swift -Xcc -DTEST_CLASS=struct -Xcc -DTEST_PRIVATE=protected
37+
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -module-name main %t/subdir/subsubdir/blessed.swift
38+
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -module-name main %t/subdir/subsubdir/blessed.swift -Xcc -DTEST_CLASS=struct
39+
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -module-name main %t/subdir/subsubdir/blessed.swift -Xcc -DTEST_PRIVATE=protected
40+
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -module-name main %t/subdir/subsubdir/blessed.swift -Xcc -DTEST_CLASS=struct -Xcc -DTEST_PRIVATE=protected
4441
//
4542
// To check that fileID is sensitive to module names, rename cursed.swift to
4643
// "blessed.swift", but typecheck in a module not called "main".
4744
//
4845
// RUN: mv %t/cursed.swift %t/blessed.swift
49-
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -enable-experimental-feature ImportNonPublicCxxMembers -module-name brain %t/blessed.swift
50-
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -enable-experimental-feature ImportNonPublicCxxMembers -module-name brain %t/blessed.swift -Xcc -DTEST_CLASS=struct
51-
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -enable-experimental-feature ImportNonPublicCxxMembers -module-name brain %t/blessed.swift -Xcc -DTEST_PRIVATE=protected
52-
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -enable-experimental-feature ImportNonPublicCxxMembers -module-name brain %t/blessed.swift -Xcc -DTEST_CLASS=struct -Xcc -DTEST_PRIVATE=protected
46+
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -module-name brain %t/blessed.swift
47+
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -module-name brain %t/blessed.swift -Xcc -DTEST_CLASS=struct
48+
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -module-name brain %t/blessed.swift -Xcc -DTEST_PRIVATE=protected
49+
// RUN: %target-swift-frontend -typecheck -verify -I %S/Inputs -cxx-interoperability-mode=default -module-name brain %t/blessed.swift -Xcc -DTEST_CLASS=struct -Xcc -DTEST_PRIVATE=protected
5350

5451
//--- blessed.swift
5552

0 commit comments

Comments
 (0)