Fixes PR38940.
Details
Details
Diff Detail
Diff Detail
- Repository
- rL LLVM
Event Timeline
| COFF/Driver.cpp | ||
|---|---|---|
| 827 ↗ | (On Diff #168515) | Move this comment before this function definition to make it a function comment. |
| 834 ↗ | (On Diff #168515) | I think you can omit llvm::. |
| 843 ↗ | (On Diff #168515) | 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
....
} |
Comment Actions
Thanks!
| COFF/Driver.cpp | ||
|---|---|---|
| 843 ↗ | (On Diff #168515) | 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 :-) |
Comment Actions
Hmm, okay, it seems all the choices are not that different, so I'm fine with your original one. Thank you for writing all the pieces of code by the way! :)
LGTM