Skip to content

Commit 014f863

Browse files
committed
SerializeLoc: address more comments from Jordan. NFC
1 parent adaf790 commit 014f863

File tree

10 files changed

+140
-120
lines changed

10 files changed

+140
-120
lines changed

include/swift/Serialization/SerializedModuleLoader.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -89,11 +89,11 @@ class SerializedModuleLoaderBase : public ModuleLoader {
8989
StringRef ModuleDocPath,
9090
std::unique_ptr<llvm::MemoryBuffer> *ModuleDocBuffer);
9191

92-
std::error_code
93-
openModuleSourceInfoFile(AccessPathElem ModuleID,
94-
StringRef ModulePath,
95-
StringRef ModuleSourceInfoFileName,
96-
std::unique_ptr<llvm::MemoryBuffer> *ModuleSourceInfoBuffer);
92+
void
93+
openModuleSourceInfoFileIfPresent(AccessPathElem ModuleID,
94+
StringRef ModulePath,
95+
StringRef ModuleSourceInfoFileName,
96+
std::unique_ptr<llvm::MemoryBuffer> *ModuleSourceInfoBuffer);
9797

9898
/// If the module loader subclass knows that all options have been tried for
9999
/// loading an architecture-specific file out of a swiftmodule bundle, try

lib/Driver/Driver.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2793,7 +2793,7 @@ static void chooseModuleAuxiliaryOutputFilePath(Compilation &C,
27932793
StringRef workingDirectory,
27942794
CommandOutput *Output,
27952795
file_types::ID fileID,
2796-
bool isProject = false,
2796+
bool shouldUseProjectFolder = false,
27972797
Optional<options::ID> optId = llvm::None) {
27982798
if (hasExistingAdditionalOutput(*Output, fileID))
27992799
return;
@@ -2819,7 +2819,7 @@ static void chooseModuleAuxiliaryOutputFilePath(Compilation &C,
28192819
bool isTempFile = C.isTemporaryFile(ModulePath);
28202820
auto ModuleName = llvm::sys::path::filename(ModulePath);
28212821
llvm::SmallString<128> Path(llvm::sys::path::parent_path(ModulePath));
2822-
if (isProject) {
2822+
if (shouldUseProjectFolder) {
28232823
llvm::sys::path::append(Path, "Project");
28242824
// If the build system has created a Project dir for us to include the file, use it.
28252825
if (!llvm::sys::fs::exists(Path)) {
@@ -2840,7 +2840,8 @@ void Driver::chooseSwiftSourceInfoOutputPath(Compilation &C,
28402840
CommandOutput *Output) const {
28412841
chooseModuleAuxiliaryOutputFilePath(C, OutputMap, workingDirectory, Output,
28422842
file_types::TY_SwiftSourceInfoFile,
2843-
/*isProject*/true, options::OPT_emit_module_source_info_path);
2843+
/*shouldUseProjectFolder*/true,
2844+
options::OPT_emit_module_source_info_path);
28442845
}
28452846

28462847
void Driver::chooseSwiftModuleDocOutputPath(Compilation &C,

lib/Frontend/Frontend.cpp

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -520,18 +520,19 @@ Optional<CompilerInstance::ModuleBuffers> CompilerInstance::getInputBuffersIfPre
520520

521521
Optional<std::unique_ptr<llvm::MemoryBuffer>>
522522
CompilerInstance::openModuleSourceInfo(const InputFile &input) {
523-
llvm::SmallString<128> moduleSourceInfoFilePath(input.file());
524-
llvm::sys::path::replace_extension(moduleSourceInfoFilePath,
525-
file_types::getExtension(file_types::TY_SwiftSourceInfoFile));
526-
std::string NonPrivatePath = moduleSourceInfoFilePath.str().str();
527-
StringRef fileName = llvm::sys::path::filename(NonPrivatePath);
528-
llvm::sys::path::remove_filename(moduleSourceInfoFilePath);
529-
llvm::sys::path::append(moduleSourceInfoFilePath, "Project");
530-
llvm::sys::path::append(moduleSourceInfoFilePath, fileName);
523+
llvm::SmallString<128> pathWithoutProjectDir(input.file());
524+
llvm::sys::path::replace_extension(pathWithoutProjectDir,
525+
file_types::getExtension(file_types::TY_SwiftSourceInfoFile));
526+
llvm::SmallString<128> pathWithProjectDir = pathWithoutProjectDir.str();
527+
StringRef fileName = llvm::sys::path::filename(pathWithoutProjectDir);
528+
llvm::sys::path::remove_filename(pathWithProjectDir);
529+
llvm::sys::path::append(pathWithProjectDir, "Project");
530+
llvm::sys::path::append(pathWithProjectDir, fileName);
531531
if (auto sourceInfoFileOrErr = swift::vfs::getFileOrSTDIN(getFileSystem(),
532-
moduleSourceInfoFilePath))
532+
pathWithProjectDir))
533533
return std::move(*sourceInfoFileOrErr);
534-
if (auto sourceInfoFileOrErr = swift::vfs::getFileOrSTDIN(getFileSystem(), NonPrivatePath))
534+
if (auto sourceInfoFileOrErr = swift::vfs::getFileOrSTDIN(getFileSystem(),
535+
pathWithoutProjectDir))
535536
return std::move(*sourceInfoFileOrErr);
536537
return None;
537538
}

lib/Frontend/ModuleInterfaceLoader.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1037,7 +1037,8 @@ std::error_code ModuleInterfaceLoader::findModuleFilesInDirectory(
10371037
*ModuleBuffer = std::move(*ModuleBufferOrErr);
10381038
}
10391039
// Open .swiftsourceinfo file if it's present.
1040-
SerializedModuleLoaderBase::openModuleSourceInfoFile(ModuleID, ModPath,
1040+
SerializedModuleLoaderBase::openModuleSourceInfoFileIfPresent(ModuleID,
1041+
ModPath,
10411042
ModuleSourceInfoFilename,
10421043
ModuleSourceInfoBuffer);
10431044
// Delegate back to the serialized module loader to load the module doc.

lib/Serialization/ModuleFile.cpp

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1224,7 +1224,7 @@ bool ModuleFile::readModuleDocIfPresent() {
12241224
}
12251225

12261226
class ModuleFile::DeclUSRTableInfo {
1227-
public:
1227+
public:
12281228
using internal_key_type = StringRef;
12291229
using external_key_type = StringRef;
12301230
using data_type = uint32_t;
@@ -1238,7 +1238,9 @@ class ModuleFile::DeclUSRTableInfo {
12381238
return llvm::djbHash(key, SWIFTSOURCEINFO_HASH_SEED);
12391239
}
12401240

1241-
static bool EqualKey(internal_key_type lhs, internal_key_type rhs) { return lhs == rhs; }
1241+
static bool EqualKey(internal_key_type lhs, internal_key_type rhs) {
1242+
return lhs == rhs;
1243+
}
12421244

12431245
static std::pair<unsigned, unsigned> ReadKeyDataLength(const uint8_t *&data) {
12441246
unsigned keyLength = endian::readNext<uint32_t, little, unaligned>(data);
@@ -1252,7 +1254,7 @@ class ModuleFile::DeclUSRTableInfo {
12521254

12531255
data_type ReadData(internal_key_type key, const uint8_t *data, unsigned length) {
12541256
assert(length == 4);
1255-
return *reinterpret_cast<const uint32_t*>(data);
1257+
return endian::readNext<uint32_t, little, unaligned>(data);
12561258
}
12571259
};
12581260

@@ -1263,8 +1265,8 @@ ModuleFile::readDeclUSRsTable(ArrayRef<uint64_t> fields, StringRef blobData) {
12631265
uint32_t tableOffset = static_cast<uint32_t>(fields.front());
12641266
auto base = reinterpret_cast<const uint8_t *>(blobData.data());
12651267
return std::unique_ptr<SerializedDeclUSRTable>(
1266-
SerializedDeclUSRTable::Create(base + tableOffset, base + sizeof(uint32_t), base,
1267-
DeclUSRTableInfo()));
1268+
SerializedDeclUSRTable::Create(base + tableOffset, base + sizeof(uint32_t),
1269+
base));
12681270
}
12691271

12701272
bool ModuleFile::readDeclLocsBlock(llvm::BitstreamCursor &cursor) {
@@ -2364,7 +2366,8 @@ Optional<CommentInfo> ModuleFile::getCommentForDecl(const Decl *D) const {
23642366
return getCommentForDeclByUSR(USRBuffer.str());
23652367
}
23662368

2367-
Optional<BasicDeclLocs> ModuleFile::getBasicDeclLocsForDecl(const Decl *D) const {
2369+
Optional<BasicDeclLocs>
2370+
ModuleFile::getBasicDeclLocsForDecl(const Decl *D) const {
23682371
assert(D);
23692372

23702373
// Keep these as assertions instead of early exits to ensure that we are not
@@ -2375,6 +2378,9 @@ Optional<BasicDeclLocs> ModuleFile::getBasicDeclLocsForDecl(const Decl *D) const
23752378
"Decl is from a different serialized file");
23762379
if (!DeclUSRsTable)
23772380
return None;
2381+
// Future compilers may not provide BasicDeclLocsData anymore.
2382+
if (BasicDeclLocsData.empty())
2383+
return None;
23782384
// Compute the USR.
23792385
llvm::SmallString<128> USRBuffer;
23802386
llvm::raw_svector_ostream OS(USRBuffer);
@@ -2394,18 +2400,17 @@ Optional<BasicDeclLocs> ModuleFile::getBasicDeclLocsForDecl(const Decl *D) const
23942400
assert(RecordOffset < BasicDeclLocsData.size());
23952401
assert(BasicDeclLocsData.size() % RecordSize == 0);
23962402
BasicDeclLocs Result;
2397-
auto *Record = reinterpret_cast<const uint32_t*>(BasicDeclLocsData.data() + RecordOffset);
2403+
auto *Record = BasicDeclLocsData.data() + RecordOffset;
23982404
auto ReadNext = [&Record]() {
2399-
uint32_t Result = *Record;
2400-
++ Record;
2401-
return Result;
2405+
return endian::readNext<uint32_t, little, unaligned>(Record);
24022406
};
2407+
24032408
auto FilePath = SourceLocsTextData.substr(ReadNext());
24042409
size_t TerminatorOffset = FilePath.find('\0');
24052410
assert(TerminatorOffset != StringRef::npos && "unterminated string data");
24062411
Result.SourceFilePath = FilePath.slice(0, TerminatorOffset);
2407-
#define READ_FIELD(X) \
2408-
Result.X.Line = ReadNext(); \
2412+
#define READ_FIELD(X) \
2413+
Result.X.Line = ReadNext(); \
24092414
Result.X.Column = ReadNext();
24102415
READ_FIELD(Loc)
24112416
READ_FIELD(StartLoc)

lib/Serialization/ModuleFile.h

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -411,6 +411,13 @@ class ModuleFile
411411
llvm::OnDiskIterableChainedHashTable<DeclUSRTableInfo>;
412412
std::unique_ptr<SerializedDeclUSRTable> DeclUSRsTable;
413413

414+
/// A blob of 0 terminated string segments referenced in \c SourceLocsTextData
415+
StringRef SourceLocsTextData;
416+
417+
/// An array of fixed size source location data for each USR appearing in
418+
/// \c DeclUSRsTable.
419+
StringRef BasicDeclLocsData;
420+
414421
struct ModuleBits {
415422
/// The decl ID of the main class in this module file, if it has one.
416423
unsigned EntryPointDeclID : 31;
@@ -557,14 +564,6 @@ class ModuleFile
557564
/// Returns false if there was an error.
558565
bool readModuleSourceInfoIfPresent();
559566

560-
/// Read an on-disk decl hash table stored in
561-
/// \c sourceinfo_block::BasicDeclLocsLayout format.
562-
StringRef BasicDeclLocsData;
563-
564-
/// Read an on-disk decl hash table stored in
565-
/// \c sourceinfo_block::SourceFilePathsLayout format.
566-
StringRef SourceLocsTextData;
567-
568567
/// Read an on-disk decl hash table stored in
569568
/// \c sourceinfo_block::DeclUSRSLayout format.
570569
std::unique_ptr<SerializedDeclUSRTable>

lib/Serialization/ModuleFormat.h

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -642,14 +642,23 @@ enum BlockID {
642642
/// The module source location container block, which contains all other
643643
/// source location blocks.
644644
///
645-
/// This is part of a stable format and must not be renumbered!
645+
/// This is part of a stable format and should not be renumbered.
646+
///
647+
/// Though we strive to keep the format stable, breaking the format of
648+
/// .swiftsourceinfo doesn't have consequences as serious as breaking the
649+
/// format of .swiftdoc because .swiftsourceinfo file is for local development
650+
/// use only.
646651
MODULE_SOURCEINFO_BLOCK_ID = 192,
647652

648653
/// The source location block, which contains decl locations.
649654
///
650-
/// This is part of a stable format and must not be renumbered!
655+
/// This is part of a stable format and should not be renumbered.
656+
///
657+
/// Though we strive to keep the format stable, breaking the format of
658+
/// .swiftsourceinfo doesn't have consequences as serious as breaking the format
659+
/// of .swiftdoc because .swiftsourceinfo file is for local development use only.
651660
///
652-
/// \sa sourceinfo_block
661+
/// \sa decl_locs_block
653662
DECL_LOCS_BLOCK_ID,
654663
};
655664

lib/Serialization/SerializeDoc.cpp

Lines changed: 43 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,16 @@
1313
#include "DocFormat.h"
1414
#include "Serialization.h"
1515
#include "SourceInfoFormat.h"
16-
#include "swift/Basic/Defer.h"
1716
#include "swift/AST/ASTContext.h"
1817
#include "swift/AST/ASTWalker.h"
1918
#include "swift/AST/DiagnosticsCommon.h"
2019
#include "swift/AST/Module.h"
2120
#include "swift/AST/ParameterList.h"
2221
#include "swift/AST/SourceFile.h"
2322
#include "swift/AST/USRGeneration.h"
23+
#include "swift/Basic/Defer.h"
2424
#include "swift/Basic/SourceManager.h"
25+
#include "llvm/ADT/StringSet.h"
2526
#include "llvm/Support/DJB.h"
2627
#include "llvm/Support/EndianStream.h"
2728
#include "llvm/Support/OnDiskHashTable.h"
@@ -546,25 +547,29 @@ class USRTableInfo {
546547
out << key;
547548
}
548549

549-
void EmitData(raw_ostream &out, key_type_ref key, data_type_ref data, unsigned len) {
550+
void EmitData(raw_ostream &out, key_type_ref key, data_type_ref data,
551+
unsigned len) {
550552
endian::Writer writer(out, little);
551553
writer.write<uint32_t>(data);
552554
}
553555
};
554556

555557
class DeclUSRsTableWriter {
556-
llvm::StringMap<uint32_t> USRMap;
558+
llvm::StringSet<> USRs;
557559
llvm::OnDiskChainedHashTableGenerator<USRTableInfo> generator;
558-
uint32_t CurId = 0;
559560
public:
560-
uint32_t peekNextId() const { return CurId; }
561-
Optional<uint32_t> getNewUSRID(StringRef USR) {
562-
if (USRMap.find(USR) == USRMap.end()) {
563-
generator.insert(USRMap.insert(std::make_pair(USR, CurId)).first->getKey(), CurId);
564-
++CurId;
565-
return USRMap.find(USR)->second;
566-
}
567-
return None;
561+
uint32_t peekNextId() const { return USRs.size(); }
562+
Optional<uint32_t> getNewUSRId(StringRef USR) {
563+
// Attempt to insert the USR into the StringSet.
564+
auto It = USRs.insert(USR);
565+
// If the USR exists in the StringSet, return None.
566+
if (!It.second)
567+
return None;
568+
auto Id = USRs.size() - 1;
569+
// We have to insert the USR from the StringSet because it's where the
570+
// memory is owned.
571+
generator.insert(It.first->getKey(), Id);
572+
return Id;
568573
}
569574
void emitUSRsRecord(llvm::BitstreamWriter &out) {
570575
decl_locs_block::DeclUSRSLayout USRsList(out);
@@ -606,10 +611,11 @@ struct BasicDeclLocsTableWriter : public ASTWalker {
606611
DeclUSRsTableWriter &USRWriter;
607612
StringWriter &FWriter;
608613
BasicDeclLocsTableWriter(DeclUSRsTableWriter &USRWriter,
609-
StringWriter &FWriter): USRWriter(USRWriter), FWriter(FWriter) {}
614+
StringWriter &FWriter): USRWriter(USRWriter),
615+
FWriter(FWriter) {}
610616

611-
std::pair<bool, Stmt *> walkToStmtPre(Stmt *S) override { return { false, S }; }
612-
std::pair<bool, Expr *> walkToExprPre(Expr *E) override { return { false, E }; }
617+
std::pair<bool, Stmt *> walkToStmtPre(Stmt *S) override { return { false, S };}
618+
std::pair<bool, Expr *> walkToExprPre(Expr *E) override { return { false, E };}
613619
bool walkToTypeLocPre(TypeLoc &TL) override { return false; }
614620
bool walkToTypeReprPre(TypeRepr *T) override { return false; }
615621
bool walkToParameterListPre(ParameterList *PL) override { return false; }
@@ -618,8 +624,8 @@ struct BasicDeclLocsTableWriter : public ASTWalker {
618624
llvm::raw_svector_ostream out(Buffer);
619625
endian::Writer writer(out, little);
620626
writer.write<uint32_t>(data.SourceFileOffset);
621-
#define WRITE_LINE_COLUMN(X) \
622-
writer.write<uint32_t>(data.X.Line); \
627+
#define WRITE_LINE_COLUMN(X) \
628+
writer.write<uint32_t>(data.X.Line); \
623629
writer.write<uint32_t>(data.X.Column);
624630
WRITE_LINE_COLUMN(Loc)
625631
WRITE_LINE_COLUMN(StartLoc);
@@ -632,7 +638,7 @@ writer.write<uint32_t>(data.X.Column);
632638
llvm::raw_svector_ostream OS(Buffer);
633639
if (ide::printDeclUSR(D, OS))
634640
return None;
635-
return USRWriter.getNewUSRID(OS.str());
641+
return USRWriter.getNewUSRId(OS.str());
636642
}
637643

638644
LineColumn getLineColumn(SourceManager &SM, SourceLoc Loc) {
@@ -674,8 +680,8 @@ writer.write<uint32_t>(data.X.Column);
674680
return None;
675681
DeclLocationsTableData Result;
676682
Result.SourceFileOffset = FWriter.getTextOffset(Locs->SourceFilePath);
677-
#define COPY_LINE_COLUMN(X) \
678-
Result.X.Line = Locs->X.Line; \
683+
#define COPY_LINE_COLUMN(X) \
684+
Result.X.Line = Locs->X.Line; \
679685
Result.X.Column = Locs->X.Column;
680686
COPY_LINE_COLUMN(Loc)
681687
COPY_LINE_COLUMN(StartLoc)
@@ -693,13 +699,13 @@ Result.X.Column = Locs->X.Column;
693699

694700
bool walkToDeclPre(Decl *D) override {
695701
SWIFT_DEFER {
696-
assert(USRWriter.peekNextId() * sizeof(DeclLocationsTableData) == Buffer.size() &&
697-
"USR Id has a one-to-one mapping with DeclLocationsTableData");
702+
assert(USRWriter.peekNextId() * sizeof(DeclLocationsTableData)
703+
== Buffer.size() &&
704+
"USR Id has a one-to-one mapping with DeclLocationsTableData");
698705
};
699-
// We shouldn't expose any Decls that .swiftdoc file isn't willing to expose.
700-
// .swiftdoc doesn't include comments for double underscored symbols, but for .swiftsourceinfo,
701-
// having the source location for these symbols isn't a concern becuase these
702-
// symbols are in .swiftinterface anyway.
706+
// .swiftdoc doesn't include comments for double underscored symbols, but
707+
// for .swiftsourceinfo, having the source location for these symbols isn't
708+
// a concern becuase these symbols are in .swiftinterface anyway.
703709
if (!shouldIncludeDecl(D, /*ExcludeDoubleUnderscore*/false))
704710
return false;
705711
if (!shouldSerializeSourceLoc(D))
@@ -718,7 +724,8 @@ Result.X.Column = Locs->X.Column;
718724
};
719725

720726
static void emitBasicLocsRecord(llvm::BitstreamWriter &Out,
721-
ModuleOrSourceFile MSF, DeclUSRsTableWriter &USRWriter,
727+
ModuleOrSourceFile MSF,
728+
DeclUSRsTableWriter &USRWriter,
722729
StringWriter &FWriter) {
723730
assert(MSF);
724731
const decl_locs_block::BasicDeclLocsLayout DeclLocsList(Out);
@@ -773,17 +780,19 @@ class SourceInfoSerializer : public SerializerBase {
773780
control_block::TargetLayout Target(Out);
774781

775782
auto& LangOpts = M->getASTContext().LangOpts;
776-
Metadata.emit(ScratchRecord, SWIFTSOURCEINFO_VERSION_MAJOR, SWIFTSOURCEINFO_VERSION_MINOR,
783+
Metadata.emit(ScratchRecord, SWIFTSOURCEINFO_VERSION_MAJOR,
784+
SWIFTSOURCEINFO_VERSION_MINOR,
777785
/*short version string length*/0, /*compatibility length*/0,
778-
version::getSwiftFullVersion(LangOpts.EffectiveLanguageVersion));
786+
version::getSwiftFullVersion(LangOpts.EffectiveLanguageVersion));
779787

780788
ModuleName.emit(ScratchRecord, M->getName().str());
781789
Target.emit(ScratchRecord, LangOpts.Target.str());
782790
}
783791
}
784792
};
785793
}
786-
void serialization::writeSourceInfoToStream(raw_ostream &os, ModuleOrSourceFile DC) {
794+
void serialization::writeSourceInfoToStream(raw_ostream &os,
795+
ModuleOrSourceFile DC) {
787796
assert(DC);
788797
SourceInfoSerializer S{SWIFTSOURCEINFO_SIGNATURE, DC};
789798
// FIXME: This is only really needed for debugging. We don't actually use it.
@@ -797,10 +806,11 @@ void serialization::writeSourceInfoToStream(raw_ostream &os, ModuleOrSourceFile
797806
StringWriter FPWriter;
798807
emitBasicLocsRecord(S.Out, DC, USRWriter, FPWriter);
799808
// Emit USR table mapping from a USR to USR Id.
800-
// The basic locs record uses USR Id instead of actual USR, so that we don't need to repeat
801-
// USR texts for newly added records.
809+
// The basic locs record uses USR Id instead of actual USR, so that we
810+
// don't need to repeat USR texts for newly added records.
802811
USRWriter.emitUSRsRecord(S.Out);
803-
// A blob of 0 terminated strings referenced by the location records, e.g. file paths.
812+
// A blob of 0 terminated strings referenced by the location records,
813+
// e.g. file paths.
804814
FPWriter.emitSourceFilesRecord(S.Out);
805815
}
806816
}

0 commit comments

Comments
 (0)