ilya-biryukov added a comment.

Could you please run `clang-format` on every submission?



================
Comment at: clangd/GlobalCompilationDatabase.cpp:98
+  {
+    CompileCommandsDir = "/";  
+    return tryLoadDatabaseFromPath(CompileCommandsDir.getValue());  
----------------
Is this some kind of accidental change? Why do we need to assign `"/"` to 
`CompileCommandsDir`?


================
Comment at: clangd/tool/ClangdMain.cpp:20
 #include <thread>
+#include <unistd.h>
 
----------------
We certainly don't need that include.


================
Comment at: clangd/tool/ClangdMain.cpp:79
+    CompileCommandsDirPath = llvm::None;
+  else
+  {
----------------
This should be something like:

```
  if (CompileCommandsDir.empty()) {
    //...
    CompileCommandsDirPath = llvm::None;
  } else if (!is_absolute(...)) {
    //....
    CompileCommandsDirPath = llvm::None;
  } else if (!exists(...)) {
    // ....
    CompileCommandsDirPath = llvm::None;
  } else {
    CompileCommandsDirPath = CompileCommandsDir;
  }
```



  - It will have less nesting, therefore making code more readable.
  - It will fix an error in the current implementation. (Currently, `exists` 
check will run on an empty string if `-compile-commands-dir` is not an absolute 
path).




https://reviews.llvm.org/D37150



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to