From 86e708a39b9727cd16632919603beba1e4f11bad Mon Sep 17 00:00:00 2001 From: Gabor Horvath Date: Mon, 16 Sep 2024 17:35:04 +0100 Subject: [PATCH] [cxx-interop] Import more C++ source locations into Swift Occasionally, when the Swift compiler emits a diagnostic for a construct that was imported from C++ we get a diagnostic with unknown location. This is a bad user experience. It is particularly bad with the borrow-checker related diagnostics. This patch extends the source location importing to declarations in ClangImporter. There are some invariants enforced by the Swift compile, e.g., a source range is comprised of two valid source locations or two invalid ones. As a result, this patch adds approximate source locations to some separators like braces or parens that are not maintained by Clang. Having slightly incorrect ranges in this case is better than emitting unknown source locations. --- lib/AST/Decl.cpp | 8 ++++++-- lib/ClangImporter/ClangImporter.cpp | 16 ++++++++-------- lib/ClangImporter/ImportDecl.cpp | 15 ++++++++++----- lib/ClangImporter/ImportType.cpp | 10 +++++++++- lib/ClangImporter/ImporterImpl.h | 16 ++++++++++++++++ lib/Sema/TypeCheckMacros.cpp | 4 +++- 6 files changed, 52 insertions(+), 17 deletions(-) diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 4c6cbf788d087..0b8851c62ebe7 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -5502,8 +5502,12 @@ SourceRange GenericTypeParamDecl::getSourceRange() const { if (const auto specifierLoc = getSpecifierLoc()) startLoc = specifierLoc; - if (!getInherited().empty()) - endLoc = getInherited().getEndLoc(); + if (!getInherited().empty()) { + if (getInherited().getEndLoc().isValid()) + endLoc = getInherited().getEndLoc(); + else + assert(startLoc.isInvalid() || this->hasClangNode()); + } return {startLoc, endLoc}; } diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp index aa4da54392079..2fa3932336cf3 100644 --- a/lib/ClangImporter/ClangImporter.cpp +++ b/lib/ClangImporter/ClangImporter.cpp @@ -42,6 +42,7 @@ #include "swift/Basic/Defer.h" #include "swift/Basic/Platform.h" #include "swift/Basic/Range.h" +#include "swift/Basic/SourceLoc.h" #include "swift/Basic/StringExtras.h" #include "swift/Basic/Version.h" #include "swift/ClangImporter/ClangImporterRequests.h" @@ -2860,14 +2861,12 @@ ClangImporter::Implementation::exportSourceLoc(SourceLoc loc) { SourceLoc ClangImporter::Implementation::importSourceLoc(clang::SourceLocation loc) { - // FIXME: Implement! - return SourceLoc(); + return BuffersForDiagnostics.resolveSourceLocation(Instance->getSourceManager(), loc); } SourceRange -ClangImporter::Implementation::importSourceRange(clang::SourceRange loc) { - // FIXME: Implement! - return SourceRange(); +ClangImporter::Implementation::importSourceRange(clang::SourceRange range) { + return SourceRange(importSourceLoc(range.getBegin()), importSourceLoc(range.getEnd())); } #pragma mark Importing names @@ -5981,9 +5980,10 @@ cloneBaseMemberDecl(ValueDecl *decl, DeclContext *newContext) { if (auto typeAlias = dyn_cast(decl)) { auto rawMemory = allocateMemoryForDecl( typeAlias->getASTContext(), sizeof(TypeAliasDecl), false); - auto out = new (rawMemory) TypeAliasDecl( - typeAlias->getLoc(), typeAlias->getEqualLoc(), typeAlias->getName(), - typeAlias->getNameLoc(), typeAlias->getGenericParams(), newContext); + auto out = new (rawMemory) + TypeAliasDecl(typeAlias->getStartLoc(), typeAlias->getEqualLoc(), + typeAlias->getName(), typeAlias->getNameLoc(), + typeAlias->getGenericParams(), newContext); out->setUnderlyingType(typeAlias->getUnderlyingType()); out->copyFormalAccessFrom(typeAlias); return out; diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp index dec0315b3f254..4c88c3d113dbe 100644 --- a/lib/ClangImporter/ImportDecl.cpp +++ b/lib/ClangImporter/ImportDecl.cpp @@ -43,6 +43,7 @@ #include "swift/Basic/Assertions.h" #include "swift/Basic/Defer.h" #include "swift/Basic/PrettyStackTrace.h" +#include "swift/Basic/SourceLoc.h" #include "swift/Basic/Statistic.h" #include "swift/Basic/StringExtras.h" #include "swift/Basic/Version.h" @@ -1661,6 +1662,8 @@ namespace { // Create the wrapper struct. errorWrapper = new (C) StructDecl(loc, name, loc, std::nullopt, nullptr, dc); + SourceLoc end = Impl.importSourceLoc(decl->getEndLoc()); + errorWrapper->setBraces(SourceRange(loc, end)); errorWrapper->setAccess(AccessLevel::Public); errorWrapper->getAttrs().add( new (Impl.SwiftContext) FrozenAttr(/*IsImplicit*/true)); @@ -3606,9 +3609,9 @@ namespace { auto *typeParam = Impl.createDeclWithClangNode( param, AccessLevel::Public, dc, Impl.SwiftContext.getIdentifier(param->getName()), - /*nameLoc*/ SourceLoc(), /*specifierLoc*/ SourceLoc(), - /*depth*/ 0, /*index*/ i, - GenericTypeParamKind::Type); + /*nameLoc*/ Impl.importSourceLoc(param->getLocation()), + /*specifierLoc*/ SourceLoc(), + /*depth*/ 0, /*index*/ i, GenericTypeParamKind::Type); templateParams.push_back(typeParam); (void)++i; } @@ -6468,7 +6471,8 @@ Decl *SwiftDeclConverter::importGlobalAsInitializer( } auto result = Impl.createDeclWithClangNode( - decl, AccessLevel::Public, name, /*NameLoc=*/SourceLoc(), failable, + decl, AccessLevel::Public, name, + Impl.importSourceLoc(decl->getLocation()), failable, /*FailabilityLoc=*/SourceLoc(), /*Async=*/false, /*AsyncLoc=*/SourceLoc(), /*Throws=*/false, /*ThrowsLoc=*/SourceLoc(), /*ThrownType=*/TypeLoc(), @@ -6983,7 +6987,8 @@ ConstructorDecl *SwiftDeclConverter::importConstructor( assert(!importedName.getAsyncInfo()); auto result = Impl.createDeclWithClangNode( objcMethod, AccessLevel::Public, importedName.getDeclName(), - /*NameLoc=*/SourceLoc(), failability, /*FailabilityLoc=*/SourceLoc(), + /*NameLoc=*/Impl.importSourceLoc(objcMethod->getLocation()), failability, + /*FailabilityLoc=*/SourceLoc(), /*Async=*/false, /*AsyncLoc=*/SourceLoc(), /*Throws=*/importedName.getErrorInfo().has_value(), /*ThrowsLoc=*/SourceLoc(), /*ThrownType=*/TypeLoc(), bodyParams, diff --git a/lib/ClangImporter/ImportType.cpp b/lib/ClangImporter/ImportType.cpp index 289e78222fefe..763eae72d4d90 100644 --- a/lib/ClangImporter/ImportType.cpp +++ b/lib/ClangImporter/ImportType.cpp @@ -2766,7 +2766,15 @@ ParameterList *ClangImporter::Implementation::importFunctionParameterList( } // Form the parameter list. - return ParameterList::create(SwiftContext, parameters); + // Estimate locations for the begin and end of parameter list. + auto begin = clangDecl->getLocation(); + auto end = begin; + if (!params.empty()) { + begin = params.front()->getBeginLoc(); + end = params.back()->getEndLoc(); + } + return ParameterList::create(SwiftContext, importSourceLoc(begin), parameters, + importSourceLoc(end)); } static bool isObjCMethodResultAudited(const clang::Decl *decl) { diff --git a/lib/ClangImporter/ImporterImpl.h b/lib/ClangImporter/ImporterImpl.h index 18939159e273a..b0360910df3cc 100644 --- a/lib/ClangImporter/ImporterImpl.h +++ b/lib/ClangImporter/ImporterImpl.h @@ -23,12 +23,14 @@ #include "ImportName.h" #include "SwiftLookupTable.h" #include "swift/AST/ASTContext.h" +#include "swift/AST/Decl.h" #include "swift/AST/ForeignErrorConvention.h" #include "swift/AST/LazyResolver.h" #include "swift/AST/Module.h" #include "swift/AST/RequirementSignature.h" #include "swift/AST/Type.h" #include "swift/Basic/FileTypes.h" +#include "swift/Basic/SourceLoc.h" #include "swift/Basic/StringExtras.h" #include "swift/ClangImporter/ClangImporter.h" #include "swift/ClangImporter/ClangModule.h" @@ -1715,6 +1717,20 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation if (auto ASD = dyn_cast(D)) ASD->setSetterAccess(access); + if constexpr (std::is_base_of_v) { + // Estimate brace locations. + auto begin = ClangN.getAsDecl()->getBeginLoc(); + auto end = ClangN.getAsDecl()->getEndLoc(); + SourceRange range; + if (begin.isValid() && end.isValid() && D->getNameLoc().isValid()) + range = SourceRange(importSourceLoc(begin), importSourceLoc(end)); + else { + range = SourceRange(D->getNameLoc(), D->getNameLoc()); + } + assert(range.isValid() == D->getNameLoc().isValid()); + D->setBraces(range); + } + // SwiftAttrs on ParamDecls are interpreted by applyParamAttributes(). if (!isa(D)) importSwiftAttrAttributes(D); diff --git a/lib/Sema/TypeCheckMacros.cpp b/lib/Sema/TypeCheckMacros.cpp index 435f37e4225b2..4a842652c86f0 100644 --- a/lib/Sema/TypeCheckMacros.cpp +++ b/lib/Sema/TypeCheckMacros.cpp @@ -1354,8 +1354,10 @@ static SourceFile *evaluateAttachedMacro(MacroDecl *macro, Decl *attachedTo, // If the declaration has no source location and comes from a Clang module, // pretty-print the declaration and use that location. SourceLoc attachedToLoc = attachedTo->getLoc(); + bool isPrettyPrintedDecl = false; if (attachedToLoc.isInvalid() && isa(dc->getModuleScopeContext())) { + isPrettyPrintedDecl = true; attachedToLoc = evaluateOrDefault( ctx.evaluator, PrettyPrintDeclRequest{attachedTo}, SourceLoc()); } @@ -1501,7 +1503,7 @@ static SourceFile *evaluateAttachedMacro(MacroDecl *macro, Decl *attachedTo, searchDecl = var->getParentPatternBinding(); auto startLoc = searchDecl->getStartLoc(); - if (startLoc.isInvalid() && isa(dc->getModuleScopeContext())) { + if (isPrettyPrintedDecl) { startLoc = attachedToLoc; }