[PATCH] D52942: lld-link: Implement support for %_PDB% and %_EXT% for /pdbaltpath:.
Nico Weber via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Oct 5 16:00:49 PDT 2018
thakis marked 2 inline comments as done.
thakis added a comment.
Thanks!
================
Comment at: COFF/Driver.cpp:843
+ // Invariant:
+ // +--------- Cursor ('a...' might be the empty string).
+ // | +----- FirstMark
----------------
ruiu wrote:
> Instead of handling `a...` fragment all at once, you can read one character at a time, and I believe that's slightly simpler. E.g.
>
> while (!AltPath.empty()) {
> if (!AltPath.startswith("%")
> Buf.append(AltPath[0]);
> else if (AltPath.startswith("%_pdb%")
> ....
> else if (AltPath.startswith("%_ext%")
> ....
> else
> ....
> }
I'm not sure it's much simpler, because the else needs to skip until the next % (for printing the warning, and for correctness). Here's how it looks with the .... filled in (maybe I didn't fill in the .... as you pictured though):
```
static void parsePDBAltPath(StringRef AltPath) {
SmallString<128> Buf;
StringRef PDBBasename =
sys::path::filename(Config->PDBPath, sys::path::Style::windows);
StringRef BinaryExtension =
sys::path::extension(Config->OutputFile, sys::path::Style::windows);
if (!BinaryExtension.empty())
BinaryExtension = BinaryExtension.substr(1); // %_EXT% does not include '.'.
while (!AltPath.empty()) {
if (!AltPath.startswith("%")) {
Buf.append(1, AltPath[0]);
AltPath = AltPath.drop_front(1);
} else if (AltPath.startswith_lower("%_pdb%")) {
Buf.append(PDBBasename);
AltPath = AltPath.drop_front(strlen("%_pdb%"));
} else if (AltPath.startswith_lower("%_ext%")) {
Buf.append(BinaryExtension);
AltPath = AltPath.drop_front(strlen("%_ext%"));
} else {
size_t P = AltPath.find('%', 1);
if (P == StringRef::npos) {
Buf.append(AltPath);
break;
}
StringRef Var = AltPath.substr(0, P + 1);
warn("only %_PDB% and %_EXT% supported in /pdbaltpath:, keeping " + Var +
" as literal");
Buf.append(Var);
AltPath = AltPath.drop_front(Var.size());
}
}
Config->PDBAltPath = Buf;
}
```
or, slightly different:
```
static void parsePDBAltPath(StringRef AltPath) {
SmallString<128> Buf;
StringRef PDBBasename =
sys::path::filename(Config->PDBPath, sys::path::Style::windows);
StringRef BinaryExtension =
sys::path::extension(Config->OutputFile, sys::path::Style::windows);
if (!BinaryExtension.empty())
BinaryExtension = BinaryExtension.substr(1); // %_EXT% does not include '.'.
while (!AltPath.empty()) {
if (!AltPath.startswith("%")) {
Buf.append(1, AltPath[0]);
AltPath = AltPath.drop_front(1);
} else {
size_t P = AltPath.find('%', 1);
if (P == StringRef::npos) {
Buf.append(AltPath);
break;
}
StringRef Var = AltPath.substr(0, P + 1);
if (Var.equals_lower("%_pdb%"))
Buf.append(PDBBasename);
else if (Var.equals_lower("%_ext%"))
Buf.append(BinaryExtension);
else {
warn("only %_PDB% and %_EXT% supported in /pdbaltpath:, keeping " +
Var + " as literal");
Buf.append(Var);
}
AltPath = AltPath.drop_front(Var.size());
}
}
Config->PDBAltPath = Buf;
}
```
Here's the algorithm in the patch with drop_front:
```
static void parsePDBAltPath(StringRef AltPath) {
SmallString<128> Buf;
StringRef PDBBasename =
sys::path::filename(Config->PDBPath, sys::path::Style::windows);
StringRef BinaryExtension =
sys::path::extension(Config->OutputFile, sys::path::Style::windows);
if (!BinaryExtension.empty())
BinaryExtension = BinaryExtension.substr(1); // %_EXT% does not include '.'.
size_t Cursor = 0;
while (Cursor < AltPath.size()) {
size_t FirstMark, SecondMark;
if ((FirstMark = AltPath.find('%', Cursor)) == StringRef::npos ||
(SecondMark = AltPath.find('%', FirstMark + 1)) == StringRef::npos) {
Buf.append(AltPath.substr(Cursor));
break;
}
Buf.append(AltPath.substr(Cursor, FirstMark - Cursor));
StringRef Var = AltPath.substr(FirstMark, SecondMark - FirstMark + 1);
if (Var.equals_lower("%_pdb%"))
Buf.append(PDBBasename);
else if (Var.equals_lower("%_ext%"))
Buf.append(BinaryExtension);
else {
warn("only %_PDB% and %_EXT% supported in /pdbaltpath:, keeping " +
Var + " as literal");
Buf.append(Var);
}
Cursor = SecondMark + 1;
}
Config->PDBAltPath = Buf;
}
```
Take your pick :-)
https://reviews.llvm.org/D52942
More information about the llvm-commits
mailing list