Skip to content
Open
73 changes: 54 additions & 19 deletions llvm/lib/TableGen/Main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@
#include "llvm/TableGen/Main.h"
#include "TGLexer.h"
#include "TGParser.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/Twine.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/ErrorOr.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/SMLoc.h"
#include "llvm/Support/SourceMgr.h"
#include "llvm/Support/ToolOutputFile.h"
Expand All @@ -37,32 +39,32 @@
#include <utility>
using namespace llvm;

static cl::opt<std::string>
OutputFilename("o", cl::desc("Output filename"), cl::value_desc("filename"),
cl::init("-"));
static cl::opt<std::string> OutputFilename("o", cl::desc("Output filename"),
cl::value_desc("filename"),
cl::init("-"));

static cl::opt<std::string>
DependFilename("d",
cl::desc("Dependency filename"),
cl::value_desc("filename"),
cl::init(""));
static cl::opt<std::string> DependFilename("d", cl::desc("Dependency filename"),
cl::value_desc("filename"),
cl::init(""));

static cl::opt<std::string>
InputFilename(cl::Positional, cl::desc("<input file>"), cl::init("-"));
InputFilename(cl::Positional, cl::desc("<input file>"), cl::init("-"));

static cl::list<std::string>
IncludeDirs("I", cl::desc("Directory of include files"),
cl::value_desc("directory"), cl::Prefix);
static cl::list<std::string> IncludeDirs("I",
cl::desc("Directory of include files"),
cl::value_desc("directory"),
cl::Prefix);

static cl::list<std::string>
MacroNames("D", cl::desc("Name of the macro to be defined"),
cl::value_desc("macro name"), cl::Prefix);
MacroNames("D", cl::desc("Name of the macro to be defined"),
cl::value_desc("macro name"), cl::Prefix);

static cl::opt<bool>
WriteIfChanged("write-if-changed", cl::desc("Only write output if it changed"));
WriteIfChanged("write-if-changed",
cl::desc("Only write output if it changed"));

static cl::opt<bool>
TimePhases("time-phases", cl::desc("Time phases of parser and backend"));
static cl::opt<bool> TimePhases("time-phases",
cl::desc("Time phases of parser and backend"));

namespace llvm {
cl::opt<bool> EmitLongStrLiterals(
Expand All @@ -83,6 +85,39 @@ static int reportError(const char *ProgName, Twine Msg) {
return 1;
}

/// Escape a filename in the dependency file so that it is correctly
/// interpreted by `make`. This is consistent with Clang, GCC, and lld.
static std::string escapeDependencyFilename(StringRef Filename) {
std::string Res;
raw_string_ostream OS(Res);

// Only transform the path to native on Windows, where backslashes are valid
// path separators. On non-Windows platforms, we don't want backslashes in
// filenames to be incorrectly treated as path separators.
#ifdef _WIN32
SmallString<256> NativePath;
sys::path::native(Filename, NativePath);
#else
StringRef NativePath = Filename;
#endif

for (unsigned I = 0, E = NativePath.size(); I != E; ++I) {
if (NativePath[I] == '#')
OS << '\\';
else if (NativePath[I] == ' ') {
OS << '\\';
unsigned J = I;
while (J > 0 && NativePath[--J] == '\\')
OS << '\\';
} else if (NativePath[I] == '$')
OS << '$';
OS << NativePath[I];
}

OS.flush();
return Res;
}

/// Create a dependency file for `-d` option.
///
/// This functionality is really only for the benefit of the build system.
Expand All @@ -96,9 +131,9 @@ static int createDependencyFile(const TGParser &Parser, const char *argv0) {
if (EC)
return reportError(argv0, "error opening " + DependFilename + ":" +
EC.message() + "\n");
DepOut.os() << OutputFilename << ":";
DepOut.os() << escapeDependencyFilename(OutputFilename) << ":";
for (const auto &Dep : Parser.getDependencies()) {
DepOut.os() << ' ' << Dep;
DepOut.os() << ' ' << escapeDependencyFilename(Dep);
}
DepOut.os() << "\n";
DepOut.keep();
Expand Down
38 changes: 38 additions & 0 deletions llvm/test/TableGen/escape-dependency-filenames.td
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// RUN: rm -rf %t; split-file %s %t

//--- normal-file.td
class NormalClass {}

//--- file with spaces.td
class SpaceClass {}

//--- file#with#hash.td
class HashClass {}

//--- file$with$dollar.td
class DollarClass {}

//--- file with escape\ before spaces.td
class EscapeBeforeSpacesClass {}

//--- main.td
include "normal-file.td"
include "file with spaces.td"
include "file#with#hash.td"
include "file$with$dollar.td"
include "file with escape\\ before spaces.td" // backslash itself needs escaping

def Normal : NormalClass;
def Spaces : SpaceClass;
def Hash : HashClass;
def Dollar : DollarClass;
def EscapeBeforeSpaces : EscapeBeforeSpacesClass;

// RUN: llvm-tblgen -I %t -d %t.d -o %t.out %t/main.td
// RUN: FileCheck --input-file=%t.d %s

// CHECK-DAG: normal-file.td
// CHECK-DAG: file\ with\ spaces.td
// CHECK-DAG: file\#with\#hash.td
// CHECK-DAG: file$$with$$dollar.td
// CHECK-DAG: file\ with\ escape\\\ before\ spaces.td
Copy link
Member Author

@ZhongRuoyu ZhongRuoyu Sep 26, 2025

Choose a reason for hiding this comment

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

This test (file with escape\ before spaces) actually failed on UNIX before 7718c55, as the filename was escaped as file\ with\ space/\ before\ spaces.td suggesting that the backslash in the filename was mistakenly (?) transformed to the path separator / due to this:

llvm::replace(Path, '\\', '/');

So if the test case actually makes sense, I believe it's more correct to only transform the path to its native form using sys::path::native on Windows. Its "On Unix, it converts all '' to '/'" behavior doesn't seem to be needed here (*-tblgen don't have the -fms-compatibility flag where backslashes are treated as path separators):

/// Convert path to the native form. This is used to give paths to users and
/// operating system calls in the platform's normal way. For example, on Windows
/// all '/' are converted to '\'. On Unix, it converts all '\' to '/'.
///
/// @param path A path that is transformed to native format.
/// @param result Holds the result of the transformation.
LLVM_ABI void native(const Twine &path, SmallVectorImpl<char> &result,
Style style = Style::native);

As for Clang and lld (as mentioned in the PR description), this also seems to be an issue that's worth checking.

$ echo '#include <a\ b>' | gcc-15 -M -MG -xc -
-: a\\\ b
$ echo '#include <a\ b>' | clang-21 -M -MG -xc - 
-.o: a/\ b

Copy link
Member Author

@ZhongRuoyu ZhongRuoyu Sep 26, 2025

Choose a reason for hiding this comment

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

Proposed the same change in Clang and lld, respectively:

If these changes are accepted we can probably DRY this up into a new llvm/support helper class.