[clang-tools-extra] [clangd] Add tweak to override pure virtuals (PR #139348)
qinkunbao wrote: Hi, it looks like it still needs some time to fix the build bot errors. I am going to revert this PR according to https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy Please reland this PR after the error is fixed. Thanks! https://github.com/llvm/llvm-project/pull/139348 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add tweak to override pure virtuals (PR #139348)
qinkunbao wrote: Hi @marcogmaia and @kadircet , I believe the this PR broke a few buildbots. Would you mind taking a look? ``` [ RUN ] OverridePureVirtualsTests.MultiAccessSpecifiersOverride Built preamble of size 211928 for file /clangd-test/TestTU.cpp version null in 0.00 seconds #0 0x64c6a459954e llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /home/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/llvm/lib/Support/Unix/Signals.inc:834:13 #1 0x64c6a45964a6 llvm::sys::RunSignalHandlers() /home/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/llvm/lib/Support/Signals.cpp:105:18 #2 0x64c6a459aab8 SignalHandler(int, siginfo_t*, void*) /home/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/llvm/lib/Support/Unix/Signals.inc:426:38 #3 0x77593c045250 (/lib/x86_64-linux-gnu/libc.so.6+0x45250) #4 0x77593c0a3f1c pthread_kill (/lib/x86_64-linux-gnu/libc.so.6+0xa3f1c) #5 0x77593c04519e raise (/lib/x86_64-linux-gnu/libc.so.6+0x4519e) #6 0x77593c028902 abort (/lib/x86_64-linux-gnu/libc.so.6+0x28902) #7 0x64c6a399fbdc (/home/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/tools/clang/tools/extra/clangd/unittests/./ClangdTests+0x3377bdc) #8 0x64c6a399eb6e __sanitizer::Die() /home/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_termination.cpp:52:5 #9 0x64c6a39ab079 (/home/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/tools/clang/tools/extra/clangd/unittests/./ClangdTests+0x3383079) #10 0x64c6a3af6673 llvm::DenseMapBase, llvm::detail::DenseMapPair>, clang::AccessSpecifier, unsigned int, llvm::DenseMapInfo, llvm::detail::DenseMapPair>::initEmpty() /home/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/llvm/include/llvm/ADT/DenseMap.h:443:18 #11 0x64c6a3af725d grow /home/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/llvm/include/llvm/ADT/DenseMap.h:0:64 #12 0x64c6a3af725d llvm::detail::DenseMapPair* llvm::DenseMapBase, llvm::detail::DenseMapPair>, clang::AccessSpecifier, unsigned int, llvm::DenseMapInfo, llvm::detail::DenseMapPair>::InsertIntoBucketImpl(clang::AccessSpecifier const&, llvm::detail::DenseMapPair*) /home/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/llvm/include/llvm/ADT/DenseMap.h:0:0 #13 0x64c6a3af7032 InsertIntoBucket /home/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/llvm/include/llvm/ADT/DenseMap.h:596:29 #14 0x64c6a3af7032 std::__1::pair, llvm::detail::DenseMapPair, false>, bool> llvm::DenseMapBase, llvm::detail::DenseMapPair>, clang::AccessSpecifier, unsigned int, llvm::DenseMapInfo, llvm::detail::DenseMapPair>::try_emplace<>(clang::AccessSpecifier const&) /home/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/llvm/include/llvm/ADT/DenseMap.h:292:17 #15 0x64c6a3af6af3 isHandleInSync /home/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/llvm/include/llvm/ADT/EpochTracker.h:72:43 #16 0x64c6a3af6af3 operator-> /home/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/llvm/include/llvm/ADT/DenseMap.h:1289:5 #17 0x64c6a3af6af3 llvm::MapVector, llvm::DenseMap, llvm::detail::DenseMapPair>, llvm::SmallVector>, 0u>>::operator[](clang::AccessSpecifier const&) /home/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/llvm/include/llvm/ADT/MapVector.h:103:15 #18 0x64c6a3af40a5 collectMissingPureVirtuals /home/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/clang-tools-extra/clangd/refactor/tweaks/OverridePureVirtuals.cpp:262:55 #19 0x64c6a3af40a5 clang::clangd::(anonymous namespace)::OverridePureVirtuals::apply(clang::clangd::Tweak::Selection const&) /home/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/clang-tools-extra/clangd/refactor/tweaks/OverridePureVirtuals.cpp:298:3 #20 0x64c6a44bd94e operator() /home/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp:73:33 #21 0x64c6a44bd94e bool llvm::function_ref::callback_fn(long, clang::clangd::SelectionTree) /home/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/llvm/include/llvm/ADT/STLFunctionalExtras.h:46:12 #22 0x64c6a67a3091 operator() /home/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/llvm/include/llvm/ADT/STLFunctionalExtras.h:69:12 #23 0x64c6a67a3091 clang::clangd::SelectionTree::createEach(clang::ASTContext&, clang::syntax::TokenBuffer const&, unsigned int, unsigned int, llvm::function_ref) /home/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/clang-tools-extra/clangd/Selection.cpp:1062:9 #24 0x64c6a44bc289 has_value /home/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/libcxx_install_ubsan/include/c++/v1/optional:361:82 #25 0x64c6a44bc289 operator bool /home/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/libcxx_install_ubsan/include/c++/v1/optional:
[clang-tools-extra] [clangd] Add tweak to override pure virtuals (PR #139348)
llvm-ci wrote:
LLVM Buildbot has detected a new failure on builder `clang-aarch64-quick`
running on `linaro-clang-aarch64-quick` while building `clang-tools-extra` at
step 5 "ninja check 1".
Full details are available at:
https://lab.llvm.org/buildbot/#/builders/65/builds/19884
Here is the relevant piece of the build log for the reference
```
Step 5 (ninja check 1) failure: stage 1 checked (failure)
TEST 'Clangd Unit Tests :: ./ClangdTests/74/165' FAILED
Script(shard):
--
GTEST_OUTPUT=json:/home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/tools/clang/tools/extra/clangd/unittests/./ClangdTests-Clangd
Unit Tests-3258315-74-165.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=165
GTEST_SHARD_INDEX=74
/home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/tools/clang/tools/extra/clangd/unittests/./ClangdTests
--
Note: This is test shard 75 of 165.
[==] Running 8 tests from 8 test suites.
[--] Global test environment set-up.
[--] 1 test from LSPTest
[ RUN ] LSPTest.CDBConfigIntegration
[0;34m<<< initialize: {}
[0m[0;33m<-- initialize(0)
[0m[0;33m--> reply:initialize(0) 1 ms
[0m[0;34m>>> reply: {
"capabilities": {
"astProvider": true,
"callHierarchyProvider": true,
"clangdInlayHintsProvider": true,
"codeActionProvider": true,
"compilationDatabase": {
"automaticReload": true
},
"completionProvider": {
"resolveProvider": false,
"triggerCharacters": [
".",
"<",
">",
":",
"\"",
"/",
"*"
]
},
"declarationProvider": true,
"definitionProvider": true,
"documentFormattingProvider": true,
"documentHighlightProvider": true,
"documentLinkProvider": {
"resolveProvider": false
},
"documentOnTypeFormattingProvider": {
"firstTriggerCharacter": "\n",
"moreTriggerCharacter": []
},
"documentRangeFormattingProvider": {
"rangesSupport": true
},
"documentSymbolProvider": true,
...
```
https://github.com/llvm/llvm-project/pull/139348
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add tweak to override pure virtuals (PR #139348)
llvm-ci wrote: LLVM Buildbot has detected a new failure on builder `premerge-monolithic-linux` running on `premerge-linux-1` while building `clang-tools-extra` at step 7 "test-build-unified-tree-check-all". Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/38756 Here is the relevant piece of the build log for the reference ``` Step 7 (test-build-unified-tree-check-all) failure: test (failure) ... PASS: lld :: COFF/driver-opt.s (98780 of 101740) PASS: lld :: COFF/entry-inference-mingw.s (98781 of 101740) PASS: lld :: COFF/exported-dllmain.test (98782 of 101740) PASS: lld :: COFF/deploadflag-cfg-x64.s (98783 of 101740) PASS: lld :: COFF/export-weak-alias.s (98784 of 101740) PASS: lld :: COFF/export-thunk.test (98785 of 101740) PASS: lld :: COFF/duplicate-imp-func.s (98786 of 101740) PASS: lld :: COFF/gfids-corrupt.s (98787 of 101740) PASS: lit :: googletest-timeout.py (98788 of 101740) TIMEOUT: MLIR :: Examples/standalone/test.toy (98789 of 101740) TEST 'MLIR :: Examples/standalone/test.toy' FAILED Exit Code: 1 Timeout: Reached timeout of 60 seconds Command Output (stdout): -- # RUN: at line 1 "/etc/cmake/bin/cmake" "/build/buildbot/premerge-monolithic-linux/llvm-project/mlir/examples/standalone" -G "Ninja" -DCMAKE_CXX_COMPILER=/usr/bin/clang++ -DCMAKE_C_COMPILER=/usr/bin/clang -DLLVM_ENABLE_LIBCXX=OFF -DMLIR_DIR=/build/buildbot/premerge-monolithic-linux/build/lib/cmake/mlir -DLLVM_USE_LINKER=lld -DPython3_EXECUTABLE="/usr/bin/python3.10" # executed command: /etc/cmake/bin/cmake /build/buildbot/premerge-monolithic-linux/llvm-project/mlir/examples/standalone -G Ninja -DCMAKE_CXX_COMPILER=/usr/bin/clang++ -DCMAKE_C_COMPILER=/usr/bin/clang -DLLVM_ENABLE_LIBCXX=OFF -DMLIR_DIR=/build/buildbot/premerge-monolithic-linux/build/lib/cmake/mlir -DLLVM_USE_LINKER=lld -DPython3_EXECUTABLE=/usr/bin/python3.10 # .---command stdout # | -- The CXX compiler identification is Clang 16.0.6 # | -- The C compiler identification is Clang 16.0.6 # | -- Detecting CXX compiler ABI info # | -- Detecting CXX compiler ABI info - done # | -- Check for working CXX compiler: /usr/bin/clang++ - skipped # | -- Detecting CXX compile features # | -- Detecting CXX compile features - done # | -- Detecting C compiler ABI info # | -- Detecting C compiler ABI info - done # | -- Check for working C compiler: /usr/bin/clang - skipped # | -- Detecting C compile features # | -- Detecting C compile features - done # | -- Looking for histedit.h # | -- Looking for histedit.h - found # | -- Found LibEdit: /usr/include (found version "2.11") # | -- Found ZLIB: /usr/lib/x86_64-linux-gnu/libz.so (found version "1.2.11") # | -- Found LibXml2: /usr/lib/x86_64-linux-gnu/libxml2.so (found version "2.9.13") # | -- Using MLIRConfig.cmake in: /build/buildbot/premerge-monolithic-linux/build/lib/cmake/mlir # | -- Using LLVMConfig.cmake in: /build/buildbot/premerge-monolithic-linux/build/lib/cmake/llvm # | -- Linker detection: unknown # | -- Performing Test LLVM_LIBSTDCXX_MIN # | -- Performing Test LLVM_LIBSTDCXX_MIN - Success # | -- Performing Test LLVM_LIBSTDCXX_SOFT_ERROR # | -- Performing Test LLVM_LIBSTDCXX_SOFT_ERROR - Success # | -- Performing Test CXX_SUPPORTS_CUSTOM_LINKER # | -- Performing Test CXX_SUPPORTS_CUSTOM_LINKER - Success # | -- Performing Test C_SUPPORTS_FPIC # | -- Performing Test C_SUPPORTS_FPIC - Success # | -- Performing Test CXX_SUPPORTS_FPIC ``` https://github.com/llvm/llvm-project/pull/139348 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add tweak to override pure virtuals (PR #139348)
kadircet wrote: thanks a lot again! https://github.com/llvm/llvm-project/pull/139348 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add tweak to override pure virtuals (PR #139348)
github-actions[bot] wrote: @marcogmaia Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our [build bots](https://lab.llvm.org/buildbot/). If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail [here](https://llvm.org/docs/MyFirstTypoFix.html#myfirsttypofix-issues-after-landing-your-pr). If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of [LLVM development](https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy). You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! https://github.com/llvm/llvm-project/pull/139348 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add tweak to override pure virtuals (PR #139348)
https://github.com/kadircet closed https://github.com/llvm/llvm-project/pull/139348 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add tweak to override pure virtuals (PR #139348)
marcogmaia wrote: Ping https://github.com/llvm/llvm-project/pull/139348 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add tweak to override pure virtuals (PR #139348)
@@ -70,6 +70,16 @@ Code completion Code actions +- New ``clangd`` code action: "Override pure virtual methods". When invoked on a + class definition, this action automatically generates C++ ``override`` + declarations for all pure virtual methods inherited from its base classes that + have not yet been implemented. The generated method stubs include a ``TODO`` marcogmaia wrote: Removed. https://github.com/llvm/llvm-project/pull/139348 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add tweak to override pure virtuals (PR #139348)
@@ -70,6 +70,16 @@ Code completion Code actions +- New ``clangd`` code action: "Override pure virtual methods". When invoked on a marcogmaia wrote: Agreed. Dropped. https://github.com/llvm/llvm-project/pull/139348 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add tweak to override pure virtuals (PR #139348)
https://github.com/marcogmaia updated
https://github.com/llvm/llvm-project/pull/139348
>From 76503bd8f5618b710e2909d1303de5d34723 Mon Sep 17 00:00:00 2001
From: Marco Maia
Date: Sat, 10 May 2025 00:48:39 -0300
Subject: [PATCH 01/14] [clangd] Add tweak to add pure virtual overrides
---
.../clangd/refactor/tweaks/CMakeLists.txt | 3 +-
.../refactor/tweaks/OverridePureVirtuals.cpp | 366
.../clangd/unittests/CMakeLists.txt | 1 +
.../tweaks/OverridePureVirtualsTests.cpp | 410 ++
4 files changed, 779 insertions(+), 1 deletion(-)
create mode 100644
clang-tools-extra/clangd/refactor/tweaks/OverridePureVirtuals.cpp
create mode 100644
clang-tools-extra/clangd/unittests/tweaks/OverridePureVirtualsTests.cpp
diff --git a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
index 59475b0dfd3d2..1d6e38088ad67 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
+++ b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
@@ -14,9 +14,9 @@ set(LLVM_LINK_COMPONENTS
add_clang_library(clangDaemonTweaks OBJECT
AddUsing.cpp
AnnotateHighlightings.cpp
- DumpAST.cpp
DefineInline.cpp
DefineOutline.cpp
+ DumpAST.cpp
ExpandDeducedType.cpp
ExpandMacro.cpp
ExtractFunction.cpp
@@ -24,6 +24,7 @@ add_clang_library(clangDaemonTweaks OBJECT
MemberwiseConstructor.cpp
ObjCLocalizeStringLiteral.cpp
ObjCMemberwiseInitializer.cpp
+ OverridePureVirtuals.cpp
PopulateSwitch.cpp
RawStringLiteral.cpp
RemoveUsingNamespace.cpp
diff --git a/clang-tools-extra/clangd/refactor/tweaks/OverridePureVirtuals.cpp
b/clang-tools-extra/clangd/refactor/tweaks/OverridePureVirtuals.cpp
new file mode 100644
index 0..b8880433fdd52
--- /dev/null
+++ b/clang-tools-extra/clangd/refactor/tweaks/OverridePureVirtuals.cpp
@@ -0,0 +1,366 @@
+//===--- AddPureVirtualOverride.cpp --*-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "refactor/Tweak.h"
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/Type.h"
+#include "clang/AST/TypeLoc.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/Support/FormatVariadic.h"
+#include
+#include
+#include
+#include
+#include
+#include
+
+namespace clang {
+namespace clangd {
+namespace {
+
+class OverridePureVirtuals : public Tweak {
+public:
+ const char *id() const final; // defined by REGISTER_TWEAK.
+ bool prepare(const Selection &Sel) override;
+ Expected apply(const Selection &Sel) override;
+ std::string title() const override { return "Override pure virtual methods";
}
+ llvm::StringLiteral kind() const override {
+return CodeAction::REFACTOR_KIND;
+ }
+
+private:
+ // Stores the CXXRecordDecl of the class being modified.
+ const CXXRecordDecl *CurrentDecl = nullptr;
+ // Stores pure virtual methods that need overriding, grouped by their
original
+ // access specifier.
+ std::map>
+ MissingMethodsByAccess;
+ // Stores the source locations of existing access specifiers in CurrentDecl.
+ std::map AccessSpecifierLocations;
+
+ // Helper function to gather information before applying the tweak.
+ void collectMissingPureVirtuals(const Selection &Sel);
+};
+
+REGISTER_TWEAK(OverridePureVirtuals)
+
+// Collects all unique, canonical pure virtual methods from a class and its
+// entire inheritance hierarchy. This function aims to find methods that
*could*
+// make a derived class abstract if not implemented.
+std::vector
+getAllUniquePureVirtualsFromHierarchy(const CXXRecordDecl *Decl) {
+ std::vector Result;
+ llvm::DenseSet VisitedCanonicalMethods;
+ // We declare it as a std::function because we are going to call it
+ // recursively.
+ std::function Collect;
+
+ Collect = [&](const CXXRecordDecl *CurrentClass) {
+if (!CurrentClass) {
+ return;
+}
+const CXXRecordDecl *Def = CurrentClass->getDefinition();
+if (!Def) {
+ return;
+}
+
+for (const CXXMethodDecl *M : Def->methods()) {
+ // Add if its canonical declaration hasn't been processed yet.
+ // This ensures each distinct pure virtual method signature is collected
+ // once.
+ if (M->isPureVirtual() &&
+ VisitedCanonicalMethods.insert(M->getCanonicalDecl()).second) {
+Result.emplace_back(M); // Store the specific declaration encountered.
+ }
+}
+
+for (const auto &BaseSpec : Def->bases()) {
+ if (const CXXRecordDecl *BaseDef =
+ BaseSpec.getType()->getAsCXXRecordDecl()) {
+Collect(BaseDe
[clang-tools-extra] [clangd] Add tweak to override pure virtuals (PR #139348)
@@ -70,6 +70,16 @@ Code completion Code actions +- New ``clangd`` code action: "Override pure virtual methods". When invoked on a kadircet wrote: nit: i'd drop `clangd`, it's already under clangd section. https://github.com/llvm/llvm-project/pull/139348 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add tweak to override pure virtuals (PR #139348)
@@ -70,6 +70,16 @@ Code completion Code actions +- New ``clangd`` code action: "Override pure virtual methods". When invoked on a + class definition, this action automatically generates C++ ``override`` + declarations for all pure virtual methods inherited from its base classes that + have not yet been implemented. The generated method stubs include a ``TODO`` kadircet wrote: i think content starting with `The generated method stubs include a ``TODO`` ...` is too much details for release notes, i'd probably drop. https://github.com/llvm/llvm-project/pull/139348 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add tweak to override pure virtuals (PR #139348)
https://github.com/kadircet approved this pull request. thanks and sorry for not getting back to this for so long. i don't think i'll get to follow up and looks like others are also busy. i have some usability concerns like: - Generating method bodies in class definitions instead of out-of-line in implementation files. - Providing implementations for all missing pure virtuals, without giving user a control might be noisy at times. - Not providing implementations for some overriden virtuals is also something i'd miss. But I guess most of these are improvements we can build up-on here. AFAICT, `::prepare` is both performance and resilient and we got the docs to figure out intent and fix bugs if need be. So thins LGTM as well. https://github.com/llvm/llvm-project/pull/139348 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add tweak to override pure virtuals (PR #139348)
https://github.com/kadircet edited https://github.com/llvm/llvm-project/pull/139348 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add tweak to override pure virtuals (PR #139348)
zwuis wrote: Do you need the help to merge this PR? https://github.com/llvm/llvm-project/pull/139348 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add tweak to override pure virtuals (PR #139348)
https://github.com/llvm-beanz approved this pull request. Sorry for the extreme delay in review. I think everyone trying to keep up with clangd reviews has been swamped and pretty over subscribed. The implementation here looks clean and follows existing conventions in clangd. It is also quite well commented. The test coverage seems solid. The one concern I have is that there could be quite a few string allocations and reallocations. Generally I prefer to see code building up strings using `SmallString` and `raw_svector_ostream` to build up the string. That said, much of the code in other clangd tweaks uses std::string pretty aggressively so I think this is probably fine and we can revisit for performance if we encounter issues. https://github.com/llvm/llvm-project/pull/139348 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add tweak to override pure virtuals (PR #139348)
marcogmaia wrote: Ping https://github.com/llvm/llvm-project/pull/139348 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add tweak to override pure virtuals (PR #139348)
marcogmaia wrote: Ping https://github.com/llvm/llvm-project/pull/139348 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add tweak to override pure virtuals (PR #139348)
https://github.com/marcogmaia updated
https://github.com/llvm/llvm-project/pull/139348
>From 76503bd8f5618b710e2909d1303de5d34723 Mon Sep 17 00:00:00 2001
From: Marco Maia
Date: Sat, 10 May 2025 00:48:39 -0300
Subject: [PATCH 01/13] [clangd] Add tweak to add pure virtual overrides
---
.../clangd/refactor/tweaks/CMakeLists.txt | 3 +-
.../refactor/tweaks/OverridePureVirtuals.cpp | 366
.../clangd/unittests/CMakeLists.txt | 1 +
.../tweaks/OverridePureVirtualsTests.cpp | 410 ++
4 files changed, 779 insertions(+), 1 deletion(-)
create mode 100644
clang-tools-extra/clangd/refactor/tweaks/OverridePureVirtuals.cpp
create mode 100644
clang-tools-extra/clangd/unittests/tweaks/OverridePureVirtualsTests.cpp
diff --git a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
index 59475b0dfd3d2..1d6e38088ad67 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
+++ b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
@@ -14,9 +14,9 @@ set(LLVM_LINK_COMPONENTS
add_clang_library(clangDaemonTweaks OBJECT
AddUsing.cpp
AnnotateHighlightings.cpp
- DumpAST.cpp
DefineInline.cpp
DefineOutline.cpp
+ DumpAST.cpp
ExpandDeducedType.cpp
ExpandMacro.cpp
ExtractFunction.cpp
@@ -24,6 +24,7 @@ add_clang_library(clangDaemonTweaks OBJECT
MemberwiseConstructor.cpp
ObjCLocalizeStringLiteral.cpp
ObjCMemberwiseInitializer.cpp
+ OverridePureVirtuals.cpp
PopulateSwitch.cpp
RawStringLiteral.cpp
RemoveUsingNamespace.cpp
diff --git a/clang-tools-extra/clangd/refactor/tweaks/OverridePureVirtuals.cpp
b/clang-tools-extra/clangd/refactor/tweaks/OverridePureVirtuals.cpp
new file mode 100644
index 0..b8880433fdd52
--- /dev/null
+++ b/clang-tools-extra/clangd/refactor/tweaks/OverridePureVirtuals.cpp
@@ -0,0 +1,366 @@
+//===--- AddPureVirtualOverride.cpp --*-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "refactor/Tweak.h"
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/Type.h"
+#include "clang/AST/TypeLoc.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/Support/FormatVariadic.h"
+#include
+#include
+#include
+#include
+#include
+#include
+
+namespace clang {
+namespace clangd {
+namespace {
+
+class OverridePureVirtuals : public Tweak {
+public:
+ const char *id() const final; // defined by REGISTER_TWEAK.
+ bool prepare(const Selection &Sel) override;
+ Expected apply(const Selection &Sel) override;
+ std::string title() const override { return "Override pure virtual methods";
}
+ llvm::StringLiteral kind() const override {
+return CodeAction::REFACTOR_KIND;
+ }
+
+private:
+ // Stores the CXXRecordDecl of the class being modified.
+ const CXXRecordDecl *CurrentDecl = nullptr;
+ // Stores pure virtual methods that need overriding, grouped by their
original
+ // access specifier.
+ std::map>
+ MissingMethodsByAccess;
+ // Stores the source locations of existing access specifiers in CurrentDecl.
+ std::map AccessSpecifierLocations;
+
+ // Helper function to gather information before applying the tweak.
+ void collectMissingPureVirtuals(const Selection &Sel);
+};
+
+REGISTER_TWEAK(OverridePureVirtuals)
+
+// Collects all unique, canonical pure virtual methods from a class and its
+// entire inheritance hierarchy. This function aims to find methods that
*could*
+// make a derived class abstract if not implemented.
+std::vector
+getAllUniquePureVirtualsFromHierarchy(const CXXRecordDecl *Decl) {
+ std::vector Result;
+ llvm::DenseSet VisitedCanonicalMethods;
+ // We declare it as a std::function because we are going to call it
+ // recursively.
+ std::function Collect;
+
+ Collect = [&](const CXXRecordDecl *CurrentClass) {
+if (!CurrentClass) {
+ return;
+}
+const CXXRecordDecl *Def = CurrentClass->getDefinition();
+if (!Def) {
+ return;
+}
+
+for (const CXXMethodDecl *M : Def->methods()) {
+ // Add if its canonical declaration hasn't been processed yet.
+ // This ensures each distinct pure virtual method signature is collected
+ // once.
+ if (M->isPureVirtual() &&
+ VisitedCanonicalMethods.insert(M->getCanonicalDecl()).second) {
+Result.emplace_back(M); // Store the specific declaration encountered.
+ }
+}
+
+for (const auto &BaseSpec : Def->bases()) {
+ if (const CXXRecordDecl *BaseDef =
+ BaseSpec.getType()->getAsCXXRecordDecl()) {
+Collect(BaseDe
[clang-tools-extra] [clangd] Add tweak to override pure virtuals (PR #139348)
marcogmaia wrote: Ping https://github.com/llvm/llvm-project/pull/139348 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add tweak to override pure virtuals (PR #139348)
marcogmaia wrote: Ping https://github.com/llvm/llvm-project/pull/139348 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add tweak to override pure virtuals (PR #139348)
https://github.com/marcogmaia updated
https://github.com/llvm/llvm-project/pull/139348
>From 76503bd8f5618b710e2909d1303de5d34723 Mon Sep 17 00:00:00 2001
From: Marco Maia
Date: Sat, 10 May 2025 00:48:39 -0300
Subject: [PATCH 01/13] [clangd] Add tweak to add pure virtual overrides
---
.../clangd/refactor/tweaks/CMakeLists.txt | 3 +-
.../refactor/tweaks/OverridePureVirtuals.cpp | 366
.../clangd/unittests/CMakeLists.txt | 1 +
.../tweaks/OverridePureVirtualsTests.cpp | 410 ++
4 files changed, 779 insertions(+), 1 deletion(-)
create mode 100644
clang-tools-extra/clangd/refactor/tweaks/OverridePureVirtuals.cpp
create mode 100644
clang-tools-extra/clangd/unittests/tweaks/OverridePureVirtualsTests.cpp
diff --git a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
index 59475b0dfd3d2..1d6e38088ad67 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
+++ b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
@@ -14,9 +14,9 @@ set(LLVM_LINK_COMPONENTS
add_clang_library(clangDaemonTweaks OBJECT
AddUsing.cpp
AnnotateHighlightings.cpp
- DumpAST.cpp
DefineInline.cpp
DefineOutline.cpp
+ DumpAST.cpp
ExpandDeducedType.cpp
ExpandMacro.cpp
ExtractFunction.cpp
@@ -24,6 +24,7 @@ add_clang_library(clangDaemonTweaks OBJECT
MemberwiseConstructor.cpp
ObjCLocalizeStringLiteral.cpp
ObjCMemberwiseInitializer.cpp
+ OverridePureVirtuals.cpp
PopulateSwitch.cpp
RawStringLiteral.cpp
RemoveUsingNamespace.cpp
diff --git a/clang-tools-extra/clangd/refactor/tweaks/OverridePureVirtuals.cpp
b/clang-tools-extra/clangd/refactor/tweaks/OverridePureVirtuals.cpp
new file mode 100644
index 0..b8880433fdd52
--- /dev/null
+++ b/clang-tools-extra/clangd/refactor/tweaks/OverridePureVirtuals.cpp
@@ -0,0 +1,366 @@
+//===--- AddPureVirtualOverride.cpp --*-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "refactor/Tweak.h"
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/Type.h"
+#include "clang/AST/TypeLoc.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/Support/FormatVariadic.h"
+#include
+#include
+#include
+#include
+#include
+#include
+
+namespace clang {
+namespace clangd {
+namespace {
+
+class OverridePureVirtuals : public Tweak {
+public:
+ const char *id() const final; // defined by REGISTER_TWEAK.
+ bool prepare(const Selection &Sel) override;
+ Expected apply(const Selection &Sel) override;
+ std::string title() const override { return "Override pure virtual methods";
}
+ llvm::StringLiteral kind() const override {
+return CodeAction::REFACTOR_KIND;
+ }
+
+private:
+ // Stores the CXXRecordDecl of the class being modified.
+ const CXXRecordDecl *CurrentDecl = nullptr;
+ // Stores pure virtual methods that need overriding, grouped by their
original
+ // access specifier.
+ std::map>
+ MissingMethodsByAccess;
+ // Stores the source locations of existing access specifiers in CurrentDecl.
+ std::map AccessSpecifierLocations;
+
+ // Helper function to gather information before applying the tweak.
+ void collectMissingPureVirtuals(const Selection &Sel);
+};
+
+REGISTER_TWEAK(OverridePureVirtuals)
+
+// Collects all unique, canonical pure virtual methods from a class and its
+// entire inheritance hierarchy. This function aims to find methods that
*could*
+// make a derived class abstract if not implemented.
+std::vector
+getAllUniquePureVirtualsFromHierarchy(const CXXRecordDecl *Decl) {
+ std::vector Result;
+ llvm::DenseSet VisitedCanonicalMethods;
+ // We declare it as a std::function because we are going to call it
+ // recursively.
+ std::function Collect;
+
+ Collect = [&](const CXXRecordDecl *CurrentClass) {
+if (!CurrentClass) {
+ return;
+}
+const CXXRecordDecl *Def = CurrentClass->getDefinition();
+if (!Def) {
+ return;
+}
+
+for (const CXXMethodDecl *M : Def->methods()) {
+ // Add if its canonical declaration hasn't been processed yet.
+ // This ensures each distinct pure virtual method signature is collected
+ // once.
+ if (M->isPureVirtual() &&
+ VisitedCanonicalMethods.insert(M->getCanonicalDecl()).second) {
+Result.emplace_back(M); // Store the specific declaration encountered.
+ }
+}
+
+for (const auto &BaseSpec : Def->bases()) {
+ if (const CXXRecordDecl *BaseDef =
+ BaseSpec.getType()->getAsCXXRecordDecl()) {
+Collect(BaseDe
[clang-tools-extra] [clangd] Add tweak to override pure virtuals (PR #139348)
https://github.com/marcogmaia edited https://github.com/llvm/llvm-project/pull/139348 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add tweak to override pure virtuals (PR #139348)
marcogmaia wrote: @kadircet I've added a description about the tweak at the beginning of the file as requested. https://github.com/llvm/llvm-project/pull/139348 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add tweak to override pure virtuals (PR #139348)
marcogmaia wrote: Ping https://github.com/llvm/llvm-project/pull/139348 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add tweak to override pure virtuals (PR #139348)
https://github.com/marcogmaia edited https://github.com/llvm/llvm-project/pull/139348 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add tweak to override pure virtuals (PR #139348)
HighCommander4 wrote: > If you're looking for a clangd reviewer, perhaps one of our other clangd > maintainers, @llvm-beanz or @kadircet, could help with this one. (I realized you are probably not able to edit the Reviewers field, so I went ahead and added the mentioned folks.) https://github.com/llvm/llvm-project/pull/139348 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add tweak to override pure virtuals (PR #139348)
https://github.com/marcogmaia updated
https://github.com/llvm/llvm-project/pull/139348
>From 76503bd8f5618b710e2909d1303de5d34723 Mon Sep 17 00:00:00 2001
From: Marco Maia
Date: Sat, 10 May 2025 00:48:39 -0300
Subject: [PATCH 01/12] [clangd] Add tweak to add pure virtual overrides
---
.../clangd/refactor/tweaks/CMakeLists.txt | 3 +-
.../refactor/tweaks/OverridePureVirtuals.cpp | 366
.../clangd/unittests/CMakeLists.txt | 1 +
.../tweaks/OverridePureVirtualsTests.cpp | 410 ++
4 files changed, 779 insertions(+), 1 deletion(-)
create mode 100644
clang-tools-extra/clangd/refactor/tweaks/OverridePureVirtuals.cpp
create mode 100644
clang-tools-extra/clangd/unittests/tweaks/OverridePureVirtualsTests.cpp
diff --git a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
index 59475b0dfd3d2..1d6e38088ad67 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
+++ b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
@@ -14,9 +14,9 @@ set(LLVM_LINK_COMPONENTS
add_clang_library(clangDaemonTweaks OBJECT
AddUsing.cpp
AnnotateHighlightings.cpp
- DumpAST.cpp
DefineInline.cpp
DefineOutline.cpp
+ DumpAST.cpp
ExpandDeducedType.cpp
ExpandMacro.cpp
ExtractFunction.cpp
@@ -24,6 +24,7 @@ add_clang_library(clangDaemonTweaks OBJECT
MemberwiseConstructor.cpp
ObjCLocalizeStringLiteral.cpp
ObjCMemberwiseInitializer.cpp
+ OverridePureVirtuals.cpp
PopulateSwitch.cpp
RawStringLiteral.cpp
RemoveUsingNamespace.cpp
diff --git a/clang-tools-extra/clangd/refactor/tweaks/OverridePureVirtuals.cpp
b/clang-tools-extra/clangd/refactor/tweaks/OverridePureVirtuals.cpp
new file mode 100644
index 0..b8880433fdd52
--- /dev/null
+++ b/clang-tools-extra/clangd/refactor/tweaks/OverridePureVirtuals.cpp
@@ -0,0 +1,366 @@
+//===--- AddPureVirtualOverride.cpp --*-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "refactor/Tweak.h"
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/Type.h"
+#include "clang/AST/TypeLoc.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/Support/FormatVariadic.h"
+#include
+#include
+#include
+#include
+#include
+#include
+
+namespace clang {
+namespace clangd {
+namespace {
+
+class OverridePureVirtuals : public Tweak {
+public:
+ const char *id() const final; // defined by REGISTER_TWEAK.
+ bool prepare(const Selection &Sel) override;
+ Expected apply(const Selection &Sel) override;
+ std::string title() const override { return "Override pure virtual methods";
}
+ llvm::StringLiteral kind() const override {
+return CodeAction::REFACTOR_KIND;
+ }
+
+private:
+ // Stores the CXXRecordDecl of the class being modified.
+ const CXXRecordDecl *CurrentDecl = nullptr;
+ // Stores pure virtual methods that need overriding, grouped by their
original
+ // access specifier.
+ std::map>
+ MissingMethodsByAccess;
+ // Stores the source locations of existing access specifiers in CurrentDecl.
+ std::map AccessSpecifierLocations;
+
+ // Helper function to gather information before applying the tweak.
+ void collectMissingPureVirtuals(const Selection &Sel);
+};
+
+REGISTER_TWEAK(OverridePureVirtuals)
+
+// Collects all unique, canonical pure virtual methods from a class and its
+// entire inheritance hierarchy. This function aims to find methods that
*could*
+// make a derived class abstract if not implemented.
+std::vector
+getAllUniquePureVirtualsFromHierarchy(const CXXRecordDecl *Decl) {
+ std::vector Result;
+ llvm::DenseSet VisitedCanonicalMethods;
+ // We declare it as a std::function because we are going to call it
+ // recursively.
+ std::function Collect;
+
+ Collect = [&](const CXXRecordDecl *CurrentClass) {
+if (!CurrentClass) {
+ return;
+}
+const CXXRecordDecl *Def = CurrentClass->getDefinition();
+if (!Def) {
+ return;
+}
+
+for (const CXXMethodDecl *M : Def->methods()) {
+ // Add if its canonical declaration hasn't been processed yet.
+ // This ensures each distinct pure virtual method signature is collected
+ // once.
+ if (M->isPureVirtual() &&
+ VisitedCanonicalMethods.insert(M->getCanonicalDecl()).second) {
+Result.emplace_back(M); // Store the specific declaration encountered.
+ }
+}
+
+for (const auto &BaseSpec : Def->bases()) {
+ if (const CXXRecordDecl *BaseDef =
+ BaseSpec.getType()->getAsCXXRecordDecl()) {
+Collect(BaseDe
[clang-tools-extra] [clangd] Add tweak to override pure virtuals (PR #139348)
https://github.com/marcogmaia edited https://github.com/llvm/llvm-project/pull/139348 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
