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

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE325337: [clangd] Assert path is absolute when assigning to URIForFile. (authored by ibiryukov, committed by ). Changed prior to commit: https://reviews.llvm.org/D43246?vs=134592=134593#toc

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

ilya-biryukov updated this revision to Diff 134592. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Rename getFile() to file() - Removed the Path.h include - Rebased onto head Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43246 Files:

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

ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: clangd/Protocol.h:52 struct URIForFile { + URIForFile() = default; sammccall wrote: > ioeric wrote: > > ilya-biryukov wrote: > > > sammccall wrote: > > > > I

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

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. LG if we want to do this (please getFile -> file though!) Comment at: clangd/Protocol.h:52 struct URIForFile { + URIForFile() = default; ioeric

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

ioeric added inline comments. Comment at: clangd/Protocol.h:52 struct URIForFile { + URIForFile() = default; ilya-biryukov wrote: > sammccall wrote: > > I don't like how the API changes here take us further away from the other > > structs in this file. > >

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

ilya-biryukov added a comment. In https://reviews.llvm.org/D43246#1006525, @ioeric wrote: > I think another option to prevent the bug in r325029 from happening would be > providing a canonical approach for retrieving (absolute) paths from > `FileManager`. We already have code in symbol

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

ilya-biryukov updated this revision to Diff 134194. ilya-biryukov added a comment. - Remove URIForFile::setFile(), rely on copy and move constructors instead. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43246 Files: clangd/ClangdLSPServer.cpp clangd/Protocol.cpp

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

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

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

ioeric added a comment. I think another option to prevent the bug in r325029 from happening would be providing a canonical approach for retrieving (absolute) paths from `FileManager`. We already have code in symbol collector that does this, and we have similar code in XRefs.cpp now. We should

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

ilya-biryukov created this revision. ilya-biryukov added reviewers: hokein, ioeric, sammccall. Herald added subscribers: jkorous-apple, klimek. The assertion will point directly to misbehaving code, so that debugging related problems (like the one fixed by r325029) is easier. Repository: rCTE