Skip to content

Commit e1d8a23

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 72615fe commit e1d8a23

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"
@@ -2854,14 +2855,12 @@ ClangImporter::Implementation::exportSourceLoc(SourceLoc loc) {
28542855

28552856
SourceLoc
28562857
ClangImporter::Implementation::importSourceLoc(clang::SourceLocation loc) {
2857-
// FIXME: Implement!
2858-
return SourceLoc();
2858+
return BuffersForDiagnostics.resolveSourceLocation(Instance->getSourceManager(), loc);
28592859
}
28602860

28612861
SourceRange
2862-
ClangImporter::Implementation::importSourceRange(clang::SourceRange loc) {
2863-
// FIXME: Implement!
2864-
return SourceRange();
2862+
ClangImporter::Implementation::importSourceRange(clang::SourceRange range) {
2863+
return SourceRange(importSourceLoc(range.getBegin()), importSourceLoc(range.getEnd()));
28652864
}
28662865

28672866
#pragma mark Importing names
@@ -5975,9 +5974,10 @@ cloneBaseMemberDecl(ValueDecl *decl, DeclContext *newContext) {
59755974
if (auto typeAlias = dyn_cast<TypeAliasDecl>(decl)) {
59765975
auto rawMemory = allocateMemoryForDecl<TypeAliasDecl>(
59775976
typeAlias->getASTContext(), sizeof(TypeAliasDecl), false);
5978-
auto out = new (rawMemory) TypeAliasDecl(
5979-
typeAlias->getLoc(), typeAlias->getEqualLoc(), typeAlias->getName(),
5980-
typeAlias->getNameLoc(), typeAlias->getGenericParams(), newContext);
5977+
auto out = new (rawMemory)
5978+
TypeAliasDecl(typeAlias->getStartLoc(), typeAlias->getEqualLoc(),
5979+
typeAlias->getName(), typeAlias->getNameLoc(),
5980+
typeAlias->getGenericParams(), newContext);
59815981
out->setUnderlyingType(typeAlias->getUnderlyingType());
59825982
out->copyFormalAccessFrom(typeAlias);
59835983
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"
@@ -1680,6 +1681,8 @@ namespace {
16801681
// Create the wrapper struct.
16811682
errorWrapper =
16821683
new (C) StructDecl(loc, name, loc, std::nullopt, nullptr, dc);
1684+
SourceLoc end = Impl.importSourceLoc(decl->getEndLoc());
1685+
errorWrapper->setBraces(SourceRange(loc, end));
16831686
errorWrapper->setAccess(AccessLevel::Public);
16841687
errorWrapper->getAttrs().add(
16851688
new (Impl.SwiftContext) FrozenAttr(/*IsImplicit*/true));
@@ -3625,9 +3628,9 @@ namespace {
36253628
auto *typeParam = Impl.createDeclWithClangNode<GenericTypeParamDecl>(
36263629
param, AccessLevel::Public, dc,
36273630
Impl.SwiftContext.getIdentifier(param->getName()),
3628-
/*nameLoc*/ SourceLoc(), /*specifierLoc*/ SourceLoc(),
3629-
/*depth*/ 0, /*index*/ i,
3630-
GenericTypeParamKind::Type);
3631+
/*nameLoc*/ Impl.importSourceLoc(param->getLocation()),
3632+
/*specifierLoc*/ SourceLoc(),
3633+
/*depth*/ 0, /*index*/ i, GenericTypeParamKind::Type);
36313634
templateParams.push_back(typeParam);
36323635
(void)++i;
36333636
}
@@ -6491,7 +6494,8 @@ Decl *SwiftDeclConverter::importGlobalAsInitializer(
64916494
}
64926495

64936496
auto result = Impl.createDeclWithClangNode<ConstructorDecl>(
6494-
decl, AccessLevel::Public, name, /*NameLoc=*/SourceLoc(), failable,
6497+
decl, AccessLevel::Public, name,
6498+
Impl.importSourceLoc(decl->getLocation()), failable,
64956499
/*FailabilityLoc=*/SourceLoc(),
64966500
/*Async=*/false, /*AsyncLoc=*/SourceLoc(),
64976501
/*Throws=*/false, /*ThrowsLoc=*/SourceLoc(), /*ThrownType=*/TypeLoc(),
@@ -7006,7 +7010,8 @@ ConstructorDecl *SwiftDeclConverter::importConstructor(
70067010
assert(!importedName.getAsyncInfo());
70077011
auto result = Impl.createDeclWithClangNode<ConstructorDecl>(
70087012
objcMethod, AccessLevel::Public, importedName.getDeclName(),
7009-
/*NameLoc=*/SourceLoc(), failability, /*FailabilityLoc=*/SourceLoc(),
7013+
/*NameLoc=*/Impl.importSourceLoc(objcMethod->getLocation()), failability,
7014+
/*FailabilityLoc=*/SourceLoc(),
70107015
/*Async=*/false, /*AsyncLoc=*/SourceLoc(),
70117016
/*Throws=*/importedName.getErrorInfo().has_value(),
70127017
/*ThrowsLoc=*/SourceLoc(), /*ThrownType=*/TypeLoc(), bodyParams,

lib/ClangImporter/ImportType.cpp

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

27902790
// Form the parameter list.
2791-
return ParameterList::create(SwiftContext, parameters);
2791+
// Estimate locations for the begin and end of parameter list.
2792+
auto begin = clangDecl->getLocation();
2793+
auto end = begin;
2794+
if (!params.empty()) {
2795+
begin = params.front()->getBeginLoc();
2796+
end = params.back()->getEndLoc();
2797+
}
2798+
return ParameterList::create(SwiftContext, importSourceLoc(begin), parameters,
2799+
importSourceLoc(end));
27922800
}
27932801

27942802
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)