kadircet added inline comments.
================
Comment at: clang-tools-extra/clangd/HeaderSourceSwitch.cpp:21
+// file.
+class CollectIndexableLocalDecls {
+public:
----------------
this class doesn't seem to have any state(apart from saving AST in constructor
and using it in call to collect), why not just have a `vector<Decl>
collectDecls(ParsedAST& AST);` ?
================
Comment at: clang-tools-extra/clangd/HeaderSourceSwitch.cpp:61
+
+llvm::Optional<std::string> resolveURI(const char *FileURI,
+ llvm::StringRef HintPath) {
----------------
It seems like we have a bunch of different implementations for this function in
background.cpp, codecomplete.cpp, ... Basically any place calling
`URI::resolve`, do you mind adding an overload for `URI::resolve` that'll take
a `const char*` instead of a `URI`?
================
Comment at: clang-tools-extra/clangd/HeaderSourceSwitch.cpp:78
+
+llvm::Optional<Path> getCorrespondingHeaderOrSource(const Path &OriginalFile,
+ ParsedAST &AST,
----------------
As discussed in the design for define out-of-line, this function's dependency
on AST and Index should rather be optional.
- if we have AST, then we can boost the files containing the canonical
declarations for symbols defined in the main file.
- if we have just the Index, currently there's nothing much we can do, but we
can put a fixme to add a new endpoint to symbol index for "fetching symbols in
a given file",
then we can use symbol information for declaration/definition files without
an AST.
- if we have AST+Index, we can boost the files containing the definition for
symbols declared in the main file and vice-versa(what you already do in this
patch).
- If we don't have anything we'll just make use of the filename, by changing
the extension, as current `ClangdServer::switchSourceHeader` implementation
does.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67907/new/
https://reviews.llvm.org/D67907
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
