Skip to content

Commit 86e708a

Browse files
author
Gabor Horvath
committed
[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.
1 parent 8e2a033 commit 86e708a

File tree

6 files changed

+52
-17
lines changed

6 files changed

+52
-17
lines changed

lib/AST/Decl.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5502,8 +5502,12 @@ SourceRange GenericTypeParamDecl::getSourceRange() const {
55025502
if (const auto specifierLoc = getSpecifierLoc())
55035503
startLoc = specifierLoc;
55045504

5505-
if (!getInherited().empty())
5506-
endLoc = getInherited().getEndLoc();
5505+
if (!getInherited().empty()) {
5506+
if (getInherited().getEndLoc().isValid())
5507+
endLoc = getInherited().getEndLoc();
5508+
else
5509+
assert(startLoc.isInvalid() || this->hasClangNode());
5510+
}
55075511

55085512
return {startLoc, endLoc};
55095513
}

lib/ClangImporter/ClangImporter.cpp

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
#include "swift/Basic/Defer.h"
4343
#include "swift/Basic/Platform.h"
4444
#include "swift/Basic/Range.h"
45+
#include "swift/Basic/SourceLoc.h"
4546
#include "swift/Basic/StringExtras.h"
4647
#include "swift/Basic/Version.h"
4748
#include "swift/ClangImporter/ClangImporterRequests.h"
@@ -2860,14 +2861,12 @@ ClangImporter::Implementation::exportSourceLoc(SourceLoc loc) {
28602861

28612862
SourceLoc
28622863
ClangImporter::Implementation::importSourceLoc(clang::SourceLocation loc) {
2863-
// FIXME: Implement!
2864-
return SourceLoc();
2864+
return BuffersForDiagnostics.resolveSourceLocation(Instance->getSourceManager(), loc);
28652865
}
28662866

28672867
SourceRange
2868-
ClangImporter::Implementation::importSourceRange(clang::SourceRange loc) {
2869-
// FIXME: Implement!
2870-
return SourceRange();
2868+
ClangImporter::Implementation::importSourceRange(clang::SourceRange range) {
2869+
return SourceRange(importSourceLoc(range.getBegin()), importSourceLoc(range.getEnd()));
28712870
}
28722871

28732872
#pragma mark Importing names
@@ -5981,9 +5980,10 @@ cloneBaseMemberDecl(ValueDecl *decl, DeclContext *newContext) {
59815980
if (auto typeAlias = dyn_cast<TypeAliasDecl>(decl)) {
59825981
auto rawMemory = allocateMemoryForDecl<TypeAliasDecl>(
59835982
typeAlias->getASTContext(), sizeof(TypeAliasDecl), false);
5984-
auto out = new (rawMemory) TypeAliasDecl(
5985-
typeAlias->getLoc(), typeAlias->getEqualLoc(), typeAlias->getName(),
5986-
typeAlias->getNameLoc(), typeAlias->getGenericParams(), newContext);
5983+
auto out = new (rawMemory)
5984+
TypeAliasDecl(typeAlias->getStartLoc(), typeAlias->getEqualLoc(),
5985+
typeAlias->getName(), typeAlias->getNameLoc(),
5986+
typeAlias->getGenericParams(), newContext);
59875987
out->setUnderlyingType(typeAlias->getUnderlyingType());
59885988
out->copyFormalAccessFrom(typeAlias);
59895989
return out;

lib/ClangImporter/ImportDecl.cpp

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
#include "swift/Basic/Assertions.h"
4444
#include "swift/Basic/Defer.h"
4545
#include "swift/Basic/PrettyStackTrace.h"
46+
#include "swift/Basic/SourceLoc.h"
4647
#include "swift/Basic/Statistic.h"
4748
#include "swift/Basic/StringExtras.h"
4849
#include "swift/Basic/Version.h"
@@ -1661,6 +1662,8 @@ namespace {
16611662
// Create the wrapper struct.
16621663
errorWrapper =
16631664
new (C) StructDecl(loc, name, loc, std::nullopt, nullptr, dc);
1665+
SourceLoc end = Impl.importSourceLoc(decl->getEndLoc());
1666+
errorWrapper->setBraces(SourceRange(loc, end));
16641667
errorWrapper->setAccess(AccessLevel::Public);
16651668
errorWrapper->getAttrs().add(
16661669
new (Impl.SwiftContext) FrozenAttr(/*IsImplicit*/true));
@@ -3606,9 +3609,9 @@ namespace {
36063609
auto *typeParam = Impl.createDeclWithClangNode<GenericTypeParamDecl>(
36073610
param, AccessLevel::Public, dc,
36083611
Impl.SwiftContext.getIdentifier(param->getName()),
3609-
/*nameLoc*/ SourceLoc(), /*specifierLoc*/ SourceLoc(),
3610-
/*depth*/ 0, /*index*/ i,
3611-
GenericTypeParamKind::Type);
3612+
/*nameLoc*/ Impl.importSourceLoc(param->getLocation()),
3613+
/*specifierLoc*/ SourceLoc(),
3614+
/*depth*/ 0, /*index*/ i, GenericTypeParamKind::Type);
36123615
templateParams.push_back(typeParam);
36133616
(void)++i;
36143617
}
@@ -6468,7 +6471,8 @@ Decl *SwiftDeclConverter::importGlobalAsInitializer(
64686471
}
64696472

64706473
auto result = Impl.createDeclWithClangNode<ConstructorDecl>(
6471-
decl, AccessLevel::Public, name, /*NameLoc=*/SourceLoc(), failable,
6474+
decl, AccessLevel::Public, name,
6475+
Impl.importSourceLoc(decl->getLocation()), failable,
64726476
/*FailabilityLoc=*/SourceLoc(),
64736477
/*Async=*/false, /*AsyncLoc=*/SourceLoc(),
64746478
/*Throws=*/false, /*ThrowsLoc=*/SourceLoc(), /*ThrownType=*/TypeLoc(),
@@ -6983,7 +6987,8 @@ ConstructorDecl *SwiftDeclConverter::importConstructor(
69836987
assert(!importedName.getAsyncInfo());
69846988
auto result = Impl.createDeclWithClangNode<ConstructorDecl>(
69856989
objcMethod, AccessLevel::Public, importedName.getDeclName(),
6986-
/*NameLoc=*/SourceLoc(), failability, /*FailabilityLoc=*/SourceLoc(),
6990+
/*NameLoc=*/Impl.importSourceLoc(objcMethod->getLocation()), failability,
6991+
/*FailabilityLoc=*/SourceLoc(),
69876992
/*Async=*/false, /*AsyncLoc=*/SourceLoc(),
69886993
/*Throws=*/importedName.getErrorInfo().has_value(),
69896994
/*ThrowsLoc=*/SourceLoc(), /*ThrownType=*/TypeLoc(), bodyParams,

lib/ClangImporter/ImportType.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2766,7 +2766,15 @@ ParameterList *ClangImporter::Implementation::importFunctionParameterList(
27662766
}
27672767

27682768
// Form the parameter list.
2769-
return ParameterList::create(SwiftContext, parameters);
2769+
// Estimate locations for the begin and end of parameter list.
2770+
auto begin = clangDecl->getLocation();
2771+
auto end = begin;
2772+
if (!params.empty()) {
2773+
begin = params.front()->getBeginLoc();
2774+
end = params.back()->getEndLoc();
2775+
}
2776+
return ParameterList::create(SwiftContext, importSourceLoc(begin), parameters,
2777+
importSourceLoc(end));
27702778
}
27712779

27722780
static bool isObjCMethodResultAudited(const clang::Decl *decl) {

lib/ClangImporter/ImporterImpl.h

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,14 @@
2323
#include "ImportName.h"
2424
#include "SwiftLookupTable.h"
2525
#include "swift/AST/ASTContext.h"
26+
#include "swift/AST/Decl.h"
2627
#include "swift/AST/ForeignErrorConvention.h"
2728
#include "swift/AST/LazyResolver.h"
2829
#include "swift/AST/Module.h"
2930
#include "swift/AST/RequirementSignature.h"
3031
#include "swift/AST/Type.h"
3132
#include "swift/Basic/FileTypes.h"
33+
#include "swift/Basic/SourceLoc.h"
3234
#include "swift/Basic/StringExtras.h"
3335
#include "swift/ClangImporter/ClangImporter.h"
3436
#include "swift/ClangImporter/ClangModule.h"
@@ -1715,6 +1717,20 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
17151717
if (auto ASD = dyn_cast<AbstractStorageDecl>(D))
17161718
ASD->setSetterAccess(access);
17171719

1720+
if constexpr (std::is_base_of_v<NominalTypeDecl, DeclTy>) {
1721+
// Estimate brace locations.
1722+
auto begin = ClangN.getAsDecl()->getBeginLoc();
1723+
auto end = ClangN.getAsDecl()->getEndLoc();
1724+
SourceRange range;
1725+
if (begin.isValid() && end.isValid() && D->getNameLoc().isValid())
1726+
range = SourceRange(importSourceLoc(begin), importSourceLoc(end));
1727+
else {
1728+
range = SourceRange(D->getNameLoc(), D->getNameLoc());
1729+
}
1730+
assert(range.isValid() == D->getNameLoc().isValid());
1731+
D->setBraces(range);
1732+
}
1733+
17181734
// SwiftAttrs on ParamDecls are interpreted by applyParamAttributes().
17191735
if (!isa<ParamDecl>(D))
17201736
importSwiftAttrAttributes(D);

lib/Sema/TypeCheckMacros.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1354,8 +1354,10 @@ static SourceFile *evaluateAttachedMacro(MacroDecl *macro, Decl *attachedTo,
13541354
// If the declaration has no source location and comes from a Clang module,
13551355
// pretty-print the declaration and use that location.
13561356
SourceLoc attachedToLoc = attachedTo->getLoc();
1357+
bool isPrettyPrintedDecl = false;
13571358
if (attachedToLoc.isInvalid() &&
13581359
isa<ClangModuleUnit>(dc->getModuleScopeContext())) {
1360+
isPrettyPrintedDecl = true;
13591361
attachedToLoc = evaluateOrDefault(
13601362
ctx.evaluator, PrettyPrintDeclRequest{attachedTo}, SourceLoc());
13611363
}
@@ -1501,7 +1503,7 @@ static SourceFile *evaluateAttachedMacro(MacroDecl *macro, Decl *attachedTo,
15011503
searchDecl = var->getParentPatternBinding();
15021504

15031505
auto startLoc = searchDecl->getStartLoc();
1504-
if (startLoc.isInvalid() && isa<ClangModuleUnit>(dc->getModuleScopeContext())) {
1506+
if (isPrettyPrintedDecl) {
15051507
startLoc = attachedToLoc;
15061508
}
15071509

0 commit comments

Comments
 (0)