Skip to content

Ensure that SourceFiles always have a backing buffer in the SourceManager #76512

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.

Sign up for GitHub

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 4 commits into from
Sep 18, 2024
Merged

Ensure that SourceFiles always have a backing buffer in the SourceManager #76512

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4e7fb73
Provide the buffer ID for source files created for C swift_attr attri…
DougGregor Sep 10, 2024
49aa0e9
Ensure that SourceFiles always have a backing buffer in the SourceMan…
DougGregor Sep 17, 2024
031be42
Ensure that we don't get a very-likely-wrong buffer ID
DougGregor Sep 17, 2024
0bd5195
Silence warning about mismatched signs
DougGregor Sep 17, 2024
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
10 changes: 3 additions & 7 deletions include/swift/AST/SourceFile.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,7 @@ class SourceFile final : public FileUnit {
std::optional<StableHasher> InterfaceHasher;

/// The ID for the memory buffer containing this file's source.
///
/// May be -1, to indicate no association with a buffer.
int BufferID;
unsigned BufferID;

/// The parsing options for the file.
ParsingOptions ParsingOpts;
Expand Down Expand Up @@ -370,7 +368,7 @@ class SourceFile final : public FileUnit {
/// \c #sourceLocation(file:) declarations.
llvm::StringMap<SourceFilePathInfo> getInfoForUsedFilePaths() const;

SourceFile(ModuleDecl &M, SourceFileKind K, std::optional<unsigned> bufferID,
SourceFile(ModuleDecl &M, SourceFileKind K, unsigned bufferID,
ParsingOptions parsingOpts = {}, bool isPrimary = false);

~SourceFile();
Expand Down Expand Up @@ -590,9 +588,7 @@ class SourceFile final : public FileUnit {

/// The buffer ID for the file that was imported, or None if there
/// is no associated buffer.
std::optional<unsigned> getBufferID() const {
if (BufferID == -1)
return std::nullopt;
unsigned getBufferID() const {
return BufferID;
}

Expand Down
2 changes: 1 addition & 1 deletion include/swift/Frontend/Frontend.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,7 @@ class CompilerInstance {
/// Creates a new source file for the main module.
SourceFile *createSourceFileForMainModule(ModuleDecl *mod,
SourceFileKind FileKind,
std::optional<unsigned> BufferID,
unsigned BufferID,
bool isMainBuffer = false) const;

/// Creates all the files to be added to the main module, appending them to
Expand Down
4 changes: 2 additions & 2 deletions include/swift/IDE/IDERequests.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ struct CursorInfoOwner {
return !(lhs == rhs);
}
bool isValid() const {
return File && File->getBufferID() && Loc.isValid();
return File && Loc.isValid();
}
};

Expand Down Expand Up @@ -110,7 +110,7 @@ struct RangeInfoOwner {
}

bool isValid() const {
return File && File->getBufferID() && StartLoc.isValid() && EndLoc.isValid();
return File && StartLoc.isValid() && EndLoc.isValid();
}
};

Expand Down
2 changes: 1 addition & 1 deletion include/swift/Migrator/ASTMigratorPass.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class ASTMigratorPass {
ASTMigratorPass(EditorAdapter &Editor, SourceFile *SF,
const MigratorOptions &Opts)
: Editor(Editor), SF(SF), Opts(Opts), Filename(SF->getFilename()),
BufferID(SF->getBufferID().value()),
BufferID(SF->getBufferID()),
SM(SF->getASTContext().SourceMgr), Diags(SF->getASTContext().Diags) {}
};

Expand Down
3 changes: 1 addition & 2 deletions lib/AST/ASTContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6622,9 +6622,8 @@ void VarDecl::setOriginalWrappedProperty(VarDecl *originalProperty) {
static bool isSourceLocInOrignalBuffer(const Decl *D, SourceLoc Loc) {
assert(Loc.isValid());
auto bufferID = D->getDeclContext()->getParentSourceFile()->getBufferID();
assert(bufferID.has_value() && "Source buffer ID must be set");
auto &SM = D->getASTContext().SourceMgr;
return SM.getRangeForBuffer(*bufferID).contains(Loc);
return SM.getRangeForBuffer(bufferID).contains(Loc);
}
#endif

Expand Down
2 changes: 1 addition & 1 deletion lib/AST/ASTScopeCreation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ ASTSourceFileScope::ASTSourceFileScope(SourceFile *SF,

if (SF->Kind == SourceFileKind::DefaultArgument) {
auto genInfo = *SF->getASTContext().SourceMgr.getGeneratedSourceInfo(
*SF->getBufferID());
SF->getBufferID());
parentLoc = ASTNode::getFromOpaqueValue(genInfo.astNode).getStartLoc();
if (auto parentScope =
findStartingScopeForLookup(enclosingSF, parentLoc)) {
Expand Down
7 changes: 1 addition & 6 deletions lib/AST/ASTScopePrinting.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,8 @@ void ASTScopeImpl::dump() const { print(llvm::errs(), 0, false); }
void ASTScopeImpl::dumpOneScopeMapLocation(
std::pair<unsigned, unsigned> lineColumn) {
auto bufferID = getSourceFile()->getBufferID();
if (!bufferID) {
llvm::errs() << "***No buffer, dumping all scopes***";
print(llvm::errs());
return;
}
SourceLoc loc = getSourceManager().getLocForLineCol(
*bufferID, lineColumn.first, lineColumn.second);
bufferID, lineColumn.first, lineColumn.second);
if (loc.isInvalid())
return;

Expand Down
14 changes: 3 additions & 11 deletions lib/AST/ASTScopeSourceRange.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,17 +203,9 @@ SourceRange GenericParamScope::getSourceRangeOfThisASTNode(

SourceRange ASTSourceFileScope::getSourceRangeOfThisASTNode(
const bool omitAssertions) const {
if (auto bufferID = SF->getBufferID()) {
auto charRange = getSourceManager().getRangeForBuffer(*bufferID);
return SourceRange(charRange.getStart(), charRange.getEnd());
}

if (SF->getTopLevelItems().empty())
return SourceRange();

// Use the source ranges of the declarations in the file.
return SourceRange(SF->getTopLevelItems().front().getStartLoc(),
SF->getTopLevelItems().back().getEndLoc());
auto bufferID = SF->getBufferID();
auto charRange = getSourceManager().getRangeForBuffer(bufferID);
return SourceRange(charRange.getStart(), charRange.getEnd());
}

SourceRange GenericTypeOrExtensionScope::getSourceRangeOfThisASTNode(
Expand Down
4 changes: 1 addition & 3 deletions lib/AST/DiagnosticEngine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1759,9 +1759,7 @@ BufferIndirectlyCausingDiagnosticRAII::BufferIndirectlyCausingDiagnosticRAII(
const SourceFile &SF)
: Diags(SF.getASTContext().Diags) {
auto id = SF.getBufferID();
if (!id)
return;
auto loc = SF.getASTContext().SourceMgr.getLocForBufferStart(*id);
auto loc = SF.getASTContext().SourceMgr.getLocForBufferStart(id);
if (loc.isValid())
Diags.setBufferIndirectlyCausingDiagnosticToInput(loc);
}
Expand Down
44 changes: 20 additions & 24 deletions lib/AST/Module.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -775,8 +775,8 @@ namespace {
SourceManager *sourceMgr;

bool operator()(SourceFile *lhs, SourceFile *rhs) const {
auto lhsRange = sourceMgr->getRangeForBuffer(*lhs->getBufferID());
auto rhsRange = sourceMgr->getRangeForBuffer(*rhs->getBufferID());
auto lhsRange = sourceMgr->getRangeForBuffer(lhs->getBufferID());
auto rhsRange = sourceMgr->getRangeForBuffer(rhs->getBufferID());

std::less<const char *> pointerCompare;
return pointerCompare(
Expand All @@ -785,7 +785,7 @@ namespace {
}

bool operator()(SourceFile *lhs, SourceLoc rhsLoc) const {
auto lhsRange = sourceMgr->getRangeForBuffer(*lhs->getBufferID());
auto lhsRange = sourceMgr->getRangeForBuffer(lhs->getBufferID());

std::less<const char *> pointerCompare;
return pointerCompare(
Expand All @@ -794,7 +794,7 @@ namespace {
}

bool operator()(SourceLoc lhsLoc, SourceFile *rhs) const {
auto rhsRange = sourceMgr->getRangeForBuffer(*rhs->getBufferID());
auto rhsRange = sourceMgr->getRangeForBuffer(rhs->getBufferID());

std::less<const char *> pointerCompare;
return pointerCompare(
Expand Down Expand Up @@ -835,8 +835,7 @@ void ModuleDecl::updateSourceFileLocationMap() {
// First, add all of the source files with a backing buffer.
for (auto *fileUnit : files) {
if (auto sourceFile = dyn_cast<SourceFile>(fileUnit)) {
if (sourceFile->getBufferID())
sourceFileLocationMap->allSourceFiles.push_back(sourceFile);
sourceFileLocationMap->allSourceFiles.push_back(sourceFile);
}
}

Expand Down Expand Up @@ -872,7 +871,7 @@ SourceFile *ModuleDecl::getSourceFileContainingLocation(SourceLoc loc) {
// in to see if it contains this.
if (sourceFileLocationMap) {
if (auto lastSourceFile = sourceFileLocationMap->lastSourceFile) {
auto range = sourceMgr.getRangeForBuffer(*lastSourceFile->getBufferID());
auto range = sourceMgr.getRangeForBuffer(lastSourceFile->getBufferID());
if (range.contains(adjustedLoc))
return lastSourceFile;
}
Expand All @@ -888,7 +887,7 @@ SourceFile *ModuleDecl::getSourceFileContainingLocation(SourceLoc loc) {
return nullptr;

auto foundSourceFile = *found;
auto foundRange = sourceMgr.getRangeForBuffer(*foundSourceFile->getBufferID());
auto foundRange = sourceMgr.getRangeForBuffer(foundSourceFile->getBufferID());
// Positions inside an empty file or at EOF should still be considered within
// this file.
if (!foundRange.contains(adjustedLoc) && adjustedLoc != foundRange.getEnd())
Expand Down Expand Up @@ -1192,7 +1191,7 @@ SourceRange SourceFile::getMacroInsertionRange() const {
return SourceRange();

auto generatedInfo =
*getASTContext().SourceMgr.getGeneratedSourceInfo(*getBufferID());
*getASTContext().SourceMgr.getGeneratedSourceInfo(getBufferID());
auto origRange = generatedInfo.originalSourceRange;
return {origRange.getStart(), origRange.getEnd()};
}
Expand All @@ -1202,7 +1201,7 @@ CustomAttr *SourceFile::getAttachedMacroAttribute() const {
return nullptr;

auto genInfo =
*getASTContext().SourceMgr.getGeneratedSourceInfo(*getBufferID());
*getASTContext().SourceMgr.getGeneratedSourceInfo(getBufferID());
return genInfo.attachedMacroCustomAttr;
}

Expand All @@ -1211,7 +1210,7 @@ std::optional<MacroRole> SourceFile::getFulfilledMacroRole() const {
return std::nullopt;

auto genInfo =
*getASTContext().SourceMgr.getGeneratedSourceInfo(*getBufferID());
*getASTContext().SourceMgr.getGeneratedSourceInfo(getBufferID());
switch (genInfo.kind) {
#define MACRO_ROLE(Name, Description) \
case GeneratedSourceInfo::Name##MacroExpansion: \
Expand All @@ -1231,7 +1230,7 @@ SourceFile *SourceFile::getEnclosingSourceFile() const {
return nullptr;

auto genInfo =
*getASTContext().SourceMgr.getGeneratedSourceInfo(*getBufferID());
*getASTContext().SourceMgr.getGeneratedSourceInfo(getBufferID());
auto sourceLoc = genInfo.originalSourceRange.getStart();
return getParentModule()->getSourceFileContainingLocation(sourceLoc);
}
Expand All @@ -1242,7 +1241,7 @@ ASTNode SourceFile::getNodeInEnclosingSourceFile() const {
return nullptr;

auto genInfo =
*getASTContext().SourceMgr.getGeneratedSourceInfo(*getBufferID());
*getASTContext().SourceMgr.getGeneratedSourceInfo(getBufferID());
return ASTNode::getFromOpaqueValue(genInfo.astNode);
}

Expand Down Expand Up @@ -1470,7 +1469,7 @@ SourceFile::getExternalRawLocsForDecl(const Decl *D) const {
bool InGeneratedBuffer =
!SM.rangeContainsTokenLoc(SM.getRangeForBuffer(BufferID), MainLoc);
if (InGeneratedBuffer) {
int UnderlyingBufferID;
unsigned UnderlyingBufferID;
std::tie(UnderlyingBufferID, MainLoc) =
D->getModuleContext()->getOriginalLocation(MainLoc);
if (BufferID != UnderlyingBufferID)
Expand Down Expand Up @@ -2108,8 +2107,8 @@ bool ModuleDecl::registerEntryPointFile(
if (existingDecl) {
existingDiagLoc = sourceFile->getMainDeclDiagLoc();
} else {
if (auto bufID = sourceFile->getBufferID())
existingDiagLoc = getASTContext().SourceMgr.getLocForBufferStart(*bufID);
auto bufID = sourceFile->getBufferID();
existingDiagLoc = getASTContext().SourceMgr.getLocForBufferStart(bufID);
}
}

Expand Down Expand Up @@ -3296,10 +3295,8 @@ llvm::StringMap<SourceFilePathInfo>
SourceFile::getInfoForUsedFilePaths() const {
llvm::StringMap<SourceFilePathInfo> result;

if (BufferID != -1) {
result[getFilename()].physicalFileLoc =
getASTContext().SourceMgr.getLocForBufferStart(BufferID);
}
result[getFilename()].physicalFileLoc =
getASTContext().SourceMgr.getLocForBufferStart(BufferID);

for (auto &vpath : VirtualFilePaths) {
result[vpath.Item].virtualFileLocs.insert(vpath.Loc);
Expand Down Expand Up @@ -3433,10 +3430,11 @@ ModuleDecl::computeFileIDMap(bool shouldDiagnose) const {
}

SourceFile::SourceFile(ModuleDecl &M, SourceFileKind K,
std::optional<unsigned> bufferID,
unsigned bufferID,
ParsingOptions parsingOpts, bool isPrimary)
: FileUnit(FileUnitKind::Source, M), BufferID(bufferID ? *bufferID : -1),
: FileUnit(FileUnitKind::Source, M), BufferID(bufferID),
ParsingOpts(parsingOpts), IsPrimary(isPrimary), Kind(K) {
assert(BufferID != (unsigned)~0);
M.getASTContext().addDestructorCleanup(*this);
Copy link
Contributor

@CodaFi CodaFi Sep 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Should this assert that the buffer ID isn't ~0 or is that now valid (if... very unlikely to be encountered in the wild)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea!


assert(!IsPrimary || M.isMainModule() &&
Expand Down Expand Up @@ -3643,8 +3641,6 @@ bool SourceFile::walk(ASTWalker &walker) {
}

StringRef SourceFile::getFilename() const {
if (BufferID == -1)
return "";
SourceManager &SM = getASTContext().SourceMgr;
return SM.getIdentifierForBuffer(BufferID);
}
Expand Down
2 changes: 1 addition & 1 deletion lib/AST/TypeRefinementContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ TypeRefinementContext::createForSourceFile(SourceFile *SF,
// root context should be nested under.
if (auto parentTRC =
SF->getEnclosingSourceFile()->getTypeRefinementContext()) {
auto charRange = Ctx.SourceMgr.getRangeForBuffer(*SF->getBufferID());
auto charRange = Ctx.SourceMgr.getRangeForBuffer(SF->getBufferID());
range = SourceRange(charRange.getStart(), charRange.getEnd());
auto originalNode = SF->getNodeInEnclosingSourceFile();
parentContext =
Expand Down
10 changes: 7 additions & 3 deletions lib/ClangImporter/ImportDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8063,14 +8063,18 @@ unsigned ClangImporter::Implementation::getClangSwiftAttrSourceBuffer(
}

SourceFile &ClangImporter::Implementation::getClangSwiftAttrSourceFile(
ModuleDecl &module) {
ModuleDecl &module, unsigned bufferID) {
auto known = ClangSwiftAttrSourceFiles.find(&module);
if (known != ClangSwiftAttrSourceFiles.end())
return *known->second;

auto sourceFile = new (SwiftContext)
SourceFile(module, SourceFileKind::Library, std::nullopt);
SourceFile(module, SourceFileKind::Library, bufferID);
ClangSwiftAttrSourceFiles.insert({&module, sourceFile});

// Record this attribute in the module.
module.addAuxiliaryFile(*sourceFile);

return *sourceFile;
}

Expand Down Expand Up @@ -8228,7 +8232,7 @@ ClangImporter::Implementation::importSwiftAttrAttributes(Decl *MappedDecl) {

// Dig out a source file we can use for parsing.
auto &sourceFile = getClangSwiftAttrSourceFile(
*MappedDecl->getDeclContext()->getParentModule());
*MappedDecl->getDeclContext()->getParentModule(), bufferID);

// Spin up a parser.
swift::Parser parser(
Expand Down
2 changes: 1 addition & 1 deletion lib/ClangImporter/ImporterImpl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1062,7 +1062,7 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation

/// Retrieve the placeholder source file for use in parsing Swift attributes
/// in the given module.
SourceFile &getClangSwiftAttrSourceFile(ModuleDecl &module);
SourceFile &getClangSwiftAttrSourceFile(ModuleDecl &module, unsigned bufferID);

/// Utility function to import Clang attributes from a source Swift decl to
/// synthesized Swift decl.
Expand Down
10 changes: 2 additions & 8 deletions lib/Frontend/DependencyVerifier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,13 +324,7 @@ class DependencyVerifier {

bool DependencyVerifier::parseExpectations(
const SourceFile *SF, std::vector<Expectation> &Expectations) {
const auto MaybeBufferID = SF->getBufferID();
if (!MaybeBufferID) {
llvm::errs() << "source file has no buffer: " << SF->getFilename();
return true;
}

const auto BufferID = MaybeBufferID.value();
const auto BufferID = SF->getBufferID();
const CharSourceRange EntireRange = SM.getRangeForBuffer(BufferID);
const StringRef InputFile = SM.extractText(EntireRange);

Expand Down Expand Up @@ -484,7 +478,7 @@ bool DependencyVerifier::verifyNegativeExpectations(

bool DependencyVerifier::diagnoseUnfulfilledObligations(
const SourceFile *SF, ObligationMap &Obligations) {
CharSourceRange EntireRange = SM.getRangeForBuffer(*SF->getBufferID());
CharSourceRange EntireRange = SM.getRangeForBuffer(SF->getBufferID());
StringRef InputFile = SM.extractText(EntireRange);
auto &diags = SF->getASTContext().Diags;
auto &Ctx = SF->getASTContext();
Expand Down
4 changes: 2 additions & 2 deletions lib/Frontend/Frontend.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1743,9 +1743,9 @@ CompilerInstance::getSourceFileParsingOptions(bool forPrimary) const {
}

SourceFile *CompilerInstance::createSourceFileForMainModule(
ModuleDecl *mod, SourceFileKind fileKind, std::optional<unsigned> bufferID,
ModuleDecl *mod, SourceFileKind fileKind, unsigned bufferID,
bool isMainBuffer) const {
auto isPrimary = bufferID && isPrimaryInput(*bufferID);
auto isPrimary = isPrimaryInput(bufferID);
auto opts = getSourceFileParsingOptions(isPrimary);

auto *inputFile = new (*Context)
Expand Down
7 changes: 2 additions & 5 deletions lib/FrontendTool/FrontendTool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,11 +238,8 @@ static void countStatsOfSourceFile(UnifiedStatsReporter &Stats,
SF->getPrecedenceGroups(groups);
C.NumPrecedenceGroups += groups.size();

auto bufID = SF->getBufferID();
if (bufID.has_value()) {
C.NumSourceLines +=
SM.getEntireTextForBuffer(bufID.value()).count('\n');
}
C.NumSourceLines +=
SM.getEntireTextForBuffer(SF->getBufferID()).count('\n');
}

static void countASTStats(UnifiedStatsReporter &Stats,
Expand Down
2 changes: 1 addition & 1 deletion lib/IDE/Formatting.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3040,7 +3040,7 @@ std::pair<LineRange, std::string> swift::ide::reformat(LineRange Range,
// default value.
Options.TabWidth = Options.IndentWidth ? Options.IndentWidth : 4;
}
auto SourceBufferID = SF.getBufferID().value();
auto SourceBufferID = SF.getBufferID();
StringRef Text = SM.getLLVMSourceMgr()
.getMemoryBuffer(SourceBufferID)->getBuffer();
size_t Offset = getOffsetOfLine(Range.startLine(), Text, /*Trim*/true);
Expand Down
Loading