Skip to content

[AST/ClangImporter] Eliminate layering violation with isInOverlayModuleForImportedModule #16993

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.

Sign up for GitHub

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
Jun 6, 2018
Merged

[AST/ClangImporter] Eliminate layering violation with isInOverlayModuleForImportedModule #16993

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 12 additions & 0 deletions include/swift/AST/ClangModuleLoader.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ class Sema;

namespace swift {

class DeclContext;

class ClangModuleLoader : public ModuleLoader {
private:
virtual void anchor();
Expand All @@ -46,6 +48,16 @@ class ClangModuleLoader : public ModuleLoader {
/// \returns true if there was an error adding the search path.
virtual bool addSearchPath(StringRef newSearchPath, bool isFramework,
bool isSystem) = 0;

/// Determine whether \c overlayDC is within an overlay module for the
/// imported context enclosing \c importedDC.
///
/// This routine is used for various hacks that are only permitted within
/// overlays of imported modules, e.g., Objective-C bridging conformances.
virtual bool isInOverlayModuleForImportedModule(
const DeclContext *overlayDC,
const DeclContext *importedDC) = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation got messed up here (should be double-indented, not right-aligned).

Copy link
Member Author

@DougGregor DougGregor Jun 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double-indentation is awful for function declarations.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right-alignment is worse, since it's more likely to change if the function signature changes. But I'm not just applying my own style here; it's LLVM and clang-format style.


};

} // namespace swift
Expand Down
17 changes: 9 additions & 8 deletions include/swift/ClangImporter/ClangImporter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,15 @@ class ClangImporter final : public ClangModuleLoader {
ArrayRef<std::pair<Identifier, SourceLoc>> path)
override;

/// Determine whether \c overlayDC is within an overlay module for the
/// imported context enclosing \c importedDC.
///
/// This routine is used for various hacks that are only permitted within
/// overlays of imported modules, e.g., Objective-C bridging conformances.
bool isInOverlayModuleForImportedModule(
const DeclContext *overlayDC,
const DeclContext *importedDC) override;

/// \brief Look for declarations associated with the given name.
///
/// \param name The name we're searching for.
Expand Down Expand Up @@ -350,14 +359,6 @@ class ClangImporter final : public ClangModuleLoader {
ImportDecl *createImportDecl(ASTContext &Ctx, DeclContext *DC, ClangNode ClangN,
ArrayRef<clang::Module *> Exported);

/// Determine whether \c overlayDC is within an overlay module for the
/// imported context enclosing \c importedDC.
///
/// This routine is used for various hacks that are only permitted within
/// overlays of imported modules, e.g., Objective-C bridging conformances.
bool isInOverlayModuleForImportedModule(const DeclContext *overlayDC,
const DeclContext *importedDC);

} // end namespace swift

#endif
13 changes: 9 additions & 4 deletions lib/AST/NameLookup.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@
//===----------------------------------------------------------------------===//

#include "NameLookupImpl.h"
#include "swift/ClangImporter/ClangImporter.h"
#include "swift/AST/NameLookup.h"
#include "swift/AST/ASTContext.h"
#include "swift/AST/ASTScope.h"
#include "swift/AST/ASTVisitor.h"
#include "swift/AST/ClangModuleLoader.h"
#include "swift/AST/DebuggerClient.h"
#include "swift/AST/ExistentialLayout.h"
#include "swift/AST/LazyResolver.h"
Expand Down Expand Up @@ -299,13 +299,18 @@ bool swift::removeShadowedDecls(SmallVectorImpl<ValueDecl*> &decls,
// Prefer declarations in an overlay to similar declarations in
// the Clang module it customizes.
if (firstDecl->hasClangNode() != secondDecl->hasClangNode()) {
if (isInOverlayModuleForImportedModule(firstDecl->getDeclContext(),
secondDecl->getDeclContext())){
auto clangLoader = ctx.getClangModuleLoader();
if (!clangLoader) continue;

if (clangLoader->isInOverlayModuleForImportedModule(
firstDecl->getDeclContext(),
secondDecl->getDeclContext())) {
shadowed.insert(secondDecl);
continue;
}

if (isInOverlayModuleForImportedModule(secondDecl->getDeclContext(),
if (clangLoader->isInOverlayModuleForImportedModule(
secondDecl->getDeclContext(),
firstDecl->getDeclContext())) {
shadowed.insert(firstDecl);
break;
Expand Down
7 changes: 4 additions & 3 deletions lib/ClangImporter/ClangImporter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3598,7 +3598,8 @@ importName(const clang::NamedDecl *D,
getDeclName();
}

bool swift::isInOverlayModuleForImportedModule(const DeclContext *overlayDC,
bool ClangImporter::isInOverlayModuleForImportedModule(
const DeclContext *overlayDC,
const DeclContext *importedDC) {
overlayDC = overlayDC->getModuleScopeContext();
importedDC = importedDC->getModuleScopeContext();
Expand All @@ -3613,7 +3614,7 @@ bool swift::isInOverlayModuleForImportedModule(const DeclContext *overlayDC,

// Is this a private module that's re-exported to the public (overlay) name?
auto clangModule =
importedClangModuleUnit->getClangModule()->getTopLevelModule();
importedClangModuleUnit->getClangModule()->getTopLevelModule();
return !clangModule->ExportAsModule.empty() &&
clangModule->ExportAsModule == overlayModule->getName().str();
clangModule->ExportAsModule == overlayModule->getName().str();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation got messed up here.

}
6 changes: 4 additions & 2 deletions lib/Sema/TypeCheckAttr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@
#include "MiscDiagnostics.h"
#include "swift/AST/GenericSignatureBuilder.h"
#include "swift/AST/ASTVisitor.h"
#include "swift/AST/ClangModuleLoader.h"
#include "swift/AST/GenericEnvironment.h"
#include "swift/AST/NameLookup.h"
#include "swift/AST/ParameterList.h"
#include "swift/AST/Types.h"
#include "swift/ClangImporter/ClangImporter.h"
#include "swift/Parse/Lexer.h"
#include "llvm/Support/Debug.h"

Expand Down Expand Up @@ -1413,7 +1413,9 @@ static bool isObjCClassExtensionInOverlay(DeclContext *dc) {
if (!classDecl)
return false;

return isInOverlayModuleForImportedModule(ext, classDecl);
auto clangLoader = dc->getASTContext().getClangModuleLoader();
if (!clangLoader) return false;
return clangLoader->isInOverlayModuleForImportedModule(ext, classDecl);
}

void AttributeChecker::visitRequiredAttr(RequiredAttr *attr) {
Expand Down
5 changes: 4 additions & 1 deletion lib/Sema/TypeCheckProtocol.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "swift/AST/ASTContext.h"
#include "swift/AST/ASTMangler.h"
#include "swift/AST/ASTPrinter.h"
#include "swift/AST/ClangModuleLoader.h"
#include "swift/AST/Decl.h"
#include "swift/AST/ExistentialLayout.h"
#include "swift/AST/GenericEnvironment.h"
Expand Down Expand Up @@ -3628,8 +3629,10 @@ void ConformanceChecker::checkConformance(MissingWitnessDiagnosisKind Kind) {
if (Proto->isSpecificProtocol(KnownProtocolKind::ObjectiveCBridgeable)) {
auto nominal = Adoptee->getAnyNominal();
if (!TC.Context.isTypeBridgedInExternalModule(nominal)) {
auto clangLoader = TC.Context.getClangModuleLoader();
if (nominal->getParentModule() != DC->getParentModule() &&
!isInOverlayModuleForImportedModule(DC, nominal)) {
!(clangLoader &&
clangLoader->isInOverlayModuleForImportedModule(DC, nominal))) {
auto nominalModule = nominal->getParentModule();
TC.diagnose(Loc, diag::nonlocal_bridged_to_objc, nominal->getName(),
Proto->getName(), nominalModule->getName());
Expand Down