[PATCH] D43246: [clangd] Assert path is absolute when assigning to URIForFile.

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Feb 13 11:45:47 PST 2018


sammccall added inline comments.


================
Comment at: clangd/Protocol.h:52
 
 struct URIForFile {
+  URIForFile() = default;
----------------
I don't like how the API changes here take us further away from the other structs in this file.
Why does this one enforce invariants, when the others don't?

If we do want to make this more "safe", I'd suggest making it look more like a URI string (since that's what its protocol role is), like:

```
class LSPURI {
  LSPURI(StringRef URI) { assert if bad }
  static LSPURI ForFile(StringRef AbsPath) { assert if bad }
  operator string() { return URI::createFile(File).toString(); }
  StringRef file() { return AbsFile; }
  operator bool() { return !File.empty(); } 
  // move/copy constructor/assignment defaulted

 private:
  string AbsFile;
}
```

But the approach Eric suggested does seem promising: more consistent with how we handle invariants in protocol structs.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43246





More information about the cfe-commits mailing list