ilya-biryukov added inline comments.
================ Comment at: clangd/ClangdLSPServer.h:132 - RealFileSystemProvider FSProvider; /// Options used for code completion ---------------- hokein wrote: > sammccall wrote: > > ilya-biryukov wrote: > > > Could we instead call `getRealFS()` when we try to initialize a > > > clang-tidy options provider in `main()` and avoid changing this? > > > To avoid adding extra non-real-fs "modes of operation" to > > > `ClangdLSPServer`. Unless you see other uses for this. > > We already have out-of-tree modifications to ClangdLSPServer to use > > non-real FSes. > > Given that, I think this change is OK... though better still might be to > > move it into `ClangdServer::Options` > Yes, this is the main reason I did this change. It still feels that `ClangdLSPServcer` is closely tied to real fs, so I don't see how that makes things simpler. I wouldn't call the presence of out-of-tree modifications a good reason to do this change, at least not without tests and comments explaining why this needs to be configurable. WRT to `ClangdServer::Options`, I believe this goes back to the previous discussion we had about putting the non-data configuration parameters (Index+FSProvider+CompilationsDB). I'd say put `CompilationsDB` into `Options` as well if you plan to put FSProvider, they should really live together (i.e. the reason we have out-of-tree modifications is closely tied to our custom CDB, so it makes sense for both to stay together). FWIW, I still think they're FSProvider+CDB+Index should be separate parameters, but that goes back to the earlier conversation we had with @sammccall when the `Options` were added in the first place. Not a big deal, just wanted to convey the intention of my original comment. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55256/new/ https://reviews.llvm.org/D55256 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
