[analyzer] Remove deprecated option VirtualCall:PureOnly #131823
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
VirtualCallChecker.cpp implements two related checkers: - `optin.cplusplus.VirtualCall` which reports situations when constructors or destructors call virtual methods (which is bugprone because it does not trigger virtual dispatch, but can be legitmate). - `cplusplus.PureVirtualCall` reports situations when constructors or destructors call _pure_ virtual methods, which is an error. Six years ago these two functions were both reported by the same checker (called `optin.cplusplus.VirtualCall`) and it had an option called `PureOnly` which limited its output to the pure case. When (in 2019) the two checker parts were separated by the commit d3971fe, the option `PureOnly` was preserved for the sake of compatibility, but it is no longer useful (when it is set to true, it just suppresses all reports from `optin.cplusplus.VirtualCall`) so it was marked as deprecated. I'm removing this deprecated option now because it is no longer relevant and its presence caused minor complications when I was porting `VirtualCallChecker.cpp` to the new multipart checker framework (introduced in 2709998).
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Donát Nagy (NagyDonat) ChangesVirtualCallChecker.cpp implements two related checkers:
Six years ago these two functions were both reported by the same checker (called When (in 2019) the two checker parts were separated by the commit d3971fe, the option I'm removing this deprecated option now because it is no longer relevant and its presence caused minor complications when I was porting Full diff: https://github.com/llvm/llvm-project/pull/131823.diff 4 Files Affected:
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 6632254955fe6..35df4e7003ac9 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -750,24 +750,14 @@ def UninitializedObjectChecker: Checker<"UninitializedObject">,
]>,
Documentation<HasDocumentation>;
-def VirtualCallChecker : Checker<"VirtualCall">,
- HelpText<"Check virtual function calls during construction/destruction">,
- CheckerOptions<[
- CmdLineOption<Boolean,
- "ShowFixIts",
- "Enable fix-it hints for this checker",
- "false",
- InAlpha>,
- CmdLineOption<Boolean,
- "PureOnly",
- "Disables the checker. Keeps cplusplus.PureVirtualCall "
- "enabled. This option is only provided for backwards "
- "compatibility.",
- "false",
- InAlpha>
- ]>,
- Dependencies<[VirtualCallModeling]>,
- Documentation<HasDocumentation>;
+def VirtualCallChecker
+ : Checker<"VirtualCall">,
+ HelpText<"Check virtual function calls during construction/destruction">,
+ CheckerOptions<[CmdLineOption<Boolean, "ShowFixIts",
+ "Enable fix-it hints for this checker",
+ "false", InAlpha>]>,
+ Dependencies<[VirtualCallModeling]>,
+ Documentation<HasDocumentation>;
} // end: "optin.cplusplus"
diff --git a/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
index 33a9a07f9d32d..6d9c52e966022 100644
--- a/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
@@ -214,14 +214,11 @@ void ento::registerPureVirtualCallChecker(CheckerManager &Mgr) {
void ento::registerVirtualCallChecker(CheckerManager &Mgr) {
auto *Chk = Mgr.getChecker<VirtualCallChecker>();
- if (!Mgr.getAnalyzerOptions().getCheckerBooleanOption(
- Mgr.getCurrentCheckerName(), "PureOnly")) {
- Chk->BT_Impure = std::make_unique<BugType>(
- Mgr.getCurrentCheckerName(), "Unexpected loss of virtual dispatch",
- categories::CXXObjectLifecycle);
- Chk->ShowFixIts = Mgr.getAnalyzerOptions().getCheckerBooleanOption(
- Mgr.getCurrentCheckerName(), "ShowFixIts");
- }
+ Chk->BT_Impure = std::make_unique<BugType>(
+ Mgr.getCurrentCheckerName(), "Unexpected loss of virtual dispatch",
+ categories::CXXObjectLifecycle);
+ Chk->ShowFixIts = Mgr.getAnalyzerOptions().getCheckerBooleanOption(
+ Mgr.getCurrentCheckerName(), "ShowFixIts");
}
bool ento::shouldRegisterVirtualCallModeling(const CheckerManager &mgr) {
diff --git a/clang/test/Analysis/analyzer-config.c b/clang/test/Analysis/analyzer-config.c
index 00177769f3243..80cad54b039f4 100644
--- a/clang/test/Analysis/analyzer-config.c
+++ b/clang/test/Analysis/analyzer-config.c
@@ -108,7 +108,6 @@
// CHECK-NEXT: optin.cplusplus.UninitializedObject:IgnoreRecordsWithField = ""
// CHECK-NEXT: optin.cplusplus.UninitializedObject:NotesAsWarnings = false
// CHECK-NEXT: optin.cplusplus.UninitializedObject:Pedantic = false
-// CHECK-NEXT: optin.cplusplus.VirtualCall:PureOnly = false
// CHECK-NEXT: optin.cplusplus.VirtualCall:ShowFixIts = false
// CHECK-NEXT: optin.osx.cocoa.localizability.NonLocalizedStringChecker:AggressiveReport = false
// CHECK-NEXT: optin.performance.Padding:AllowedPad = 24
diff --git a/clang/test/Analysis/virtualcall.cpp b/clang/test/Analysis/virtualcall.cpp
index 80f89d14ea97c..82285b6d12844 100644
--- a/clang/test/Analysis/virtualcall.cpp
+++ b/clang/test/Analysis/virtualcall.cpp
@@ -6,29 +6,11 @@
// RUN: -analyzer-checker=debug.ExprInspection \
// RUN: -std=c++11 -verify=pure -std=c++11 %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus.VirtualCall \
-// RUN: -analyzer-config \
-// RUN: optin.cplusplus.VirtualCall:PureOnly=true \
-// RUN: -analyzer-checker=debug.ExprInspection \
-// RUN: -std=c++11 -verify=none %s
-
// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.PureVirtualCall \
// RUN: -analyzer-checker=optin.cplusplus.VirtualCall \
// RUN: -analyzer-checker=debug.ExprInspection \
// RUN: -std=c++11 -verify=pure,impure -std=c++11 %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.PureVirtualCall \
-// RUN: -analyzer-checker=optin.cplusplus.VirtualCall \
-// RUN: -analyzer-config \
-// RUN: optin.cplusplus.VirtualCall:PureOnly=true \
-// RUN: -analyzer-checker=debug.ExprInspection \
-// RUN: -std=c++11 -verify=pure %s
-
-
-// We expect no diagnostics when all checks are disabled.
-// none-no-diagnostics
-
-
#include "virtualcall.h"
void clang_analyzer_warnIfReached();
|
|
@llvm/pr-subscribers-clang-static-analyzer-1 Author: Donát Nagy (NagyDonat) ChangesVirtualCallChecker.cpp implements two related checkers:
Six years ago these two functions were both reported by the same checker (called When (in 2019) the two checker parts were separated by the commit d3971fe, the option I'm removing this deprecated option now because it is no longer relevant and its presence caused minor complications when I was porting Full diff: https://github.com/llvm/llvm-project/pull/131823.diff 4 Files Affected:
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 6632254955fe6..35df4e7003ac9 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -750,24 +750,14 @@ def UninitializedObjectChecker: Checker<"UninitializedObject">,
]>,
Documentation<HasDocumentation>;
-def VirtualCallChecker : Checker<"VirtualCall">,
- HelpText<"Check virtual function calls during construction/destruction">,
- CheckerOptions<[
- CmdLineOption<Boolean,
- "ShowFixIts",
- "Enable fix-it hints for this checker",
- "false",
- InAlpha>,
- CmdLineOption<Boolean,
- "PureOnly",
- "Disables the checker. Keeps cplusplus.PureVirtualCall "
- "enabled. This option is only provided for backwards "
- "compatibility.",
- "false",
- InAlpha>
- ]>,
- Dependencies<[VirtualCallModeling]>,
- Documentation<HasDocumentation>;
+def VirtualCallChecker
+ : Checker<"VirtualCall">,
+ HelpText<"Check virtual function calls during construction/destruction">,
+ CheckerOptions<[CmdLineOption<Boolean, "ShowFixIts",
+ "Enable fix-it hints for this checker",
+ "false", InAlpha>]>,
+ Dependencies<[VirtualCallModeling]>,
+ Documentation<HasDocumentation>;
} // end: "optin.cplusplus"
diff --git a/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
index 33a9a07f9d32d..6d9c52e966022 100644
--- a/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
@@ -214,14 +214,11 @@ void ento::registerPureVirtualCallChecker(CheckerManager &Mgr) {
void ento::registerVirtualCallChecker(CheckerManager &Mgr) {
auto *Chk = Mgr.getChecker<VirtualCallChecker>();
- if (!Mgr.getAnalyzerOptions().getCheckerBooleanOption(
- Mgr.getCurrentCheckerName(), "PureOnly")) {
- Chk->BT_Impure = std::make_unique<BugType>(
- Mgr.getCurrentCheckerName(), "Unexpected loss of virtual dispatch",
- categories::CXXObjectLifecycle);
- Chk->ShowFixIts = Mgr.getAnalyzerOptions().getCheckerBooleanOption(
- Mgr.getCurrentCheckerName(), "ShowFixIts");
- }
+ Chk->BT_Impure = std::make_unique<BugType>(
+ Mgr.getCurrentCheckerName(), "Unexpected loss of virtual dispatch",
+ categories::CXXObjectLifecycle);
+ Chk->ShowFixIts = Mgr.getAnalyzerOptions().getCheckerBooleanOption(
+ Mgr.getCurrentCheckerName(), "ShowFixIts");
}
bool ento::shouldRegisterVirtualCallModeling(const CheckerManager &mgr) {
diff --git a/clang/test/Analysis/analyzer-config.c b/clang/test/Analysis/analyzer-config.c
index 00177769f3243..80cad54b039f4 100644
--- a/clang/test/Analysis/analyzer-config.c
+++ b/clang/test/Analysis/analyzer-config.c
@@ -108,7 +108,6 @@
// CHECK-NEXT: optin.cplusplus.UninitializedObject:IgnoreRecordsWithField = ""
// CHECK-NEXT: optin.cplusplus.UninitializedObject:NotesAsWarnings = false
// CHECK-NEXT: optin.cplusplus.UninitializedObject:Pedantic = false
-// CHECK-NEXT: optin.cplusplus.VirtualCall:PureOnly = false
// CHECK-NEXT: optin.cplusplus.VirtualCall:ShowFixIts = false
// CHECK-NEXT: optin.osx.cocoa.localizability.NonLocalizedStringChecker:AggressiveReport = false
// CHECK-NEXT: optin.performance.Padding:AllowedPad = 24
diff --git a/clang/test/Analysis/virtualcall.cpp b/clang/test/Analysis/virtualcall.cpp
index 80f89d14ea97c..82285b6d12844 100644
--- a/clang/test/Analysis/virtualcall.cpp
+++ b/clang/test/Analysis/virtualcall.cpp
@@ -6,29 +6,11 @@
// RUN: -analyzer-checker=debug.ExprInspection \
// RUN: -std=c++11 -verify=pure -std=c++11 %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus.VirtualCall \
-// RUN: -analyzer-config \
-// RUN: optin.cplusplus.VirtualCall:PureOnly=true \
-// RUN: -analyzer-checker=debug.ExprInspection \
-// RUN: -std=c++11 -verify=none %s
-
// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.PureVirtualCall \
// RUN: -analyzer-checker=optin.cplusplus.VirtualCall \
// RUN: -analyzer-checker=debug.ExprInspection \
// RUN: -std=c++11 -verify=pure,impure -std=c++11 %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.PureVirtualCall \
-// RUN: -analyzer-checker=optin.cplusplus.VirtualCall \
-// RUN: -analyzer-config \
-// RUN: optin.cplusplus.VirtualCall:PureOnly=true \
-// RUN: -analyzer-checker=debug.ExprInspection \
-// RUN: -std=c++11 -verify=pure %s
-
-
-// We expect no diagnostics when all checks are disabled.
-// none-no-diagnostics
-
-
#include "virtualcall.h"
void clang_analyzer_warnIfReached();
|
Xazax-hun
approved these changes
Mar 18, 2025
|
I added a note to the Release Notes. Is "Improvements" the right subsection for this? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
VirtualCallChecker.cpp implements two related checkers:
optin.cplusplus.VirtualCallwhich reports situations when constructors or destructors call virtual methods (which is bugprone because it does not trigger virtual dispatch, but can be legitmate).cplusplus.PureVirtualCallreports situations when constructors or destructors call pure virtual methods, which is an error.Six years ago these two bug types were both reported by the same checker (called
optin.cplusplus.VirtualCall) and it had an option calledPureOnlywhich limited its output to the pure case.When (in 2019) the two checker parts were separated by the commit d3971fe, the option
PureOnlywas preserved for the sake of compatibility, but it is no longer useful (when it is set to true, it just suppresses all reports fromoptin.cplusplus.VirtualCall) so it was marked as deprecated.I'm removing this deprecated option now because it is no longer relevant and its presence caused minor complications when I was porting
VirtualCallChecker.cppto the new multipart checker framework (introduced in 2709998).