From f34fa7ffcd4756a77fcd33ee3aeaeec8dd4a4155 Mon Sep 17 00:00:00 2001 From: Brent Royal-Gordon Date: Tue, 21 Jan 2020 14:52:32 -0800 Subject: [PATCH 1/6] =?UTF-8?q?Match=20prototype=E2=80=99s=20#file=20strin?= =?UTF-8?q?g=20format=20to=20upcoming=20SE-0274=20revision?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/SILGen/SILGenApply.cpp | 7 +++---- test/SILGen/magic_identifier_file.swift | 8 ++++---- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp index 4002810940a10..6890de5d826f7 100644 --- a/lib/SILGen/SILGenApply.cpp +++ b/lib/SILGen/SILGenApply.cpp @@ -4895,10 +4895,9 @@ std::string SILGenFunction::getMagicFileString(SourceLoc loc) { if (!getASTContext().LangOpts.EnableConcisePoundFile) return path; - auto value = llvm::sys::path::filename(path).str(); - value += " ("; - value += getModule().getSwiftModule()->getNameStr(); - value += ")"; + auto value = getModule().getSwiftModule()->getNameStr().str(); + value += "/"; + value += llvm::sys::path::filename(path).str(); return value; } diff --git a/test/SILGen/magic_identifier_file.swift b/test/SILGen/magic_identifier_file.swift index 7f1620f4f3dc3..8052f328ca0ee 100644 --- a/test/SILGen/magic_identifier_file.swift +++ b/test/SILGen/magic_identifier_file.swift @@ -8,26 +8,26 @@ func directUse() { // BOTH-LABEL: sil {{.*}} @$s3Foo9directUseyyF print(#file) // ABSOLUTE: string_literal utf8 "SOURCE_DIR/test/SILGen/magic_identifier_file.swift" -// CONCISE: string_literal utf8 "magic_identifier_file.swift (Foo)" +// CONCISE: string_literal utf8 "Foo/magic_identifier_file.swift" } func indirectUse() { // BOTH-LABEL: sil {{.*}} @$s3Foo11indirectUseyyF fatalError() // ABSOLUTE: string_literal utf8 "SOURCE_DIR/test/SILGen/magic_identifier_file.swift" -// CONCISE: string_literal utf8 "magic_identifier_file.swift (Foo)" +// CONCISE: string_literal utf8 "Foo/magic_identifier_file.swift" } func forceUnwrap(_ x: ()?) { // BOTH-LABEL: sil {{.*}} @$s3Foo11forceUnwrapyyytSgF _ = x! // ABSOLUTE: string_literal utf8 "SOURCE_DIR/test/SILGen/magic_identifier_file.swift" -// CONCISE: string_literal utf8 "magic_identifier_file.swift (Foo)" +// CONCISE: string_literal utf8 "Foo/magic_identifier_file.swift" } func forceTry(_ fn: () throws -> ()) { // BOTH-LABEL: sil {{.*}} @$s3Foo8forceTryyyyyKXEF try! fn() // ABSOLUTE: string_literal utf8 "SOURCE_DIR/test/SILGen/magic_identifier_file.swift" -// CONCISE: string_literal utf8 "magic_identifier_file.swift (Foo)" +// CONCISE: string_literal utf8 "Foo/magic_identifier_file.swift" } From 8e5ca8abdf8f5f0fda612feffac0a04702f5233f Mon Sep 17 00:00:00 2001 From: Brent Royal-Gordon Date: Thu, 23 Jan 2020 17:22:24 -0800 Subject: [PATCH 2/6] [NFC] Generate `#file` -> `#filePath` table ahead of time --- include/swift/AST/Module.h | 50 ++++++++++++++++ include/swift/AST/SourceFile.h | 6 ++ lib/AST/Module.cpp | 104 +++++++++++++++++++++++++++++++++ lib/Parse/ParseDecl.cpp | 3 +- lib/SILGen/SILGen.cpp | 3 +- lib/SILGen/SILGen.h | 3 + lib/SILGen/SILGenApply.cpp | 10 ++-- 7 files changed, 171 insertions(+), 8 deletions(-) diff --git a/include/swift/AST/Module.h b/include/swift/AST/Module.h index 618ebdbb061e9..c158ce4da529c 100644 --- a/include/swift/AST/Module.h +++ b/include/swift/AST/Module.h @@ -109,6 +109,38 @@ enum class SourceFileKind { Interface ///< Came from a .swiftinterface file, representing another module. }; +/// Contains information about where a particular path is used in +/// \c SourceFiles. +struct SourceFilePathInfo { + struct Comparator { + bool operator () (SourceLoc lhs, SourceLoc rhs) const { + return lhs.getOpaquePointerValue() < + rhs.getOpaquePointerValue(); + } + }; + + SourceLoc physicalFileLoc{}; + std::set virtualFileLocs{}; // std::set for sorting + + SourceFilePathInfo() = default; + + void merge(const SourceFilePathInfo &other) { + if (other.physicalFileLoc.isValid()) { + assert(!physicalFileLoc.isValid()); + physicalFileLoc = other.physicalFileLoc; + } + + for (auto &elem : other.virtualFileLocs) { + virtualFileLocs.insert(elem); + } + } + + bool operator == (const SourceFilePathInfo &other) const { + return physicalFileLoc == other.physicalFileLoc && + virtualFileLocs == other.virtualFileLocs; + } +}; + /// Discriminator for resilience strategy. enum class ResilienceStrategy : unsigned { /// Public nominal types: fragile @@ -257,6 +289,24 @@ class ModuleDecl : public DeclContext, public TypeDecl { void addFile(FileUnit &newFile); void removeFile(FileUnit &existingFile); + /// Creates a map from \c #filePath strings to corresponding \c #file + /// strings, diagnosing any conflicts. + /// + /// A given \c #filePath string always maps to exactly one \c #file string, + /// but it is possible for \c #sourceLocation directives to introduce + /// duplicates in the opposite direction. If there are such conflicts, this + /// method will diagnose the conflict and choose a "winner" among the paths + /// in a reproducible way. The \c bool paired with the \c #file string is + /// \c true for paths which did not have a conflict or won a conflict, and + /// \c false for paths which lost a conflict. Thus, if you want to generate a + /// reverse mapping, you should drop or special-case the \c #file strings that + /// are paired with \c false. + /// + /// Note that this returns an empty StringMap if concise \c #file strings are + /// disabled. Users should fall back to using the file path in this case. + llvm::StringMap> + computeMagicFileStringMap() const; + /// Add a file declaring a cross-import overlay. void addCrossImportOverlayFile(StringRef file); diff --git a/include/swift/AST/SourceFile.h b/include/swift/AST/SourceFile.h index ee67826873c2e..6db87ca56d98f 100644 --- a/include/swift/AST/SourceFile.h +++ b/include/swift/AST/SourceFile.h @@ -321,6 +321,12 @@ class SourceFile final : public FileUnit { /// forwarded on to IRGen. ASTStage_t ASTStage = Unprocessed; + /// Virtual filenames declared by #sourceLocation(file:) directives in this + /// file. + llvm::SmallVector, 0> VirtualFilenames; + + llvm::StringMap getInfoForUsedFilePaths() const; + SourceFile(ModuleDecl &M, SourceFileKind K, Optional bufferID, ImplicitModuleImportKind ModImpKind, bool KeepParsedTokens = false, bool KeepSyntaxTree = false, ParsingOptions parsingOpts = {}); diff --git a/lib/AST/Module.cpp b/lib/AST/Module.cpp index 39ebdcde234dc..e066f1e854fd4 100644 --- a/lib/AST/Module.cpp +++ b/lib/AST/Module.cpp @@ -1955,6 +1955,110 @@ static void performAutoImport( SF.addImports(Imports); } +llvm::StringMap +SourceFile::getInfoForUsedFilePaths() const { + llvm::StringMap result; + + if (BufferID != -1) { + result[getFilename()].physicalFileLoc = + getASTContext().SourceMgr.getLocForBufferStart(BufferID); + } + + for (auto &vpath : VirtualFilenames) { + result[vpath.Item].virtualFileLocs.insert(vpath.Loc); + } + + return result; +} + +/// Returns a map of filenames to a map of file paths to SourceFilePathInfo +/// instances, for all SourceFiles in the module. +static llvm::StringMap> +getInfoForUsedFileNames(const ModuleDecl *module) { + llvm::StringMap> result; + + for (auto *file : module->getFiles()) { + auto *sourceFile = dyn_cast(file); + if (!sourceFile) continue; + + for (auto &pair : sourceFile->getInfoForUsedFilePaths()) { + StringRef fullPath = pair.first(); + StringRef fileName = llvm::sys::path::filename(fullPath); + auto &info = pair.second; + + result[fileName][fullPath].merge(info); + } + } + + return result; +} + +static void +computeMagicFileString(const ModuleDecl *module, StringRef name, + SmallVectorImpl &result) { + result.assign(module->getNameStr().begin(), module->getNameStr().end()); + result.push_back('/'); + result.append(name.begin(), name.end()); +} + +static StringRef +resolveMagicNameConflicts(const ModuleDecl *module, StringRef fileString, + const llvm::StringMap &paths) { + assert(paths.size() > 1); + assert(module->getASTContext().LangOpts.EnableConcisePoundFile); + + /// The path we consider to be "correct"; we will emit fix-its changing the + /// other paths to match this one. + StringRef winner = ""; + + // First, select a winner. + for (const auto &pathPair : paths) { + // If there is a physical file with this name, we use its path and stop + // looking. + if (pathPair.second.physicalFileLoc.isValid()) { + winner = pathPair.first(); + break; + } + + // Otherwise, we favor the lexicographically "smaller" path. + if (winner.empty() || winner > pathPair.first()) { + winner = pathPair.first(); + } + } + + return winner; +} + +llvm::StringMap> +ModuleDecl::computeMagicFileStringMap() const { + llvm::StringMap> result; + SmallString<64> scratch; + + if (!getASTContext().LangOpts.EnableConcisePoundFile) + return result; + + for (auto &namePair : getInfoForUsedFileNames(this)) { + computeMagicFileString(this, namePair.first(), scratch); + auto &infoForPaths = namePair.second; + + assert(!infoForPaths.empty()); + + // TODO: In the future, we'd like to handle these conflicts gracefully by + // generating a unique `#file` string for each conflicting name. For now, we + // will simply warn about conflicts. + StringRef winner = infoForPaths.begin()->first(); + if (infoForPaths.size() > 1) + winner = resolveMagicNameConflicts(this, scratch, infoForPaths); + + for (auto &pathPair : infoForPaths) { + result[pathPair.first()] = + std::make_pair(scratch.str().str(), pathPair.first() == winner); + } + } + + return result; +} + SourceFile::SourceFile(ModuleDecl &M, SourceFileKind K, Optional bufferID, ImplicitModuleImportKind ModImpKind, diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index 57941fe0a80f5..5b3d0b5a05591 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -4822,7 +4822,8 @@ ParserStatus Parser::parseLineDirective(bool isLine) { getStringLiteralIfNotInterpolated(Loc, "'#sourceLocation'"); if (!Filename.hasValue()) return makeParserError(); - consumeToken(tok::string_literal); + SourceLoc filenameLoc = consumeToken(tok::string_literal); + SF.VirtualFilenames.emplace_back(*Filename, filenameLoc); if (parseToken(tok::comma, diag::sourceLocation_expected, ",") || parseSpecificIdentifier("line", diag::sourceLocation_expected, diff --git a/lib/SILGen/SILGen.cpp b/lib/SILGen/SILGen.cpp index ed4f400cdc812..dca7cff24c81d 100644 --- a/lib/SILGen/SILGen.cpp +++ b/lib/SILGen/SILGen.cpp @@ -50,7 +50,8 @@ using namespace Lowering; //===----------------------------------------------------------------------===// SILGenModule::SILGenModule(SILModule &M, ModuleDecl *SM) - : M(M), Types(M.Types), SwiftModule(SM), TopLevelSGF(nullptr) { + : M(M), Types(M.Types), SwiftModule(SM), TopLevelSGF(nullptr), + MagicFileStringsByFilePath(SM->computeMagicFileStringMap()) { const SILOptions &Opts = M.getOptions(); if (!Opts.UseProfile.empty()) { auto ReaderOrErr = llvm::IndexedInstrProfReader::create(Opts.UseProfile); diff --git a/lib/SILGen/SILGen.h b/lib/SILGen/SILGen.h index 11774bc0f0a9e..3c1235cf807ac 100644 --- a/lib/SILGen/SILGen.h +++ b/lib/SILGen/SILGen.h @@ -131,6 +131,9 @@ class LLVM_LIBRARY_VISIBILITY SILGenModule : public ASTVisitor { ASTContext &getASTContext() { return M.getASTContext(); } + llvm::StringMap> + MagicFileStringsByFilePath; + static DeclName getMagicFunctionName(SILDeclRef ref); static DeclName getMagicFunctionName(DeclContext *dc); diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp index 6890de5d826f7..96afe9971ab69 100644 --- a/lib/SILGen/SILGenApply.cpp +++ b/lib/SILGen/SILGenApply.cpp @@ -4892,13 +4892,11 @@ StringRef SILGenFunction::getMagicFilePathString(SourceLoc loc) { std::string SILGenFunction::getMagicFileString(SourceLoc loc) { auto path = getMagicFilePathString(loc); - if (!getASTContext().LangOpts.EnableConcisePoundFile) - return path; + auto result = SGM.MagicFileStringsByFilePath.find(path); + if (result != SGM.MagicFileStringsByFilePath.end()) + return std::get<0>(result->second); - auto value = getModule().getSwiftModule()->getNameStr().str(); - value += "/"; - value += llvm::sys::path::filename(path).str(); - return value; + return path; } /// Emit an application of the given allocating initializer. From 54f5967d486273c233f49b09f04370dc81e8cb07 Mon Sep 17 00:00:00 2001 From: Brent Royal-Gordon Date: Thu, 23 Jan 2020 17:23:22 -0800 Subject: [PATCH 3/6] Include `#file` -> `#filePath` table in printed SIL A proof of concept for tools providing this information in more useful forms. --- lib/SIL/SILPrinter.cpp | 56 +++++++++++++++++++++++++ test/SILGen/magic_identifier_file.swift | 4 ++ 2 files changed, 60 insertions(+) diff --git a/lib/SIL/SILPrinter.cpp b/lib/SIL/SILPrinter.cpp index e7c6e9b224214..cd66339c2e435 100644 --- a/lib/SIL/SILPrinter.cpp +++ b/lib/SIL/SILPrinter.cpp @@ -47,6 +47,7 @@ #include "llvm/Support/CommandLine.h" #include "llvm/Support/FormattedStream.h" #include "llvm/Support/FileSystem.h" +#include using namespace swift; @@ -2750,6 +2751,57 @@ printSILCoverageMaps(SILPrintContext &Ctx, M->print(Ctx); } +using MagicFileStringMap = + llvm::StringMap>; + +static void +printMagicFileStringMapEntry(SILPrintContext &Ctx, + const MagicFileStringMap::MapEntryTy &entry) { + auto &OS = Ctx.OS(); + OS << "// '" << std::get<0>(entry.second) + << "' => '" << entry.first() << "'"; + + if (!std::get<1>(entry.second)) + OS << " (alternate)"; + + OS << "\n"; +} + +static void printMagicFileStringMap(SILPrintContext &Ctx, + const MagicFileStringMap map) { + if (map.empty()) + return; + + Ctx.OS() << "\n\n// Mappings from '#file' to '#filePath':\n"; + + if (Ctx.sortSIL()) { + llvm::SmallVector keys; + llvm::copy(map.keys(), std::back_inserter(keys)); + + llvm::sort(keys, [&](StringRef leftKey, StringRef rightKey) -> bool { + const auto &leftValue = map.find(leftKey)->second; + const auto &rightValue = map.find(rightKey)->second; + + // Lexicographically earlier #file strings sort earlier. + if (std::get<0>(leftValue) != std::get<0>(rightValue)) + return std::get<0>(leftValue) < std::get<0>(rightValue); + + // Conflict winners sort before losers. + if (std::get<1>(leftValue) != std::get<1>(rightValue)) + return std::get<1>(leftValue); + + // Finally, lexicographically earlier #filePath strings sort earlier. + return leftKey < rightKey; + }); + + for (auto key : keys) + printMagicFileStringMapEntry(Ctx, *map.find(key)); + } else { + for (const auto &entry : map) + printMagicFileStringMapEntry(Ctx, entry); + } +} + void SILProperty::print(SILPrintContext &Ctx) const { PrintOptions Options = PrintOptions::printSIL(); @@ -2864,6 +2916,10 @@ void SILModule::print(SILPrintContext &PrintCtx, ModuleDecl *M, printSILProperties(PrintCtx, getPropertyList()); printExternallyVisibleDecls(PrintCtx, externallyVisible.getArrayRef()); + if (M) + printMagicFileStringMap( + PrintCtx, M->computeMagicFileStringMap()); + OS << "\n\n"; } diff --git a/test/SILGen/magic_identifier_file.swift b/test/SILGen/magic_identifier_file.swift index 8052f328ca0ee..7e67de96909bc 100644 --- a/test/SILGen/magic_identifier_file.swift +++ b/test/SILGen/magic_identifier_file.swift @@ -31,3 +31,7 @@ func forceTry(_ fn: () throws -> ()) { // ABSOLUTE: string_literal utf8 "SOURCE_DIR/test/SILGen/magic_identifier_file.swift" // CONCISE: string_literal utf8 "Foo/magic_identifier_file.swift" } + +// CONCISE-LABEL: // Mappings from '#file' to '#filePath': +// CONCISE: // 'Foo/magic_identifier_file.swift' => 'SOURCE_DIR/test/SILGen/magic_identifier_file.swift' + From d590823f3c468a45adf4c018092f59a098d47bf4 Mon Sep 17 00:00:00 2001 From: Brent Royal-Gordon Date: Thu, 23 Jan 2020 17:25:43 -0800 Subject: [PATCH 4/6] Warn about conflicts between #file strings --- include/swift/AST/DiagnosticsCommon.def | 8 ++ include/swift/AST/Module.h | 2 +- lib/AST/Module.cpp | 40 +++++++- lib/SIL/SILPrinter.cpp | 2 +- lib/SILGen/SILGen.cpp | 3 +- ...ic_identifier_file_conflicting_other.swift | 8 ++ ...agic_identifier_file_conflicting.swift.gyb | 96 +++++++++++++++++++ 7 files changed, 153 insertions(+), 6 deletions(-) create mode 100644 test/SILGen/Inputs/magic_identifier_file_conflicting_other.swift create mode 100644 test/SILGen/magic_identifier_file_conflicting.swift.gyb diff --git a/include/swift/AST/DiagnosticsCommon.def b/include/swift/AST/DiagnosticsCommon.def index 77212755457db..e7de3a3e203c8 100644 --- a/include/swift/AST/DiagnosticsCommon.def +++ b/include/swift/AST/DiagnosticsCommon.def @@ -139,6 +139,14 @@ ERROR(sdk_node_unrecognized_decl_kind,none, ERROR(sdk_node_unrecognized_accessor_kind,none, "unrecognized accessor kind '%0' in SDK node", (StringRef)) +// Emitted from ModuleDecl::computeMagicFileStringMap() +WARNING(pound_source_location_creates_pound_file_conflicts,none, + "'#sourceLocation' directive produces '#file' string of '%0', which " + "conflicts with '#file' strings produced by other paths in the module", + (StringRef)) +NOTE(fixit_correct_source_location_file,none, + "change file in '#sourceLocation' to '%0'", (StringRef)) + //------------------------------------------------------------------------------ // MARK: Circular reference diagnostics //------------------------------------------------------------------------------ diff --git a/include/swift/AST/Module.h b/include/swift/AST/Module.h index c158ce4da529c..e3e22833503a7 100644 --- a/include/swift/AST/Module.h +++ b/include/swift/AST/Module.h @@ -305,7 +305,7 @@ class ModuleDecl : public DeclContext, public TypeDecl { /// Note that this returns an empty StringMap if concise \c #file strings are /// disabled. Users should fall back to using the file path in this case. llvm::StringMap> - computeMagicFileStringMap() const; + computeMagicFileStringMap(bool shouldDiagnose) const; /// Add a file declaring a cross-import overlay. void addCrossImportOverlayFile(StringRef file); diff --git a/lib/AST/Module.cpp b/lib/AST/Module.cpp index e066f1e854fd4..77f4cedcbfddf 100644 --- a/lib/AST/Module.cpp +++ b/lib/AST/Module.cpp @@ -2003,7 +2003,8 @@ computeMagicFileString(const ModuleDecl *module, StringRef name, static StringRef resolveMagicNameConflicts(const ModuleDecl *module, StringRef fileString, - const llvm::StringMap &paths) { + const llvm::StringMap &paths, + bool shouldDiagnose) { assert(paths.size() > 1); assert(module->getASTContext().LangOpts.EnableConcisePoundFile); @@ -2026,11 +2027,43 @@ resolveMagicNameConflicts(const ModuleDecl *module, StringRef fileString, } } + // If we're not diagnosing, that's all we need to do. + if (!shouldDiagnose) + return winner; + + SmallString<64> winnerLiteral; + llvm::raw_svector_ostream winnerLiteralStream{winnerLiteral}; + swift::printAsQuotedString(winnerLiteralStream, winner); + + auto &diags = module->getASTContext().Diags; + + // Diagnose the conflict at each #sourceLocation that specifies it. + for (const auto &pathPair : paths) { + bool isWinner = (pathPair.first() == winner); + + // Don't diagnose #sourceLocations that match the physical file. + if (pathPair.second.physicalFileLoc.isValid()) { + assert(isWinner && "physical files should always win; duplicate name?"); + continue; + } + + for (auto loc : pathPair.second.virtualFileLocs) { + diags.diagnose(loc, + diag::pound_source_location_creates_pound_file_conflicts, + fileString); + + // Offer a fix-it unless it would be tautological. + if (!isWinner) + diags.diagnose(loc, diag::fixit_correct_source_location_file, winner) + .fixItReplace(loc, winnerLiteral); + } + } + return winner; } llvm::StringMap> -ModuleDecl::computeMagicFileStringMap() const { +ModuleDecl::computeMagicFileStringMap(bool shouldDiagnose) const { llvm::StringMap> result; SmallString<64> scratch; @@ -2048,7 +2081,8 @@ ModuleDecl::computeMagicFileStringMap() const { // will simply warn about conflicts. StringRef winner = infoForPaths.begin()->first(); if (infoForPaths.size() > 1) - winner = resolveMagicNameConflicts(this, scratch, infoForPaths); + winner = resolveMagicNameConflicts(this, scratch, infoForPaths, + shouldDiagnose); for (auto &pathPair : infoForPaths) { result[pathPair.first()] = diff --git a/lib/SIL/SILPrinter.cpp b/lib/SIL/SILPrinter.cpp index cd66339c2e435..17302665529ff 100644 --- a/lib/SIL/SILPrinter.cpp +++ b/lib/SIL/SILPrinter.cpp @@ -2918,7 +2918,7 @@ void SILModule::print(SILPrintContext &PrintCtx, ModuleDecl *M, if (M) printMagicFileStringMap( - PrintCtx, M->computeMagicFileStringMap()); + PrintCtx, M->computeMagicFileStringMap(/*shouldDiagnose=*/false)); OS << "\n\n"; } diff --git a/lib/SILGen/SILGen.cpp b/lib/SILGen/SILGen.cpp index dca7cff24c81d..f5e80b018c416 100644 --- a/lib/SILGen/SILGen.cpp +++ b/lib/SILGen/SILGen.cpp @@ -51,7 +51,8 @@ using namespace Lowering; SILGenModule::SILGenModule(SILModule &M, ModuleDecl *SM) : M(M), Types(M.Types), SwiftModule(SM), TopLevelSGF(nullptr), - MagicFileStringsByFilePath(SM->computeMagicFileStringMap()) { + MagicFileStringsByFilePath( + SM->computeMagicFileStringMap(/*shouldDiagnose=*/true)) { const SILOptions &Opts = M.getOptions(); if (!Opts.UseProfile.empty()) { auto ReaderOrErr = llvm::IndexedInstrProfReader::create(Opts.UseProfile); diff --git a/test/SILGen/Inputs/magic_identifier_file_conflicting_other.swift b/test/SILGen/Inputs/magic_identifier_file_conflicting_other.swift new file mode 100644 index 0000000000000..602b96c9b589a --- /dev/null +++ b/test/SILGen/Inputs/magic_identifier_file_conflicting_other.swift @@ -0,0 +1,8 @@ +// This file matches test/SILGen/magic_identifier_file_conflicting.swift. +// It should be compiled with -verify. + +// We should diagnose cross-file conflicts. +// expected-warning@+2 {{'#sourceLocation' directive produces '#file' string of 'Foo/other_file_c.swift', which conflicts with '#file' strings produced by other paths in the module}} +// expected-note@+1 {{change file in '#sourceLocation' to 'first/other_file_c.swift'}} {{23-50="first/other_file_c.swift"}} +#sourceLocation(file: "second/other_file_c.swift", line: 1) +#sourceLocation() diff --git a/test/SILGen/magic_identifier_file_conflicting.swift.gyb b/test/SILGen/magic_identifier_file_conflicting.swift.gyb new file mode 100644 index 0000000000000..9c434a34d05ba --- /dev/null +++ b/test/SILGen/magic_identifier_file_conflicting.swift.gyb @@ -0,0 +1,96 @@ +// RUN: %empty-directory(%t) +// RUN: %gyb -D TEMPDIR=%t %s > %t/magic_identifier_file_conflicting.swift + +// We want to check both the diagnostics and the SIL output. +// RUN: %target-swift-emit-silgen -verify -emit-sorted-sil -enable-experimental-concise-pound-file -module-name Foo %t/magic_identifier_file_conflicting.swift %S/Inputs/magic_identifier_file_conflicting_other.swift | %FileCheck %s + +%{ +TEMPDIR_ESC = TEMPDIR.replace('\\', '\\\\') +def fixit_loc(start_col, orig_suffix): + """ + Computes a "start-end" string for a fix-it replacing a string literal which + starts at start_col and joins orig_suffix to TEMPDIR with a slash. + """ + original = '"{0}/{1}"'.format(TEMPDIR, orig_suffix) + end_col = start_col + len(original) + return '{0}-{1}'.format(start_col, end_col) +}% + +// +// Same name as a physical file +// + +// There should be no warning for the exact same path. +// no-warning +#sourceLocation(file: "${TEMPDIR_ESC}/magic_identifier_file_conflicting.swift", line: 10) +#sourceLocation() + +// expected-warning@+2 {{'#sourceLocation' directive produces '#file' string of 'Foo/magic_identifier_file_conflicting.swift', which conflicts with '#file' strings produced by other paths in the module}} +// expected-note@+1 {{change file in '#sourceLocation' to '${TEMPDIR_ESC}/magic_identifier_file_conflicting.swift'}} {{${fixit_loc(23, "other_path_b/magic_identifier_file_conflicting.swift")}="${TEMPDIR_ESC}/magic_identifier_file_conflicting.swift"}} +#sourceLocation(file: "${TEMPDIR_ESC}/other_path_b/magic_identifier_file_conflicting.swift", line: 20) +#sourceLocation() + +// expected-warning@+2 {{'#sourceLocation' directive produces '#file' string of 'Foo/magic_identifier_file_conflicting.swift', which conflicts with '#file' strings produced by other paths in the module}} +// expected-note@+1 {{change file in '#sourceLocation' to '${TEMPDIR_ESC}/magic_identifier_file_conflicting.swift'}} {{23-64="${TEMPDIR_ESC}/magic_identifier_file_conflicting.swift"}} +#sourceLocation(file: "magic_identifier_file_conflicting.swift", line: 30) +#sourceLocation() + +// +// No physical file with the same name +// + +// There shoud be no warning for a purely virtual file. +// no-warning +#sourceLocation(file: "other_file_a.swift", line: 40) +#sourceLocation() + +// Even if you use it twice. +// no-warning +#sourceLocation(file: "other_file_a.swift", line: 50) +#sourceLocation() + +// But there should be warnings for different-path, same-name virtual files. +// The lexicographically first path should be treated as canonical--we diagnose +// but don't offer a fix-it. +// expected-warning@+1 {{'#sourceLocation' directive produces '#file' string of 'Foo/other_file_b.swift', which conflicts with '#file' strings produced by other paths in the module}} +#sourceLocation(file: "first/other_file_b.swift", line: 60) +#sourceLocation() + +// Subsequent paths should fix-it to the first one. +// expected-warning@+2 {{'#sourceLocation' directive produces '#file' string of 'Foo/other_file_b.swift', which conflicts with '#file' strings produced by other paths in the module}} +// expected-note@+1 {{change file in '#sourceLocation' to 'first/other_file_b.swift'}} {{23-50="first/other_file_b.swift"}} +#sourceLocation(file: "second/other_file_b.swift", line: 70) +#sourceLocation() + +// Even if there's more than one. +// expected-warning@+2 {{'#sourceLocation' directive produces '#file' string of 'Foo/other_file_b.swift', which conflicts with '#file' strings produced by other paths in the module}} +// expected-note@+1 {{change file in '#sourceLocation' to 'first/other_file_b.swift'}} {{23-49="first/other_file_b.swift"}} +#sourceLocation(file: "third/other_file_b.swift", line: 80) +#sourceLocation() + +// Even if one is duplicated. +// expected-warning@+2 {{'#sourceLocation' directive produces '#file' string of 'Foo/other_file_b.swift', which conflicts with '#file' strings produced by other paths in the module}} +// expected-note@+1 {{change file in '#sourceLocation' to 'first/other_file_b.swift'}} {{23-49="first/other_file_b.swift"}} +#sourceLocation(file: "third/other_file_b.swift", line: 90) +#sourceLocation() + +// We should diagnose cross-file conflicts. +// expected-warning@+1 {{'#sourceLocation' directive produces '#file' string of 'Foo/other_file_c.swift', which conflicts with '#file' strings produced by other paths in the module}} +#sourceLocation(file: "first/other_file_c.swift", line: 100) +#sourceLocation() + +// +// Check '#file' => '#filePath' mapping table +// + +// CHECK-LABEL: // Mappings from '#file' to '#filePath': +// CHECK-NEXT: // 'Foo/magic_identifier_file_conflicting.swift' => 'BUILD_DIR{{[/\\]}}test-{{[^/]+}}{{[/\\]}}SILGen{{[/\\]}}Output{{[/\\]}}magic_identifier_file_conflicting.swift.gyb.tmp{{[/\\]}}magic_identifier_file_conflicting.swift' +// CHECK-NEXT: // 'Foo/magic_identifier_file_conflicting.swift' => 'BUILD_DIR{{[/\\]}}test-{{[^/]+}}{{[/\\]}}SILGen{{[/\\]}}Output{{[/\\]}}magic_identifier_file_conflicting.swift.gyb.tmp{{[/\\]}}other_path_b{{[/\\]}}magic_identifier_file_conflicting.swift' (alternate) +// CHECK-NEXT: // 'Foo/magic_identifier_file_conflicting.swift' => 'magic_identifier_file_conflicting.swift' (alternate) +// CHECK-NEXT: // 'Foo/magic_identifier_file_conflicting_other.swift' => 'SOURCE_DIR{{[/\\]}}test{{[/\\]}}SILGen{{[/\\]}}Inputs{{[/\\]}}magic_identifier_file_conflicting_other.swift' +// CHECK-NEXT: // 'Foo/other_file_a.swift' => 'other_file_a.swift' +// CHECK-NEXT: // 'Foo/other_file_b.swift' => 'first/other_file_b.swift' +// CHECK-NEXT: // 'Foo/other_file_b.swift' => 'second/other_file_b.swift' (alternate) +// CHECK-NEXT: // 'Foo/other_file_b.swift' => 'third/other_file_b.swift' (alternate) +// CHECK-NEXT: // 'Foo/other_file_c.swift' => 'first/other_file_c.swift' +// CHECK-NEXT: // 'Foo/other_file_c.swift' => 'second/other_file_c.swift' (alternate) From 85599f7582b8809be889f589fe4d1d95e09ec945 Mon Sep 17 00:00:00 2001 From: Brent Royal-Gordon Date: Thu, 5 Mar 2020 21:51:54 -0800 Subject: [PATCH 5/6] Keep gyb from messing up line endings on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GYB opens its inputs and outputs in text mode. This is a problem because the recommendation in Windows is to check everything out in LF mode to avoid breaking tests, so gybbing a file ends up converting LFs to CRLFs. That broke test/SILGen/magic_identifier_file_conflicting.swift.gyb. Modify GYB to open its files in binary mode instead of text mode. This is a no-op everywhere but Windows. Note that this change is incomplete: it’s somewhat difficult to switch stdin and stdout to binary mode in Python 2. I’ve added FIXME comments about this. --- test/SILGen/magic_identifier_file_conflicting.swift.gyb | 2 +- utils/gyb.py | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/test/SILGen/magic_identifier_file_conflicting.swift.gyb b/test/SILGen/magic_identifier_file_conflicting.swift.gyb index 9c434a34d05ba..0125ed31e0895 100644 --- a/test/SILGen/magic_identifier_file_conflicting.swift.gyb +++ b/test/SILGen/magic_identifier_file_conflicting.swift.gyb @@ -1,5 +1,5 @@ // RUN: %empty-directory(%t) -// RUN: %gyb -D TEMPDIR=%t %s > %t/magic_identifier_file_conflicting.swift +// RUN: %gyb -D TEMPDIR=%t %s -o %t/magic_identifier_file_conflicting.swift // We want to check both the diagnostics and the SIL output. // RUN: %target-swift-emit-silgen -verify -emit-sorted-sil -enable-experimental-concise-pound-file -module-name Foo %t/magic_identifier_file_conflicting.swift %S/Inputs/magic_identifier_file_conflicting_other.swift | %FileCheck %s diff --git a/utils/gyb.py b/utils/gyb.py index 2df9a0311dbb6..b0f6e78c63d5a 100755 --- a/utils/gyb.py +++ b/utils/gyb.py @@ -1208,12 +1208,13 @@ def succ(a): help='''Bindings to be set in the template's execution context''') parser.add_argument( - 'file', type=argparse.FileType(), + 'file', type=argparse.FileType('rb'), help='Path to GYB template file (defaults to stdin)', nargs='?', - default=sys.stdin) + default=sys.stdin) # FIXME: stdin not binary mode on Windows parser.add_argument( - '-o', dest='target', type=argparse.FileType('w'), - help='Output file (defaults to stdout)', default=sys.stdout) + '-o', dest='target', type=argparse.FileType('wb'), + help='Output file (defaults to stdout)', + default=sys.stdout) # FIXME: stdout not binary mode on Windows parser.add_argument( '--test', action='store_true', default=False, help='Run a self-test') From 62ab3dfd642a0e60af39e899696dd92e8166df81 Mon Sep 17 00:00:00 2001 From: Brent Royal-Gordon Date: Thu, 5 Mar 2020 23:29:24 -0800 Subject: [PATCH 6/6] Temporarily disable new #file test on Windows --- test/SILGen/magic_identifier_file_conflicting.swift.gyb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/SILGen/magic_identifier_file_conflicting.swift.gyb b/test/SILGen/magic_identifier_file_conflicting.swift.gyb index 0125ed31e0895..4dcdea991b9d5 100644 --- a/test/SILGen/magic_identifier_file_conflicting.swift.gyb +++ b/test/SILGen/magic_identifier_file_conflicting.swift.gyb @@ -4,6 +4,11 @@ // We want to check both the diagnostics and the SIL output. // RUN: %target-swift-emit-silgen -verify -emit-sorted-sil -enable-experimental-concise-pound-file -module-name Foo %t/magic_identifier_file_conflicting.swift %S/Inputs/magic_identifier_file_conflicting_other.swift | %FileCheck %s +// FIXME: Make this test work on Windows. There's no fundamental reason it +// can't; we just need someone with a Windows machine to do it so we +// don't have a 30-minute debug cycle. +// UNSUPPORTED: OS=windows-msvc + %{ TEMPDIR_ESC = TEMPDIR.replace('\\', '\\\\') def fixit_loc(start_col, orig_suffix):