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

Alex Lorenz via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 18 09:06:31 PDT 2017


arphaman added inline comments.


================
Comment at: clangd/ClangdServer.cpp:296
+
+  const int sourceSize = sizeof(DEFAULT_SOURCE_EXTENSIONS) / sizeof(DEFAULT_SOURCE_EXTENSIONS[0]);
+  const int headerSize = sizeof(DEFAULT_HEADER_EXTENSIONS) / sizeof(DEFAULT_HEADER_EXTENSIONS[0]);
----------------
You can use LLVM's function `array_lengthof` here and on the next line instead.


================
Comment at: clangd/ClangdServer.cpp:302
+  std::string *p;
+  p = std::find(DEFAULT_SOURCE_EXTENSIONS,
+                DEFAULT_SOURCE_EXTENSIONS + sourceSize,
----------------
It might be better to use a `StringSet` instead of an array of strings and use one lowercase/uppercase lookup:

```
llvm::StringSet<> DEFAULT_SOURCE_EXTENSIONS[] = {".cpp", ".c", ".cc", ".cxx", ".c++", ".m", ".mm"};
// Handles both lower and uppercase extensions and source/header files in one if/else construct:
if (DEFAULT_SOURCE_EXTENSIONS.count(llvm::sys::path::extension(path).lower())) {
  ...
  isSourceFile = true;
} else if (DEFAULT_HEADER_EXTENSIONS.count(llvm::sys::path::extension(path).lower())) {
  ...
  isSourceFile = false;
}
```


https://reviews.llvm.org/D36150





More information about the cfe-commits mailing list