[PATCH] D52942: lld-link: Implement support for %_PDB% and %_EXT% for /pdbaltpath:.

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: COFF/Driver.cpp:827
+static void parsePDBAltPath(StringRef AltPath) {
+  // link.exe replaces each %foo% in AltPath with the contents of environment
+  // variable foo, and adds the two magic env vars _PDB (expands to the basename
----------------
Move this comment before this function definition to make it a function comment.


================
Comment at: COFF/Driver.cpp:834
+
+  llvm::SmallString<128> Buf;
+  StringRef PDBBasename =
----------------
I think you can omit `llvm::`.


================
Comment at: COFF/Driver.cpp:843
+  // Invariant:
+  //   +--------- Cursor ('a...' might be the empty string).
+  //   |   +----- FirstMark
----------------
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
      ....
  }


https://reviews.llvm.org/D52942