Skip to content

Attach Lazy ClangImporter Diagnostics as Notes to Sema Diagnostics #41079

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
Jan 30, 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
3 changes: 0 additions & 3 deletions include/swift/AST/ASTContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -1217,9 +1217,6 @@ class ASTContext final {
InheritedProtocolConformance *
getInheritedConformance(Type type, ProtocolConformance *inherited);

/// Check if \p decl is included in LazyContexts.
bool isLazyContext(const DeclContext *decl);

/// Get the lazy data for the given declaration.
///
/// \param lazyLoader If non-null, the lazy loader to use when creating the
Expand Down
10 changes: 8 additions & 2 deletions include/swift/AST/ClangModuleLoader.h
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,14 @@ class ClangModuleLoader : public ModuleLoader {
/// Imports a clang decl directly, rather than looking up its name.
virtual Decl *importDeclDirectly(const clang::NamedDecl *decl) = 0;

/// Emits any import diagnostics associated with the provided decl.
virtual void diagnoseDeclDirectly(const clang::NamedDecl *decl) = 0;
/// Emits diagnostics for any declarations named name
/// whose direct declaration context is a TU.
virtual void diagnoseTopLevelValue(const DeclName &name) = 0;

/// Emit diagnostics for declarations named name that are members
/// of the provided baseType.
virtual void diagnoseMemberValue(const DeclName &name,
const Type &baseType) = 0;

/// Instantiate and import class template using given arguments.
///
Expand Down
14 changes: 7 additions & 7 deletions include/swift/AST/DiagnosticsClangImporter.def
Original file line number Diff line number Diff line change
Expand Up @@ -107,20 +107,20 @@ NOTE(macro_not_imported_invalid_numeric_literal, none, "invalid numeric literal"
NOTE(macro_not_imported_unsupported_literal, none, "only numeric and string macro literals supported", ())
NOTE(macro_not_imported_nested_cast, none, "non-null nested casts not supported", ())

ERROR(macro_not_imported_function_like, none, "macro '%0' not imported: function like macros not supported", (StringRef))
ERROR(macro_not_imported_unsupported_structure, none, "macro '%0' not imported: structure not supported", (StringRef))
ERROR(macro_not_imported, none, "macro '%0' not imported", (StringRef))
NOTE(macro_not_imported_function_like, none, "macro '%0' not imported: function like macros not supported", (StringRef))
NOTE(macro_not_imported_unsupported_structure, none, "macro '%0' not imported: structure not supported", (StringRef))
NOTE(macro_not_imported, none, "macro '%0' not imported", (StringRef))

NOTE(return_type_not_imported, none, "return type not imported", ())
NOTE(parameter_type_not_imported, none, "parameter %0 not imported", (const clang::NamedDecl*))
NOTE(incomplete_interface, none, "interface %0 is incomplete", (const clang::NamedDecl*))
NOTE(incomplete_protocol, none, "protocol %0 is incomplete", (const clang::NamedDecl*))
NOTE(unsupported_builtin_type, none, "built-in type '%0' not supported", (StringRef))

WARNING(record_field_not_imported, none, "field %0 not imported", (const clang::NamedDecl*))
WARNING(invoked_func_not_imported, none, "function %0 not imported", (const clang::NamedDecl*))
WARNING(record_method_not_imported, none, "method %0 not imported", (const clang::NamedDecl*))
WARNING(objc_property_not_imported, none, "property %0 not imported", (const clang::NamedDecl*))
NOTE(record_field_not_imported, none, "field %0 not imported", (const clang::NamedDecl*))
NOTE(invoked_func_not_imported, none, "function %0 not imported", (const clang::NamedDecl*))
NOTE(record_method_not_imported, none, "method %0 not imported", (const clang::NamedDecl*))
NOTE(objc_property_not_imported, none, "property %0 not imported", (const clang::NamedDecl*))

NOTE(forward_declared_interface_label, none, "interface %0 forward declared here", (const clang::NamedDecl*))
NOTE(forward_declared_protocol_label, none, "protocol %0 forward declared here", (const clang::NamedDecl*))
Expand Down
3 changes: 0 additions & 3 deletions include/swift/AST/LazyResolver.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,6 @@ class alignas(void*) LazyMemberLoader {
loadNamedMembers(const IterableDeclContext *IDC, DeclBaseName N,
uint64_t contextData) = 0;

virtual void diagnoseMissingNamedMember(const IterableDeclContext *IDC,
DeclName name) = 0;

/// Populates the given vector with all conformances for \p D.
///
/// The implementation should \em not call setConformances on \p D.
Expand Down
3 changes: 3 additions & 0 deletions include/swift/AST/NameLookup.h
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,9 @@ void lookupVisibleMemberDecls(VisibleDeclConsumer &Consumer,

namespace namelookup {

void extractDirectlyReferencedNominalTypes(
Type type, SmallVectorImpl<NominalTypeDecl *> &decls);

/// Once name lookup has gathered a set of results, perform any necessary
/// steps to prune the result set before returning it to the caller.
void pruneLookupResultSet(const DeclContext *dc, NLOptions options,
Expand Down
9 changes: 7 additions & 2 deletions include/swift/ClangImporter/ClangImporter.h
Original file line number Diff line number Diff line change
Expand Up @@ -517,8 +517,13 @@ class ClangImporter final : public ClangModuleLoader {
/// Imports a clang decl directly, rather than looking up it's name.
Decl *importDeclDirectly(const clang::NamedDecl *decl) override;

/// Emits any pending diagnostics associated with the provided decl.
void diagnoseDeclDirectly(const clang::NamedDecl *decl) override;
/// Emits diagnostics for any declarations named name
/// whose direct declaration context is a TU.
void diagnoseTopLevelValue(const DeclName &name) override;

/// Emit diagnostics for declarations named name that are members
/// of the provided baseType.
void diagnoseMemberValue(const DeclName &name, const Type &baseType) override;
};

ImportDecl *createImportDecl(ASTContext &Ctx, DeclContext *DC, ClangNode ClangN,
Expand Down
4 changes: 0 additions & 4 deletions lib/AST/ASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2503,10 +2503,6 @@ ASTContext::getInheritedConformance(Type type, ProtocolConformance *inherited) {
return result;
}

bool ASTContext::isLazyContext(const DeclContext *dc) {
return getImpl().LazyContexts.count(dc) != 0;
}

LazyContextData *ASTContext::getOrCreateLazyContextData(
const DeclContext *dc,
LazyMemberLoader *lazyLoader) {
Expand Down
17 changes: 4 additions & 13 deletions lib/AST/NameLookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1475,16 +1475,6 @@ DirectLookupRequest::evaluate(Evaluator &evaluator,
// Look for a declaration with this name.
auto known = Table.find(name);
if (known == Table.end()) {
// Diagnose the missing member if:
// - The flag enabling ClangImporter diagnostics is passed.
// - The containing decl is a ClangDecl.
// - The containing decl (and DeclContext) is lazy.
if (ctx.LangOpts.EnableExperimentalClangImporterDiagnostics &&
ctx.isLazyContext(decl) && decl->getDecl()->getClangDecl()) {
auto ci =
ctx.getOrCreateLazyIterableContextData(decl, /*lazyLoader=*/nullptr);
ci->loader->diagnoseMissingNamedMember(decl, name);
}
return TinyPtrVector<ValueDecl *>();
}

Expand Down Expand Up @@ -1601,8 +1591,8 @@ void namelookup::pruneLookupResultSet(const DeclContext *dc, NLOptions options,

/// Inspect the given type to determine which nominal type declarations it
/// directly references, to facilitate name lookup into those types.
static void extractDirectlyReferencedNominalTypes(
Type type, SmallVectorImpl<NominalTypeDecl *> &decls) {
void namelookup::extractDirectlyReferencedNominalTypes(
Type type, SmallVectorImpl<NominalTypeDecl *> &decls) {
if (auto nominal = type->getAnyNominal()) {
decls.push_back(nominal);
return;
Expand Down Expand Up @@ -1674,7 +1664,8 @@ bool DeclContext::lookupQualified(Type type,

// Figure out which nominal types we will look into.
SmallVector<NominalTypeDecl *, 4> nominalTypesToLookInto;
extractDirectlyReferencedNominalTypes(type, nominalTypesToLookInto);
namelookup::extractDirectlyReferencedNominalTypes(type,
nominalTypesToLookInto);

return lookupQualified(nominalTypesToLookInto, member, options, decls);
}
Expand Down
94 changes: 50 additions & 44 deletions lib/ClangImporter/ClangImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3275,11 +3275,6 @@ void ClangModuleUnit::lookupValue(DeclName name, NLKind lookupKind,
if (auto lookupTable = owner.findLookupTable(clangModule)) {
// Search it.
owner.lookupValue(*lookupTable, name, *consumer);
if (getASTContext().LangOpts.EnableExperimentalClangImporterDiagnostics) {
if (results.empty()) {
owner.diagnoseValue(*lookupTable, name);
}
}
}
}

Expand Down Expand Up @@ -4127,7 +4122,7 @@ void ClangImporter::Implementation::lookupVisibleDecls(
DeclBaseName name = baseName.toDeclBaseName(SwiftContext);
if (!lookupValue(table, name, consumer) &&
SwiftContext.LangOpts.EnableExperimentalEagerClangModuleDiagnostics) {
diagnoseValue(table, name);
diagnoseTopLevelValue(name);
}
}
}
Expand Down Expand Up @@ -4183,30 +4178,42 @@ void ClangImporter::Implementation::lookupAllObjCMembers(
}
}

void ClangImporter::Implementation::diagnoseValue(SwiftLookupTable &table,
DeclName name) {
auto &clangCtx = getClangASTContext();
auto clangTU = clangCtx.getTranslationUnitDecl();

if (name.isOperator()) {
for (auto entry : table.lookupMemberOperators(name.getBaseName())) {
diagnoseTargetDirectly(entry);
void ClangImporter::Implementation::diagnoseTopLevelValue(
const DeclName &name) {
forEachLookupTable([&](SwiftLookupTable &table) -> bool {
for (const auto &entry :
table.lookup(name.getBaseName(),
EffectiveClangContext(
getClangASTContext().getTranslationUnitDecl()))) {
diagnoseTargetDirectly(importDiagnosticTargetFromLookupTableEntry(entry));
}
}
return false;
});
}

for (auto entry : table.lookup(name.getBaseName(), clangTU)) {
diagnoseTargetDirectly(importDiagnosticTargetFromLookupTableEntry(entry));
}
void ClangImporter::Implementation::diagnoseMemberValue(
const DeclName &name, const clang::DeclContext *container) {
forEachLookupTable([&](SwiftLookupTable &table) -> bool {
for (const auto &entry :
table.lookup(name.getBaseName(), EffectiveClangContext(container))) {
if (clang::NamedDecl *nd = entry.get<clang::NamedDecl *>()) {
// We are only interested in members of a particular context,
// skip other contexts.
if (nd->getDeclContext() != container)
continue;

diagnoseTargetDirectly(
importDiagnosticTargetFromLookupTableEntry(entry));
}
// If the entry is not a NamedDecl, it is a form of macro, which cannot be
// a member value.
}
return false;
});
}

void ClangImporter::Implementation::diagnoseTargetDirectly(
ImportDiagnosticTarget target) {
if (!(SwiftContext.LangOpts.EnableExperimentalClangImporterDiagnostics ||
SwiftContext.LangOpts.EnableExperimentalEagerClangModuleDiagnostics) ||
DiagnosedValues.count(target))
return;

DiagnosedValues.insert(target);
if (const clang::Decl *decl = target.dyn_cast<const clang::Decl *>()) {
Walker.TraverseDecl(const_cast<clang::Decl *>(decl));
} else if (const clang::MacroInfo *macro =
Expand Down Expand Up @@ -4363,8 +4370,6 @@ TinyPtrVector<ValueDecl *> ClangRecordMemberLookup::evaluate(
recordDecl->getClangDecl()) {
if (auto import = ctx.getClangModuleLoader()->importDeclDirectly(named))
result.push_back(cast<ValueDecl>(import));
else if (ctx.LangOpts.EnableExperimentalClangImporterDiagnostics)
ctx.getClangModuleLoader()->diagnoseDeclDirectly(named);
}
}

Expand Down Expand Up @@ -4497,22 +4502,6 @@ ClangImporter::Implementation::loadNamedMembers(
return Members;
}

void ClangImporter::Implementation::diagnoseMissingNamedMember(
const IterableDeclContext *IDC, DeclName name) {
Optional<clang::Module *> containingModule =
getClangSubmoduleForDecl(IDC->getDecl()->getClangDecl());
assert(containingModule &&
"Members should never be loaded from a forward-declared decl");
auto allResults = evaluateOrDefault(
SwiftContext.evaluator,
ClangDirectLookupRequest({const_cast<Decl *>(IDC->getDecl()),
IDC->getDecl()->getClangDecl(), name}),
{});
for (auto entry : allResults) {
diagnoseTargetDirectly(importDiagnosticTargetFromLookupTableEntry(entry));
}
}

EffectiveClangContext ClangImporter::Implementation::getEffectiveClangContext(
const NominalTypeDecl *nominal) {
// If we have a Clang declaration, look at it to determine the
Expand Down Expand Up @@ -4747,6 +4736,23 @@ Decl *ClangImporter::importDeclDirectly(const clang::NamedDecl *decl) {
return Impl.importDecl(decl, Impl.CurrentVersion);
}

void ClangImporter::diagnoseDeclDirectly(const clang::NamedDecl *decl) {
Impl.diagnoseTargetDirectly(decl);
void ClangImporter::diagnoseTopLevelValue(const DeclName &name) {
Impl.diagnoseTopLevelValue(name);
}

void ClangImporter::diagnoseMemberValue(const DeclName &name,
const Type &baseType) {
if (!baseType->getAnyNominal())
Copy link
Contributor Author

@NuriAmari NuriAmari Jan 29, 2022

Choose a reason for hiding this comment

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

Couple changes here. Since this call is made from Sema, the baseType could be anything (not just an imported ClangType). I discovered a few more checks are needed here.

First, extractDirectlyReferencedNominalTypes only knows how to handle small set of types of types. If you pass something it isn't expecting, we hit an llvm_unreachable. The getAnyNominal check takes care of this (I figure all imported types referencable from swift are nominal).

Lastly, I added checks that getClangDecl() returns a non-null value, and that this value is a clang::DeclContext. Otherwise, if either of these checks fail, but the value is passed to cast<clang::DeclContext>() anyways, it'll throw.

return;

SmallVector<NominalTypeDecl *, 4> nominalTypesToLookInto;
namelookup::extractDirectlyReferencedNominalTypes(baseType,
nominalTypesToLookInto);
for (auto containerDecl : nominalTypesToLookInto) {
const clang::Decl *clangContainerDecl = containerDecl->getClangDecl();
if (clangContainerDecl && isa<clang::DeclContext>(clangContainerDecl)) {
Impl.diagnoseMemberValue(name,
cast<clang::DeclContext>(clangContainerDecl));
}
}
}
18 changes: 8 additions & 10 deletions lib/ClangImporter/ImporterImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -404,11 +404,6 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
std::unordered_set<ImportDiagnostic, ImportDiagnosticHasher>
CollectedDiagnostics;

// ClangNodes for which all import diagnostics have been both collected and
// emitted.
std::unordered_set<ImportDiagnosticTarget, ImportDiagnosticTargetHasher>
DiagnosedValues;

const bool ImportForwardDeclarations;
const bool DisableSwiftBridgeAttr;
const bool BridgingHeaderExplicitlyRequested;
Expand Down Expand Up @@ -1455,9 +1450,6 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
loadNamedMembers(const IterableDeclContext *IDC, DeclBaseName N,
uint64_t contextData) override;

virtual void diagnoseMissingNamedMember(const IterableDeclContext *IDC,
DeclName name) override;

private:
void
loadAllMembersOfObjcContainer(Decl *D,
Expand Down Expand Up @@ -1600,8 +1592,14 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
void lookupAllObjCMembers(SwiftLookupTable &table,
VisibleDeclConsumer &consumer);

/// Emit any import diagnostics associated with the given name.
void diagnoseValue(SwiftLookupTable &table, DeclName name);
/// Emits diagnostics for any declarations named name
/// whose direct declaration context is a TU.
void diagnoseTopLevelValue(const DeclName &name);

/// Emit diagnostics for declarations named name that are members
/// of the provided container.
void diagnoseMemberValue(const DeclName &name,
const clang::DeclContext *container);

/// Emit any import diagnostics associated with the given Clang node.
void diagnoseTargetDirectly(ImportDiagnosticTarget target);
Expand Down
5 changes: 5 additions & 0 deletions lib/Sema/CSDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3626,6 +3626,11 @@ bool MissingMemberFailure::diagnoseAsError() {
emitDiagnostic(diagnostic, baseType, getName())
.highlight(getSourceRange())
.highlight(nameLoc.getSourceRange());
const auto &ctx = getSolution().getDC()->getASTContext();
if (ctx.LangOpts.EnableExperimentalClangImporterDiagnostics) {
ctx.getClangModuleLoader()->diagnoseMemberValue(getName().getFullName(),
baseType);
}
};

TypoCorrectionResults corrections(getName(), nameLoc);
Expand Down
4 changes: 4 additions & 0 deletions lib/Sema/PreCheckExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,10 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE,
.diagnose(Loc, diag::cannot_find_in_scope, Name,
Name.isOperator())
.highlight(UDRE->getSourceRange());
if (Context.LangOpts.EnableExperimentalClangImporterDiagnostics) {
Context.getClangModuleLoader()->diagnoseTopLevelValue(
Name.getFullName());
}
};

if (!isConfused) {
Expand Down
7 changes: 0 additions & 7 deletions lib/Serialization/Deserialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6402,13 +6402,6 @@ void ModuleFile::loadAllMembers(Decl *container, uint64_t contextData) {
}
}

void ModuleFile::diagnoseMissingNamedMember(const IterableDeclContext *IDC,
DeclName name) {
// TODO: Implement diagnostics for failed member lookups from module files.
llvm_unreachable(
"Missing member diangosis is not implemented for module files.");
}

static llvm::Error consumeErrorIfXRefNonLoadedModule(llvm::Error &&error) {
// Missing module errors are most likely caused by an
// implementation-only import hiding types and decls.
Expand Down
3 changes: 0 additions & 3 deletions lib/Serialization/ModuleFile.h
Original file line number Diff line number Diff line change
Expand Up @@ -699,9 +699,6 @@ class ModuleFile
virtual void loadAllMembers(Decl *D,
uint64_t contextData) override;

virtual void diagnoseMissingNamedMember(const IterableDeclContext *IDC,
DeclName name) override;

virtual TinyPtrVector<ValueDecl *>
loadNamedMembers(const IterableDeclContext *IDC, DeclBaseName N,
uint64_t contextData) override;
Expand Down
5 changes: 5 additions & 0 deletions test/ClangImporter/Inputs/custom-modules/CommonName.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
struct MyStruct {
int _Complex commonName;
};

_Complex int commonName();
5 changes: 5 additions & 0 deletions test/ClangImporter/Inputs/custom-modules/module.map
Original file line number Diff line number Diff line change
Expand Up @@ -266,3 +266,8 @@ module LocalVsFileScope {
header "LocalVsFileScope.h"
export *
}

module CommonName {
header "CommonName.h"
export *
}
Loading