[PATCH] D36150: [clangd] LSP extension to switch between source/header file

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 17 03:22:52 PDT 2017


ilya-biryukov added inline comments.


================
Comment at: clangd/ClangdServer.cpp:295
+  std::string DEFAULT_SOURCE_EXTENSIONS[] = { ".cpp", ".c", ".cc", ".cxx",
+    ".c++", ".C", ".m", ".mm" };  
+  std::string DEFAULT_HEADER_EXTENSIONS[] = { ".h", ".hh", ".hpp", ".hxx",
----------------
ilya-biryukov wrote:
> We should check all extensions in both upper-case and lower-case, not just `.c` and `.C`
> (ideally, we could use case-insensitive comparisons).
It turns out there's a very simple way to compare case-insetively. `StringRef(str).compare_lower(str2)`.
Could we use that instead of extension duplicates?


================
Comment at: clangd/ClangdServer.cpp:302
+
+  std::string pathDataRef = std::string(path.data());
+  bool isSourceFile = false, foundExtension = false;
----------------
ilya-biryukov wrote:
> `path` is already a `StringRef`, no need to convert it. 
> `std::string` is also implicitly convertible to `StringRef`, you could pass `std::string` to every function that accepts a `StringRef`
I don't think this comment was addressed. Why do we need `pathDataRef` variable? 


https://reviews.llvm.org/D36150





More information about the cfe-commits mailing list