[PATCH] D82793: [clangd] Suppress GCC -Woverloaded-virtual by renaming ThreadsafeFS extension point

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, 
ilya-biryukov, kristof.beyls.
Herald added a project: clang.

By making all overloads non-virtual and delegating to a differently-named
private method, we avoid any (harmless) name-hiding in the subclasses.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82793

Files:
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/support/ThreadsafeFS.cpp
  clang-tools-extra/clangd/support/ThreadsafeFS.h
  clang-tools-extra/clangd/unittests/ClangdTests.cpp
  clang-tools-extra/clangd/unittests/TestFS.h


Index: clang-tools-extra/clangd/unittests/TestFS.h
===================================================================
--- clang-tools-extra/clangd/unittests/TestFS.h
+++ clang-tools-extra/clangd/unittests/TestFS.h
@@ -33,8 +33,7 @@
 // A VFS provider that returns TestFSes containing a provided set of files.
 class MockFS : public ThreadsafeFS {
 public:
-  IntrusiveRefCntPtr<llvm::vfs::FileSystem>
-  view(llvm::NoneType) const override {
+  IntrusiveRefCntPtr<llvm::vfs::FileSystem> viewImpl() const override {
     return buildTestFS(Files, Timestamps);
   }
 
Index: clang-tools-extra/clangd/unittests/ClangdTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ClangdTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdTests.cpp
@@ -273,8 +273,7 @@
 TEST_F(ClangdVFSTest, PropagatesContexts) {
   static Key<int> Secret;
   struct ContextReadingFS : public ThreadsafeFS {
-    IntrusiveRefCntPtr<llvm::vfs::FileSystem>
-    view(llvm::NoneType) const override {
+    IntrusiveRefCntPtr<llvm::vfs::FileSystem> viewImpl() const override {
       Got = Context::current().getExisting(Secret);
       return buildTestFS({});
     }
@@ -929,8 +928,7 @@
     StatRecordingFS(llvm::StringMap<unsigned> &CountStats)
         : CountStats(CountStats) {}
 
-    IntrusiveRefCntPtr<llvm::vfs::FileSystem>
-    view(llvm::NoneType) const override {
+    IntrusiveRefCntPtr<llvm::vfs::FileSystem> viewImpl() const override {
       class StatRecordingVFS : public llvm::vfs::ProxyFileSystem {
       public:
         StatRecordingVFS(IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS,
Index: clang-tools-extra/clangd/support/ThreadsafeFS.h
===================================================================
--- clang-tools-extra/clangd/support/ThreadsafeFS.h
+++ clang-tools-extra/clangd/support/ThreadsafeFS.h
@@ -30,20 +30,25 @@
   virtual ~ThreadsafeFS() = default;
 
   /// Obtain a vfs::FileSystem with an arbitrary initial working directory.
-  virtual llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
-  view(llvm::NoneType CWD) const = 0;
+  llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
+  view(llvm::NoneType CWD) const {
+    return viewImpl();
+  }
 
   /// Obtain a vfs::FileSystem with a specified working directory.
   /// If the working directory can't be set (e.g. doesn't exist), logs and
   /// returns the FS anyway.
-  virtual llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
-  view(PathRef CWD) const;
+  llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> view(PathRef CWD) const;
+
+private:
+  /// Overridden by implementations to provide a vfs::FileSystem.
+  /// This is distinct from view(NoneType) to avoid GCC's -Woverloaded-virtual.
+  virtual llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> viewImpl() const = 0;
 };
 
 class RealThreadsafeFS : public ThreadsafeFS {
 public:
-  llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
-      view(llvm::NoneType) const override;
+  llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> viewImpl() const override;
 };
 
 } // namespace clangd
Index: clang-tools-extra/clangd/support/ThreadsafeFS.cpp
===================================================================
--- clang-tools-extra/clangd/support/ThreadsafeFS.cpp
+++ clang-tools-extra/clangd/support/ThreadsafeFS.cpp
@@ -83,7 +83,7 @@
 }
 
 llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
-RealThreadsafeFS::view(llvm::NoneType) const {
+RealThreadsafeFS::viewImpl() const {
   // Avoid using memory-mapped files.
   // FIXME: Try to use a similar approach in Sema instead of relying on
   //        propagation of the 'isVolatile' flag through all layers.
Index: clang-tools-extra/clangd/Preamble.cpp
===================================================================
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -230,8 +230,7 @@
 scanPreamble(llvm::StringRef Contents, const tooling::CompileCommand &Cmd) {
   class EmptyFS : public ThreadsafeFS {
   public:
-    llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
-    view(llvm::NoneType) const override {
+    llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> viewImpl() const override {
       return new llvm::vfs::InMemoryFileSystem;
     }
   };

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