Skip to content

Commit ccc5e18

Browse files
committed
[SourceKit] Support location info for macro-expanded Clang imports
Currently, when we jump-to-definition for decls that are macro-expanded from Clang imported decls (e.g., safe overloads generated by @_SwiftifyImport), setLocationInfo() emits a bongus location pointing to a generated buffer, leading the IDE to try to jump to a file that does not exist. The root cause here is that setLocationInfo() calls getOriginalRange() (earlier, getOriginalLocation()), which was not written to account for such cases where a macro is generated from another generated buffer whose kind is 'AttributeFromClang'. This patch fixes setLocationInfo() with some refactoring that addresses some issues with getOriginal{Range,Location}() that make it resistant to a more localized patch: - getOriginalRange() is inlined into setLocationInfo(), so that the generated buffer-handling logic is localized to that function. This includes how it handles buffers containing ReplacedFunctionBody source. - getOriginalLocation() is used in a couple of other places that don't actually care about the original location, only the outermost buffer a macro is expanded from. The "macro-chasing" logic is now extracted to SourceManager::getMacroUnexpandedBufferID(); the method is moved to from ModuleDecl to SourceManager because there's no reason for it to be in ModuleDecl. - GeneratedSourceInfo now carries an extra ClangNode field, which is populated by getClangSwiftAttrSourceFile() when constructing a generated buffer for an 'AttributeFromClang'. This could probably be union'ed with one or more of the other fields in the future. rdar://151020332
1 parent 6c3865c commit ccc5e18

File tree

11 files changed

+205
-131
lines changed

11 files changed

+205
-131
lines changed

include/swift/AST/Module.h

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -434,19 +434,6 @@ class ModuleDecl
434434
/// \c nullptr if the source location isn't in this module.
435435
SourceFile *getSourceFileContainingLocation(SourceLoc loc);
436436

437-
// Retrieve the buffer ID and source range of the outermost node that
438-
// caused the generation of the buffer containing \p range. \p range and its
439-
// buffer if it isn't in a generated buffer or has no original range.
440-
std::pair<unsigned, SourceRange> getOriginalRange(SourceRange range) const;
441-
442-
// Retrieve the buffer ID and source location of the outermost location that
443-
// caused the generation of the buffer containing \p loc. \p loc and its
444-
// buffer if it isn't in a generated buffer or has no original location.
445-
std::pair<unsigned, SourceLoc> getOriginalLocation(SourceLoc loc) const {
446-
auto [buffer, range] = getOriginalRange(loc);
447-
return std::make_pair(buffer, range.Start);
448-
}
449-
450437
/// Creates a map from \c #filePath strings to corresponding \c #fileID
451438
/// strings, diagnosing any conflicts.
452439
///

include/swift/Basic/SourceManager.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#ifndef SWIFT_BASIC_SOURCEMANAGER_H
1414
#define SWIFT_BASIC_SOURCEMANAGER_H
1515

16+
#include "swift/AST/ClangNode.h"
1617
#include "swift/Basic/FileSystem.h"
1718
#include "swift/Basic/SourceLoc.h"
1819
#include "clang/Basic/FileManager.h"
@@ -22,6 +23,7 @@
2223
#include "llvm/Support/SourceMgr.h"
2324
#include <map>
2425
#include <optional>
26+
#include <utility>
2527
#include <vector>
2628

2729
namespace swift {
@@ -122,6 +124,10 @@ class GeneratedSourceInfo {
122124
/// Contains the ancestors of this source buffer, starting with the root source
123125
/// buffer and ending at this source buffer.
124126
mutable llvm::ArrayRef<unsigned> ancestors = llvm::ArrayRef<unsigned>();
127+
128+
/// Clang node where this buffer comes from. This should be set when this is
129+
/// an 'AttributeFromClang'.
130+
ClangNode clangNode = ClangNode();
125131
};
126132

127133
/// This class manages and owns source buffers.
@@ -554,6 +560,10 @@ class SourceManager {
554560
/// \p _SwiftifyImport macro.
555561
bool isImportMacroGeneratedLoc(SourceLoc loc);
556562

563+
/// Find the outermost buffer ID \p BufID was originally generated from.
564+
/// Returns \p BufID if it wasn't generated from a macro.
565+
unsigned getMacroUnexpandedBufferID(unsigned BufID) const;
566+
557567
private:
558568
int getLineOffset(SourceLoc Loc) const {
559569
if (auto VFile = getVirtualFile(Loc))

lib/AST/Module.cpp

Lines changed: 3 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -847,49 +847,6 @@ SourceFile *ModuleDecl::getSourceFileContainingLocation(SourceLoc loc) {
847847
return nullptr;
848848
}
849849

850-
std::pair<unsigned, SourceRange>
851-
ModuleDecl::getOriginalRange(SourceRange range) const {
852-
assert(range.isValid());
853-
854-
SourceManager &SM = getASTContext().SourceMgr;
855-
unsigned bufferID = SM.findBufferContainingLoc(range.Start);
856-
857-
auto startRange = range;
858-
unsigned startBufferID = bufferID;
859-
while (const GeneratedSourceInfo *info =
860-
SM.getGeneratedSourceInfo(bufferID)) {
861-
switch (info->kind) {
862-
#define MACRO_ROLE(Name, Description) \
863-
case GeneratedSourceInfo::Name##MacroExpansion:
864-
#include "swift/Basic/MacroRoles.def"
865-
{
866-
// Location was within a macro expansion, return the expansion site, not
867-
// the insertion location.
868-
if (info->attachedMacroCustomAttr) {
869-
range = info->attachedMacroCustomAttr->getRange();
870-
} else {
871-
ASTNode expansionNode = ASTNode::getFromOpaqueValue(info->astNode);
872-
range = expansionNode.getSourceRange();
873-
}
874-
bufferID = SM.findBufferContainingLoc(range.Start);
875-
break;
876-
}
877-
case GeneratedSourceInfo::DefaultArgument:
878-
// No original location as it's not actually in any source file
879-
case GeneratedSourceInfo::ReplacedFunctionBody:
880-
// There's not really any "original" location for locations within
881-
// replaced function bodies. The body is actually different code to the
882-
// original file.
883-
case GeneratedSourceInfo::PrettyPrinted:
884-
case GeneratedSourceInfo::AttributeFromClang:
885-
// No original location, return the original buffer/location
886-
return {startBufferID, startRange};
887-
}
888-
}
889-
890-
return {bufferID, range};
891-
}
892-
893850
ArrayRef<SourceFile *>
894851
PrimarySourceFilesRequest::evaluate(Evaluator &evaluator,
895852
ModuleDecl *mod) const {
@@ -1430,13 +1387,9 @@ SourceFile::getExternalRawLocsForDecl(const Decl *D) const {
14301387
SourceManager &SM = getASTContext().SourceMgr;
14311388
bool InGeneratedBuffer =
14321389
!SM.rangeContainsTokenLoc(SM.getRangeForBuffer(BufferID), MainLoc);
1433-
if (InGeneratedBuffer) {
1434-
unsigned UnderlyingBufferID;
1435-
std::tie(UnderlyingBufferID, MainLoc) =
1436-
D->getModuleContext()->getOriginalLocation(MainLoc);
1437-
if (BufferID != UnderlyingBufferID)
1438-
return std::nullopt;
1439-
}
1390+
if (InGeneratedBuffer && BufferID != SM.getMacroUnexpandedBufferID(
1391+
SM.findBufferContainingLoc(MainLoc)))
1392+
return std::nullopt;
14401393

14411394
auto setLoc = [&](ExternalSourceLocs::RawLoc &RawLoc, SourceLoc Loc) {
14421395
if (!Loc.isValid())

lib/Basic/SourceLoc.cpp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -882,3 +882,26 @@ bool SourceManager::isImportMacroGeneratedLoc(SourceLoc loc) {
882882

883883
return false;
884884
}
885+
886+
unsigned SourceManager::getMacroUnexpandedBufferID(unsigned bufferID) const {
887+
auto original = bufferID;
888+
while (const auto *info = getGeneratedSourceInfo(bufferID)) {
889+
switch (info->kind) {
890+
#define MACRO_ROLE(Name, Description) \
891+
case GeneratedSourceInfo::Name##MacroExpansion:
892+
#include "swift/Basic/MacroRoles.def"
893+
if (auto *customAttr = info->attachedMacroCustomAttr)
894+
bufferID = findBufferContainingLoc(customAttr->getStartLoc());
895+
else
896+
bufferID = findBufferContainingLoc(
897+
ASTNode::getFromOpaqueValue(info->astNode).getStartLoc());
898+
continue;
899+
case GeneratedSourceInfo::ReplacedFunctionBody:
900+
case GeneratedSourceInfo::PrettyPrinted:
901+
case GeneratedSourceInfo::DefaultArgument:
902+
case GeneratedSourceInfo::AttributeFromClang:
903+
return original;
904+
}
905+
}
906+
return bufferID;
907+
}

lib/ClangImporter/ImportDecl.cpp

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
#include "swift/Basic/Defer.h"
4747
#include "swift/Basic/PrettyStackTrace.h"
4848
#include "swift/Basic/SourceLoc.h"
49+
#include "swift/Basic/SourceManager.h"
4950
#include "swift/Basic/Statistic.h"
5051
#include "swift/Basic/StringExtras.h"
5152
#include "swift/Basic/Version.h"
@@ -8669,17 +8670,16 @@ bool importer::hasSameUnderlyingType(const clang::Type *a,
86698670
}
86708671

86718672
SourceFile &ClangImporter::Implementation::getClangSwiftAttrSourceFile(
8672-
ModuleDecl &module,
8673-
StringRef attributeText,
8674-
bool cached
8675-
) {
8673+
Decl *MappedDecl, StringRef attributeText, bool cached) {
8674+
auto *module = MappedDecl->getDeclContext()->getParentModule();
8675+
86768676
::TinyPtrVector<SourceFile *> *sourceFiles = nullptr;
86778677
if (cached) {
86788678
sourceFiles = &ClangSwiftAttrSourceFiles[attributeText];
86798679

86808680
// Check whether we've already created a source file.
86818681
for (auto sourceFile : *sourceFiles) {
8682-
if (sourceFile->getParentModule() == &module)
8682+
if (sourceFile->getParentModule() == module)
86838683
return *sourceFile;
86848684
}
86858685
}
@@ -8689,20 +8689,17 @@ SourceFile &ClangImporter::Implementation::getClangSwiftAttrSourceFile(
86898689
auto &sourceMgr = SwiftContext.SourceMgr;
86908690
auto bufferID = sourceMgr.addMemBufferCopy(attributeText);
86918691

8692-
// Note that this is for an attribute.
8693-
sourceMgr.setGeneratedSourceInfo(
8694-
bufferID,
8695-
{
8696-
GeneratedSourceInfo::AttributeFromClang,
8697-
CharSourceRange(),
8698-
sourceMgr.getRangeForBuffer(bufferID),
8699-
&module
8700-
}
8701-
);
8692+
auto info = GeneratedSourceInfo{GeneratedSourceInfo::AttributeFromClang,
8693+
CharSourceRange(),
8694+
sourceMgr.getRangeForBuffer(bufferID)};
8695+
info.astNode = static_cast<void *>(module);
8696+
info.clangNode = MappedDecl->getClangNode();
8697+
8698+
sourceMgr.setGeneratedSourceInfo(bufferID, info);
87028699

87038700
// Create the source file.
8704-
auto sourceFile = new (SwiftContext)
8705-
SourceFile(module, SourceFileKind::Library, bufferID);
8701+
auto sourceFile =
8702+
new (SwiftContext) SourceFile(*module, SourceFileKind::Library, bufferID);
87068703

87078704
if (cached)
87088705
sourceFiles->push_back(sourceFile);
@@ -8725,8 +8722,8 @@ void ClangImporter::Implementation::importNontrivialAttribute(
87258722
bool cached = true;
87268723
while (true) {
87278724
// Dig out a source file we can use for parsing.
8728-
auto &sourceFile = getClangSwiftAttrSourceFile(
8729-
*MappedDecl->getDeclContext()->getParentModule(), AttrString, cached);
8725+
auto &sourceFile =
8726+
getClangSwiftAttrSourceFile(MappedDecl, AttrString, cached);
87308727

87318728
auto topLevelDecls = sourceFile.getTopLevelDecls();
87328729

lib/ClangImporter/ImporterImpl.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1065,9 +1065,9 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
10651065
StringRef getSwiftNameFromClangName(StringRef name);
10661066

10671067
/// Retrieve the placeholder source file for use in parsing Swift attributes
1068-
/// in the given module.
1069-
SourceFile &getClangSwiftAttrSourceFile(
1070-
ModuleDecl &module, StringRef attributeText, bool cached);
1068+
/// of the given Decl.
1069+
SourceFile &getClangSwiftAttrSourceFile(Decl *MappedDecl,
1070+
StringRef attributeText, bool cached);
10711071

10721072
/// Create attribute with given text and attach it to decl, creating or
10731073
/// retrieving a chached source file as needed.

lib/Index/Index.cpp

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1092,12 +1092,11 @@ class IndexSwiftASTWalker : public SourceEntityWalker {
10921092
bool inGeneratedBuffer =
10931093
!SrcMgr.rangeContainsTokenLoc(SrcMgr.getRangeForBuffer(bufferID), loc);
10941094

1095-
if (inGeneratedBuffer) {
1096-
std::tie(bufferID, loc) = CurrentModule->getOriginalLocation(loc);
1097-
if (BufferID.value() != bufferID) {
1098-
assert(false && "Location is not within file being indexed");
1099-
return std::nullopt;
1100-
}
1095+
if (inGeneratedBuffer &&
1096+
bufferID != SrcMgr.getMacroUnexpandedBufferID(
1097+
SrcMgr.findBufferContainingLoc(loc))) {
1098+
assert(false && "Location is not within file being indexed");
1099+
return std::nullopt;
11011100
}
11021101

11031102
auto [line, col] = SrcMgr.getLineAndColumnInBuffer(loc, bufferID);
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
#pragma once
2+
3+
#define __counted_by(x) __attribute__((__counted_by__(x)))
4+
#define __noescape __attribute__((noescape))
5+
#define __lifetimebound __attribute__((lifetimebound))
6+
7+
void hasBufferOverload(int len, int * __counted_by(len) p);
8+
9+
void hasSpanOverload(int len, int * __counted_by(len) __noescape p);
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
module FromClang {
2+
header "from-clang.h"
3+
export *
4+
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// REQUIRES: swift_feature_SafeInteropWrappers
2+
// REQUIRES: swift_feature_LifetimeDependence
3+
4+
import FromClang
5+
6+
// RUN: %target-swift-ide-test \
7+
// RUN: -print-module -module-to-print=FromClang -source-filename=x \
8+
// RUN: -plugin-path %swift-plugin-dir -I %S/Inputs \
9+
// RUN: -enable-experimental-feature SafeInteropWrappers \
10+
// RUN: -enable-experimental-feature LifetimeDependence \
11+
// RUN: | %FileCheck %s --check-prefix INTERFACE
12+
13+
// The macro-generated interface we're looking up source info for
14+
// (this is more so for documentation than checking correctness)
15+
//
16+
// INTERFACE: @_alwaysEmitIntoClient @_disfavoredOverload public func hasBufferOverload(_ p: UnsafeMutableBufferPointer<Int32>)
17+
// INTERFACE: @lifetime(p: copy p)
18+
// INTERFACE-NEXT: @_alwaysEmitIntoClient @_disfavoredOverload public func hasSpanOverload(_ p: inout MutableSpan<Int32>)
19+
20+
// RUN: %sourcekitd-test -req=cursor -pos=26:3 %s -- -I %S/Inputs %s \
21+
// RUN: -enable-experimental-feature SafeInteropWrappers \
22+
// RUN: -enable-experimental-feature LifetimeDependence \
23+
// RUN: | %FileCheck %s --check-prefix CLANG-HEADER
24+
@inlinable
25+
public func callWithBufferPtr(_ p: UnsafeMutableBufferPointer<CInt>) {
26+
hasBufferOverload(p)
27+
}
28+
29+
// RUN: %sourcekitd-test -req=cursor -pos=37:3 %s -- -I %S/Inputs %s \
30+
// RUN: -enable-experimental-feature SafeInteropWrappers \
31+
// RUN: -enable-experimental-feature LifetimeDependence \
32+
// RUN: | %FileCheck %s --check-prefix CLANG-HEADER
33+
@available(visionOS 1.1, tvOS 12.2, watchOS 5.2, iOS 12.2, macOS 10.14.4, *)
34+
@lifetime(p: copy p)
35+
@inlinable
36+
public func callReturnLifetimeBound(_ p: inout MutableSpan<CInt>) {
37+
hasSpanOverload(p)
38+
}
39+
40+
// Check that we get the file path of the header that Clang imported:
41+
// CLANG-HEADER: /Inputs/from-clang.h:

0 commit comments

Comments
 (0)