[clang-tidy] Disable bugprone-multi-level-pointer-conversion in C code #141209
Conversation
It is deemed more of an annoyance than a help, since the patterns the check warns about are quite common and idiomatic in C, and there are no good alternatives. Thus, enable the check only for C++, where there are safer constructs. Fixes llvm#140659
|
@llvm/pr-subscribers-clang-tools-extra Author: Carlos Galvez (carlosgalvezp) ChangesIt is deemed more of an annoyance than a help, since the patterns the check warns about are quite common and idiomatic in C, and there are no good alternatives. Thus, enable the check only for C++, where there are safer constructs. Fixes #140659 Full diff: https://github.com/llvm/llvm-project/pull/141209.diff 3 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/bugprone/MultiLevelImplicitPointerConversionCheck.h b/clang-tools-extra/clang-tidy/bugprone/MultiLevelImplicitPointerConversionCheck.h
index 13228145ff35f..c502f31d867d2 100644
--- a/clang-tools-extra/clang-tidy/bugprone/MultiLevelImplicitPointerConversionCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/MultiLevelImplicitPointerConversionCheck.h
@@ -26,6 +26,9 @@ class MultiLevelImplicitPointerConversionCheck : public ClangTidyCheck {
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
std::optional<TraversalKind> getCheckTraversalKind() const override;
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus;
+ }
};
} // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 9b29ab12e1bfa..7e76d81eed439 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -148,6 +148,10 @@ New check aliases
Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^
+- Disabled :doc:`bugprone-multi-level-implicit-pointer-conversion
+ <clang-tidy/checks/bugprone/multi-level-implicit-pointer-conversion>` check in
+ C code. The check is now only active in C++ code.
+
- Improved :doc:`bugprone-optional-value-conversion
<clang-tidy/checks/bugprone/optional-value-conversion>` check to detect
conversion in argument of ``std::make_optional``.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/multi-level-implicit-pointer-conversion.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/multi-level-implicit-pointer-conversion.rst
index e6dc5c13aa025..a226654c9ab75 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/multi-level-implicit-pointer-conversion.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/multi-level-implicit-pointer-conversion.rst
@@ -42,3 +42,12 @@ Additionally, it is recommended that developers thoroughly check and verify the
safety of the conversion before using an explicit cast. This extra level of
caution can help catch potential issues early on in the development process,
improving the overall reliability and maintainability of the code.
+
+.. note::
+
+ This check is enabled only for C++ code, not for C code. The motivation is the
+ type of conversions that this check warns about are fairly common and
+ idiomatic in C, and there are no safer alternatives. This introduces quite
+ some friction in the form of suppressions or casts. The check remains active
+ in C++ to more easily detect C-style patterns that can potentially be
+ refactored to use safer C++ constructs.
|
|
@llvm/pr-subscribers-clang-tidy Author: Carlos Galvez (carlosgalvezp) ChangesIt is deemed more of an annoyance than a help, since the patterns the check warns about are quite common and idiomatic in C, and there are no good alternatives. Thus, enable the check only for C++, where there are safer constructs. Fixes #140659 Full diff: https://github.com/llvm/llvm-project/pull/141209.diff 3 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/bugprone/MultiLevelImplicitPointerConversionCheck.h b/clang-tools-extra/clang-tidy/bugprone/MultiLevelImplicitPointerConversionCheck.h
index 13228145ff35f..c502f31d867d2 100644
--- a/clang-tools-extra/clang-tidy/bugprone/MultiLevelImplicitPointerConversionCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/MultiLevelImplicitPointerConversionCheck.h
@@ -26,6 +26,9 @@ class MultiLevelImplicitPointerConversionCheck : public ClangTidyCheck {
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
std::optional<TraversalKind> getCheckTraversalKind() const override;
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus;
+ }
};
} // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 9b29ab12e1bfa..7e76d81eed439 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -148,6 +148,10 @@ New check aliases
Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^
+- Disabled :doc:`bugprone-multi-level-implicit-pointer-conversion
+ <clang-tidy/checks/bugprone/multi-level-implicit-pointer-conversion>` check in
+ C code. The check is now only active in C++ code.
+
- Improved :doc:`bugprone-optional-value-conversion
<clang-tidy/checks/bugprone/optional-value-conversion>` check to detect
conversion in argument of ``std::make_optional``.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/multi-level-implicit-pointer-conversion.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/multi-level-implicit-pointer-conversion.rst
index e6dc5c13aa025..a226654c9ab75 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/multi-level-implicit-pointer-conversion.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/multi-level-implicit-pointer-conversion.rst
@@ -42,3 +42,12 @@ Additionally, it is recommended that developers thoroughly check and verify the
safety of the conversion before using an explicit cast. This extra level of
caution can help catch potential issues early on in the development process,
improving the overall reliability and maintainability of the code.
+
+.. note::
+
+ This check is enabled only for C++ code, not for C code. The motivation is the
+ type of conversions that this check warns about are fairly common and
+ idiomatic in C, and there are no safer alternatives. This introduces quite
+ some friction in the form of suppressions or casts. The check remains active
+ in C++ to more easily detect C-style patterns that can potentially be
+ refactored to use safer C++ constructs.
|
There was a problem hiding this comment.
I'm not sure about this. This check is about IMPLICIT casts, you can always do explicit cast in C (via functional cast or c-style cast) or C++ (reinterpret_cast) to say that mutli-level pointer conversion is expected in that place.
If some C project got lot of such issues that they don't want to fix, then probably they should just disable a check.
In such case I don't see a reason to limit check to C++ only always.
It still could be done via some check option.
|
You can check the discussion in the linked issue. The argument is that these casts are idiomatic in C, and does not make sense to pollute the code with casts. There's no other safer alternative either.
There doesn't appear to be an actual true positive in C where the check is useful as per the discussion. |
|
Ok, but that is for "some projects". If you search internet you may find examples when people omit those casts and were people add those casts. This is why I don't like disabling permanently check for C, as for some projects it could be hiding potential issues. |
It is deemed more of an annoyance than a help, since the patterns the check warns about are quite common and idiomatic in C, and there are no good alternatives.
Thus, enable the check only for C++, where there are safer constructs.
Fixes #140659