Skip to content
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

[cxx-interop] Fix access check for nested private C++ enums #80366

Merged
merged 3 commits into from
Apr 1, 2025
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
2 changes: 1 addition & 1 deletion include/swift/ClangImporter/ClangImporter.h
Original file line number Diff line number Diff line change
Expand Up @@ -757,7 +757,7 @@ AccessLevel convertClangAccess(clang::AccessSpecifier access);
/// The returned fileIDs may not be of a valid format (e.g., missing a '/'),
/// and should be parsed using swift::SourceFile::FileIDStr::parse().
SmallVector<std::pair<StringRef, clang::SourceLocation>, 1>
getPrivateFileIDAttrs(const clang::Decl *decl);
getPrivateFileIDAttrs(const clang::CXXRecordDecl *decl);

/// Use some heuristics to determine whether the clang::Decl associated with
/// \a decl would not exist without C++ interop.
Expand Down
23 changes: 18 additions & 5 deletions lib/AST/DeclContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1490,21 +1490,34 @@ bool AccessScope::allowsPrivateAccess(const DeclContext *useDC, const DeclContex
// Do not allow access if the sourceDC is in a different file
auto *useSF = useDC->getOutermostParentSourceFile();
if (useSF != sourceDC->getOutermostParentSourceFile()) {
// This might be a C++ declaration with a SWIFT_PRIVATE_FILEID
// attribute, which asks us to treat it as if it were defined in the file
// sourceDC might be a C++ record with a SWIFT_PRIVATE_FILEID attribute,
// which asks us to treat it as if it were defined in the file
// with the specified FileID.

auto clangDecl = sourceNTD->getDecl()->getClangDecl();
if (!clangDecl)

if (isa_and_nonnull<clang::EnumDecl>(clangDecl)) {
// Consider: class C { private: enum class E { M }; };
// If sourceDC is a C++ enum (e.g, E), then we are looking at one of its
// members (e.g., E.M). If this is the case, then we should consider
// the SWIFT_PRIVATE_FILEID of its enclosing class decl (e.g., C), if any.
// Doing so allows the nested private enum's members to inherit the access
// of the nested enum type itself.
clangDecl = dyn_cast<clang::CXXRecordDecl>(clangDecl->getDeclContext());
sourceDC = sourceNTD->getDeclContext();
}

if (!isa_and_nonnull<clang::CXXRecordDecl>(clangDecl))
return false;

auto recordDecl = cast<clang::CXXRecordDecl>(clangDecl);

// Diagnostics should enforce that there is at most SWIFT_PRIVATE_FILEID,
// but this handles the case where there is more than anyway (whether that
// is a feature or a bug). Allow access check to proceed if useSF is blessed
// by any of the SWIFT_PRIVATE_FILEID annotations (i.e., disallow private
// access if none of them bless useSF).
if (!llvm::any_of(
importer::getPrivateFileIDAttrs(clangDecl), [&](auto &blessed) {
importer::getPrivateFileIDAttrs(recordDecl), [&](auto &blessed) {
auto blessedFileID = SourceFile::FileIDStr::parse(blessed.first);
return blessedFileID && blessedFileID->matches(useSF);
})) {
Expand Down
7 changes: 4 additions & 3 deletions lib/ClangImporter/ClangImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6277,9 +6277,11 @@ TinyPtrVector<ValueDecl *> ClangRecordMemberLookup::evaluate(
// to import all non-public members by default; if that is disabled, we only
// import non-public members annotated with SWIFT_PRIVATE_FILEID (since those
// are the only classes that need non-public members.)
auto *cxxRecordDecl =
dyn_cast<clang::CXXRecordDecl>(inheritingDecl->getClangDecl());
auto skipIfNonPublic =
!ctx.LangOpts.hasFeature(Feature::ImportNonPublicCxxMembers) &&
importer::getPrivateFileIDAttrs(inheritingDecl->getClangDecl()).empty();
cxxRecordDecl && importer::getPrivateFileIDAttrs(cxxRecordDecl).empty();

auto directResults = evaluateOrDefault(
ctx.evaluator,
Expand Down Expand Up @@ -8761,9 +8763,8 @@ void ClangInheritanceInfo::setUnavailableIfNecessary(
}

SmallVector<std::pair<StringRef, clang::SourceLocation>, 1>
importer::getPrivateFileIDAttrs(const clang::Decl *decl) {
importer::getPrivateFileIDAttrs(const clang::CXXRecordDecl *decl) {
llvm::SmallVector<std::pair<StringRef, clang::SourceLocation>, 1> files;

constexpr auto prefix = StringRef("private_fileid:");

if (decl->hasAttrs()) {
Expand Down
9 changes: 5 additions & 4 deletions lib/ClangImporter/ImportDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10021,10 +10021,11 @@ void ClangImporter::Implementation::loadAllMembersOfRecordDecl(
// to import all non-public members by default; if that is disabled, we only
// import non-public members annotated with SWIFT_PRIVATE_FILEID (since those
// are the only classes that need non-public members.)
auto skipIfNonPublic =
!swiftDecl->getASTContext().LangOpts.hasFeature(
Feature::ImportNonPublicCxxMembers) &&
importer::getPrivateFileIDAttrs(swiftDecl->getClangDecl()).empty();
auto *baseRecord = dyn_cast<clang::CXXRecordDecl>(swiftDecl->getClangDecl());
auto skipIfNonPublic = !swiftDecl->getASTContext().LangOpts.hasFeature(
Feature::ImportNonPublicCxxMembers) &&
baseRecord &&
importer::getPrivateFileIDAttrs(baseRecord).empty();

// Import all of the members.
llvm::SmallVector<Decl *, 16> members;
Expand Down
16 changes: 8 additions & 8 deletions test/Interop/Cxx/class/access/Inputs/non-public.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@ __attribute__((__swift_attr__("private_fileid:main/blessed.swift"))) MyClass {
typedef int publTypedef;
struct publStruct {};

enum publEnum { publEnumValue1 };
enum class publEnumClass { publEnumClassValue1 };
enum publEnum { variantPublEnum };
enum { publEnumAnonValue1 };
enum class publEnumClass { variantPublEnumClass };
enum publEnumClosed {
publEnumClosedValue1
variantPublEnumClosed
} __attribute__((enum_extensibility(closed)));
enum publEnumOpen {
publEnumOpenValue1
variantPublEnumOpen
} __attribute__((enum_extensibility(open)));
enum publEnumFlag {} __attribute__((flag_enum));

Expand All @@ -48,14 +48,14 @@ __attribute__((__swift_attr__("private_fileid:main/blessed.swift"))) MyClass {
typedef int privTypedef;
struct privStruct {};

enum privEnum { privEnumValue1 };
enum class privEnumClass { privEnumClassValue1 };
enum privEnum { variantPrivEnum };
enum { privEnumAnonValue1 };
enum class privEnumClass { variantPrivEnumClass };
enum privEnumClosed {
privEnumClosedValue1
variantPrivEnumClosed
} __attribute__((enum_extensibility(closed)));
enum privEnumOpen {
privEnumOpenValue1
variantPrivEnumOpen
} __attribute__((enum_extensibility(open)));
enum privEnumFlag {} __attribute__((flag_enum));
};
Expand Down
103 changes: 52 additions & 51 deletions test/Interop/Cxx/class/access/private-fileid-typecheck.swift
Original file line number Diff line number Diff line change
Expand Up @@ -104,20 +104,20 @@ extension MyClass {
let _: privEnumFlag

// TODO: Enum variants are not being correctly imported. Test the following when that is fixed:
// let _ = publEnum.publEnumValue1
// let _ = privEnum.privEnumValue1
// let _ = variantPublEnum
// let _ = variantPrivEnum
//
// let _ = publEnumClass.publEnumClassValue1
// let _ = privEnumClass.privEnumClassValue1
//
// let _ = publEnumAnonValue1
// let _ = privEnumAnonValue1
//
// let _ = publEnumClosed.Value1
// let _ = privEnumClosed.Value1
//
// let _ = publEnumOpen.Value1
// let _ = privEnumOpen.Value1
// let _ = publEnumAnonVariant
// let _ = privEnumAnonVariant

let _ = publEnumClass.variantPublEnumClass
let _ = privEnumClass.variantPrivEnumClass

let _ = publEnumClosed.variantPublEnumClosed
let _ = privEnumClosed.variantPrivEnumClosed

let _ = publEnumOpen.variantPublEnumOpen
let _ = privEnumOpen.variantPrivEnumOpen
}

func fcutd(_ _: publTypedef) { }
Expand Down Expand Up @@ -162,20 +162,21 @@ func notExt(_ c: inout MyClass) {
let _: MyClass.privEnumFlag // expected-error {{'privEnumFlag' is inaccessible due to 'private' protection level}}

// TODO: Enum variants are not being correctly imported. Test the following when that is fixed:
// let _ = MyClass.publEnum.publEnumValue1
// let _ = MyClass.privEnum.privEnumValue1
//
// let _ = MyClass.publEnumClass.publEnumClassValue1
// let _ = MyClass.privEnumClass.privEnumClassValue1
// let _ = MyClass.variantPublEnum
// let _ = MyClass.variantPrivEnum // TODO-error {{'variantPrivEnum' is inaccessible due to 'private' protection level}}
//
// let _ = MyClass.publEnumAnonValue1
// let _ = MyClass.privEnumAnonValue1
//
// let _ = MyClass.publEnumClosed.Value1
// let _ = MyClass.privEnumClosed.Value1
//
// let _ = MyClass.publEnumOpen.Value1
// let _ = MyClass.privEnumOpen.Value1
// let _ = MyClass.publEnumAnonVariant
// let _ = MyClass.privEnumAnonVariant // TODO-error {{'privEnumAnonVariant' is inaccessible due to 'private' protection level}}

let _ = MyClass.publEnumClass.variantPublEnumClass
let _ = MyClass.privEnumClass.variantPrivEnumClass // expected-error {{'privEnumClass' is inaccessible due to 'private' protection level}}

let _ = MyClass.publEnumClosed.variantPublEnumClosed
let _ = MyClass.privEnumClosed.variantPrivEnumClosed // expected-error {{'privEnumClosed' is inaccessible due to 'private' protection level}}

let _ = MyClass.publEnumOpen.variantPublEnumOpen
let _ = MyClass.privEnumOpen.variantPrivEnumOpen // expected-error {{'privEnumOpen' is inaccessible due to 'private' protection level}}
}

//--- cursed.swift
Expand Down Expand Up @@ -228,20 +229,20 @@ extension MyClass {
let _: privEnumFlag // expected-error {{'privEnumFlag' is inaccessible due to 'private' protection level}}

// TODO: Enum variants are not being correctly imported. Test the following when that is fixed:
// let _ = publEnum.publEnumValue1
// let _ = privEnum.privEnumValue1
//
// let _ = publEnumClass.publEnumClassValue1
// let _ = privEnumClass.privEnumClassValue1
// let _ = variantPublEnum
// let _ = variantPrivEnum // TODO-error {{'variantPrivEnum' is inaccessible due to 'private' protection level}}
//
// let _ = publEnumAnonValue1
// let _ = privEnumAnonValue1
//
// let _ = publEnumClosed.Value1
// let _ = privEnumClosed.Value1
//
// let _ = publEnumOpen.Value1
// let _ = privEnumOpen.Value1
// let _ = publEnumAnonVariant
// let _ = privEnumAnonVariant // TODO-error {{'privEnumAnonVariant' is inaccessible due to 'private' protection level}}

let _ = publEnumClass.variantPublEnumClass
let _ = privEnumClass.variantPrivEnumClass// expected-error {{'privEnumClass' is inaccessible due to 'private' protection level}}

let _ = publEnumClosed.variantPublEnumClosed
let _ = privEnumClosed.variantPrivEnumClosed // expected-error {{'privEnumClosed' is inaccessible due to 'private' protection level}}

let _ = publEnumOpen.variantPublEnumOpen
let _ = privEnumOpen.variantPrivEnumOpen // expected-error {{'privEnumOpen' is inaccessible due to 'private' protection level}}
}
}

Expand Down Expand Up @@ -283,18 +284,18 @@ func notExt(_ c: inout MyClass) {
let _: MyClass.privEnumFlag // expected-error {{'privEnumFlag' is inaccessible due to 'private' protection level}}

// TODO: Enum variants are not being correctly imported. Test the following when that is fixed:
// let _ = MyClass.publEnum.publEnumValue1
// let _ = MyClass.privEnum.privEnumValue1
//
// let _ = MyClass.publEnumClass.publEnumClassValue1
// let _ = MyClass.privEnumClass.privEnumClassValue1
// let _ = MyClass.variantPublEnum
// let _ = MyClass.variantPrivEnum // TODO-error {{'variantPrivEnum' is inaccessible due to 'private' protection level}}
//
// let _ = MyClass.publEnumAnonValue1
// let _ = MyClass.privEnumAnonValue1
//
// let _ = MyClass.publEnumClosed.Value1
// let _ = MyClass.privEnumClosed.Value1
//
// let _ = MyClass.publEnumOpen.Value1
// let _ = MyClass.privEnumOpen.Value1
// let _ = MyClass.publEnumAnonVariant
// let _ = MyClass.privEnumAnonVariant // TODO-error {{'privEnumAnonVariant' is inaccessible due to 'private' protection level}}

let _ = MyClass.publEnumClass.variantPublEnumClass
let _ = MyClass.privEnumClass.variantPrivEnumClass// expected-error {{'privEnumClass' is inaccessible due to 'private' protection level}}

let _ = MyClass.publEnumClosed.variantPublEnumClosed
let _ = MyClass.privEnumClosed.variantPrivEnumClosed // expected-error {{'privEnumClosed' is inaccessible due to 'private' protection level}}

let _ = MyClass.publEnumOpen.variantPublEnumOpen
let _ = MyClass.privEnumOpen.variantPrivEnumOpen // expected-error {{'privEnumOpen' is inaccessible due to 'private' protection level}}
}