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

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 21 01:29:33 PDT 2017

ilya-biryukov added a comment.

The function in `ClangdServer` seems to be getting more complicated and more complicated, mostly because of mixing up `std::string`, `StringRef`, `SmallString`.
Here's a slightly revamped implementation of your function, hope you'll find it (or parts of it) useful to simplify your implementation and address remaining comments.

  // Note that the returned value is now llvm::Optional to clearly indicate no matching file was found.
  llvm::Optional<Path> ClangdServer::switchSourceHeader(PathRef Path) {
    const StringRef SourceExtensions[] = {/*...*/; // only lower-case extensions
    const StringRef HeaderExtensions[] = {/*...*/}; // ...
    StringRef PathExt = llvm::sys::path::extension(Path);
    // Lookup in a list of known extensions.
    auto SourceIter = std::find(SourceExtensions, /*end iterator*/, PathExt, [](StringRef LHS, StringRef RHS) { return LHS.equals_lower(RHS); });
    bool IsSource = SourceIter != /*end iterator*/;
    // ...
    bool IsHeader = ...;
    // We can only switch between extensions known extensions.
    if (!IsSource && !IsHeader)
      return llvm::None;
    // Array to lookup extensions for the switch. An opposite of where original extension was found.
    ArrayRef<StringRef> NewExts = IsSource ? HeaderExtensions : SourceExtensions;
    // Storage for the new path.
    SmallString<128> NewPath;
    // Instance of vfs::FileSystem, used for file existence checks.
    auto FS = FSProvider.getTaggedFileSystem(Path).Value;
    // Loop through switched extension candidates.
    for (StringRef NewExt : NewExts) { 
      llvm::sys::path::replace_extension(NewPath, NewExt)
      if (FS->exists(NewPath))
        return NewPath.str().str(); // First str() to convert from SmallString to StringRef, second to convert from StringRef to std::string
      // Also check NewExt in upper-case, just in case.
      llvm::sys::path::replace_extension(NewPath, NewExt.upper())
      if (FS->exists(NewPath))
        return NewPath.str().str();
    return llvm::None;

Comment at: clangd/ClangdServer.cpp:404
+        std::string returnValue = "file://" + std::string(test.str());
+        returnValue = URI::unparse(URI::fromUri(returnValue));
+        return Path(returnValue);
No need to create a `URI` here, this should be handled outside `ClangdServer`, just return a path with replaced extension.


More information about the cfe-commits mailing list