[PATCH] D29973: [PDB] Support following Type server records

[Zachary Turner via Phabricator via llvm-commits]

zturner added inline comments.


================
Comment at: llvm/lib/DebugInfo/PDB/Native/PDBTypeServerHandler.cpp:33
+
+void PDBTypeServerHandler::addSearchPath(StringRef Path) {
+  if (Path.empty() || !sys::fs::is_directory(Path))
----------------
ruiu wrote:
> Calling this function more than once with the same argument seems to add the same element to to `SearchPaths`. Is this a desired behavior?
I don't feel strongly either way, if you think it's better to change it I can.


================
Comment at: llvm/lib/DebugInfo/PDB/Native/PDBTypeServerHandler.cpp:80
+      // doesn't match and we should continue searching.
+      ignoreErrors(std::move(EC));
+      continue;
----------------
ruiu wrote:
> I believe that evaluating EC as a boolean value clears "checked" bits of the object, so you can safely ignore errors without calling this.
`Error` only clears checked bit if it was in a success state.  You have to check it regardless of if it's success or failure, but if it's an error condition you also have to handle it.

```
  /// Bool conversion. Returns true if this Error is in a failure state,
  /// and false if it is in an accept state. If the error is in a Success state
  /// it will be considered checked.
  explicit operator bool() {
    setChecked(getPtr() == nullptr);
    return getPtr() != nullptr;
  }
```


https://reviews.llvm.org/D29973