Author: Sam McCall Date: 2020-06-30T15:06:15+02:00 New Revision: 72568984b8040fa8de70a6b4f9c63b6a5aac8d3f
URL: https://github.com/llvm/llvm-project/commit/72568984b8040fa8de70a6b4f9c63b6a5aac8d3f DIFF: https://github.com/llvm/llvm-project/commit/72568984b8040fa8de70a6b4f9c63b6a5aac8d3f.diff LOG: [clangd] Suppress GCC -Woverloaded-virtual by renaming ThreadsafeFS extension point Summary: By making all overloads non-virtual and delegating to a differently-named private method, we avoid any (harmless) name-hiding in the subclasses. Reviewers: kadircet Subscribers: kristof.beyls, ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits, Quuxplusone, dblaikie Tags: #clang Differential Revision: https://reviews.llvm.org/D82793 Added: Modified: 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 Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp index ca0a76db78f4..b71afa0b1619 100644 --- a/clang-tools-extra/clangd/Preamble.cpp +++ b/clang-tools-extra/clangd/Preamble.cpp @@ -229,9 +229,8 @@ struct ScannedPreamble { llvm::Expected<ScannedPreamble> scanPreamble(llvm::StringRef Contents, const tooling::CompileCommand &Cmd) { class EmptyFS : public ThreadsafeFS { - public: - llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> - view(llvm::NoneType) const override { + private: + llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> viewImpl() const override { return new llvm::vfs::InMemoryFileSystem; } }; diff --git a/clang-tools-extra/clangd/support/ThreadsafeFS.cpp b/clang-tools-extra/clangd/support/ThreadsafeFS.cpp index a25ac773bfa2..cadda8efa095 100644 --- a/clang-tools-extra/clangd/support/ThreadsafeFS.cpp +++ b/clang-tools-extra/clangd/support/ThreadsafeFS.cpp @@ -83,7 +83,7 @@ ThreadsafeFS::view(PathRef CWD) const { } 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. diff --git a/clang-tools-extra/clangd/support/ThreadsafeFS.h b/clang-tools-extra/clangd/support/ThreadsafeFS.h index aa6825fb3999..ad0dd6425811 100644 --- a/clang-tools-extra/clangd/support/ThreadsafeFS.h +++ b/clang-tools-extra/clangd/support/ThreadsafeFS.h @@ -30,20 +30,25 @@ class ThreadsafeFS { 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; +private: + llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> viewImpl() const override; }; } // namespace clangd diff --git a/clang-tools-extra/clangd/unittests/ClangdTests.cpp b/clang-tools-extra/clangd/unittests/ClangdTests.cpp index e1f02c897fe3..04e97d0fa3f9 100644 --- a/clang-tools-extra/clangd/unittests/ClangdTests.cpp +++ b/clang-tools-extra/clangd/unittests/ClangdTests.cpp @@ -273,12 +273,13 @@ int b = a; TEST_F(ClangdVFSTest, PropagatesContexts) { static Key<int> Secret; struct ContextReadingFS : public ThreadsafeFS { - IntrusiveRefCntPtr<llvm::vfs::FileSystem> - view(llvm::NoneType) const override { + mutable int Got; + + private: + IntrusiveRefCntPtr<llvm::vfs::FileSystem> viewImpl() const override { Got = Context::current().getExisting(Secret); return buildTestFS({}); } - mutable int Got; } FS; struct Callbacks : public ClangdServer::Callbacks { void onDiagnosticsReady(PathRef File, llvm::StringRef Version, @@ -925,12 +926,17 @@ TEST_F(ClangdVFSTest, ChangedHeaderFromISystem) { // preamble again. (They should be using the preamble's stat-cache) TEST(ClangdTests, PreambleVFSStatCache) { class StatRecordingFS : public ThreadsafeFS { + llvm::StringMap<unsigned> &CountStats; + public: + // If relative paths are used, they are resolved with testPath(). + llvm::StringMap<std::string> Files; + StatRecordingFS(llvm::StringMap<unsigned> &CountStats) : CountStats(CountStats) {} - IntrusiveRefCntPtr<llvm::vfs::FileSystem> - view(llvm::NoneType) const override { + private: + IntrusiveRefCntPtr<llvm::vfs::FileSystem> viewImpl() const override { class StatRecordingVFS : public llvm::vfs::ProxyFileSystem { public: StatRecordingVFS(IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS, @@ -954,10 +960,6 @@ TEST(ClangdTests, PreambleVFSStatCache) { return IntrusiveRefCntPtr<StatRecordingVFS>( new StatRecordingVFS(buildTestFS(Files), CountStats)); } - - // If relative paths are used, they are resolved with testPath(). - llvm::StringMap<std::string> Files; - llvm::StringMap<unsigned> &CountStats; }; llvm::StringMap<unsigned> CountStats; diff --git a/clang-tools-extra/clangd/unittests/TestFS.h b/clang-tools-extra/clangd/unittests/TestFS.h index a487a361bfe1..1c713c78f7bf 100644 --- a/clang-tools-extra/clangd/unittests/TestFS.h +++ b/clang-tools-extra/clangd/unittests/TestFS.h @@ -33,8 +33,7 @@ buildTestFS(llvm::StringMap<std::string> const &Files, // 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); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
