sammccall added a comment.
Hmm, I looked into the empty diagnostic ranges, and it overlaps here a lot.
Feel free to go ahead and land this and I can merge, otherwise I can include
this fix there.
Regardless of how it gets landed, thanks for finding and fixing this, this
explains a lot of weird behavior!
================
Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:124
+/// Convert a clang::SourceLocation to a clangd::Position
+Position SourceLocToClangdPosition(const SourceManager &SrcMgr,
+ SourceLocation Location) {
----------------
sammccall wrote:
> nit: functions are `lowerCamelCase`.
> Here and below.
nit: consider just `toPosition`, with the location as the first argument
And similarly below. The shorter names and putting the main param first make
the callsites easier to read I think.
================
Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:200
public:
- StoreDiagsConsumer(std::vector<DiagWithFixIts> &Output) : Output(Output) {}
+ StoreDiagsConsumer(const LangOptions &Options,
+ std::vector<DiagWithFixIts> &Output)
----------------
Actually I think the DiagnosticConsumer interface should pass you the language
options itself, if you override the lifecycle methods.
https://reviews.llvm.org/D40860
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
