[clang-tools-extra] r284888 - Remove 'misc-pointer-and-integral-operation' clang-tidy check. The only cases
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 24 10:06:26 PDT 2016
On Mon, Oct 24, 2016 at 8:31 AM, Alexander Kornienko <alexfh at google.com>
wrote:
> Wait, I thought the check was mostly useful for C code, where it detects
> real issues? Etienne, can you comment?
>
See below. This is only checking for cases that are already ill-formed,
some of which we accept as an extension, and all of which have frontend
warnings.
We should consider enabling those of the below warnings that are disabled
by default, and should definitely give them warning flags, but I don't
think we need clang-tidy checks for ill-formed code that the frontend
already diagnoses. (Note that prior to DR1512, some of the cases detected
by this check were valid in C++).
> On Fri, Oct 21, 2016 at 11:50 PM, Richard Smith via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>> Author: rsmith
>> Date: Fri Oct 21 16:50:28 2016
>> New Revision: 284888
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=284888&view=rev
>> Log:
>> Remove 'misc-pointer-and-integral-operation' clang-tidy check. The only
>> cases
>> it detects are ill-formed (some per C++ core issue 1512, others always
>> have
>> been).
>>
>> Removed:
>> clang-tools-extra/trunk/clang-tidy/misc/PointerAndIntegralOp
>> erationCheck.cpp
>> clang-tools-extra/trunk/clang-tidy/misc/PointerAndIntegralOp
>> erationCheck.h
>> clang-tools-extra/trunk/docs/clang-tidy/checks/misc-pointer-
>> and-integral-operation.rst
>> clang-tools-extra/trunk/test/clang-tidy/misc-pointer-and-int
>> egral-operation-cxx98.cpp
>> clang-tools-extra/trunk/test/clang-tidy/misc-pointer-and-int
>> egral-operation.cpp
>> Modified:
>> clang-tools-extra/trunk/clang-tidy-vs/ClangTidy/Resources/Cl
>> angTidyChecks.yaml
>> clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
>> clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
>> clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
>>
>> Modified: clang-tools-extra/trunk/clang-tidy-vs/ClangTidy/Resources/Cl
>> angTidyChecks.yaml
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/
>> clang-tidy-vs/ClangTidy/Resources/ClangTidyChecks.yaml?rev=
>> 284888&r1=284887&r2=284888&view=diff
>> ============================================================
>> ==================
>> --- clang-tools-extra/trunk/clang-tidy-vs/ClangTidy/Resources/ClangTidyChecks.yaml
>> (original)
>> +++ clang-tools-extra/trunk/clang-tidy-vs/ClangTidy/Resources/ClangTidyChecks.yaml
>> Fri Oct 21 16:50:28 2016
>> @@ -239,10 +239,6 @@ Checks:
>> Description:
>> Name: misc-non-copyable-objects
>> - Category: Miscellaneous
>> - Label: Suspicious pointer / integer operations
>> - Description:
>> - Name: misc-pointer-and-integral-operation
>> - - Category: Miscellaneous
>> Label: Find redundant expressions
>> Description:
>> Name: misc-redundant-expression
>>
>> Modified: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/
>> clang-tidy/misc/CMakeLists.txt?rev=284888&r1=284887&r2=284888&view=diff
>> ============================================================
>> ==================
>> --- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt (original)
>> +++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt Fri Oct 21
>> 16:50:28 2016
>> @@ -24,7 +24,6 @@ add_clang_library(clangTidyMiscModule
>> NewDeleteOverloadsCheck.cpp
>> NoexceptMoveConstructorCheck.cpp
>> NonCopyableObjects.cpp
>> - PointerAndIntegralOperationCheck.cpp
>> RedundantExpressionCheck.cpp
>> SizeofContainerCheck.cpp
>> SizeofExpressionCheck.cpp
>>
>> Modified: clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/
>> clang-tidy/misc/MiscTidyModule.cpp?rev=284888&r1=284887&r2=
>> 284888&view=diff
>> ============================================================
>> ==================
>> --- clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp (original)
>> +++ clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp Fri Oct
>> 21 16:50:28 2016
>> @@ -32,7 +32,6 @@
>> #include "NewDeleteOverloadsCheck.h"
>> #include "NoexceptMoveConstructorCheck.h"
>> #include "NonCopyableObjects.h"
>> -#include "PointerAndIntegralOperationCheck.h"
>> #include "RedundantExpressionCheck.h"
>> #include "SizeofContainerCheck.h"
>> #include "SizeofExpressionCheck.h"
>> @@ -104,8 +103,6 @@ public:
>> "misc-noexcept-move-constructor");
>> CheckFactories.registerCheck<NonCopyableObjectsCheck>(
>> "misc-non-copyable-objects");
>> - CheckFactories.registerCheck<PointerAndIntegralOperationCheck>(
>> - "misc-pointer-and-integral-operation");
>> CheckFactories.registerCheck<RedundantExpressionCheck>(
>> "misc-redundant-expression");
>> CheckFactories.registerCheck<SizeofContainerCheck>("misc-si
>> zeof-container");
>>
>> Removed: clang-tools-extra/trunk/clang-tidy/misc/PointerAndIntegralOp
>> erationCheck.cpp
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/
>> clang-tidy/misc/PointerAndIntegralOperationCheck.cpp?rev=284887&view=auto
>> ============================================================
>> ==================
>> --- clang-tools-extra/trunk/clang-tidy/misc/PointerAndIntegralOperationCheck.cpp
>> (original)
>> +++ clang-tools-extra/trunk/clang-tidy/misc/PointerAndIntegralOperationCheck.cpp
>> (removed)
>> @@ -1,104 +0,0 @@
>> -//===--- PointerAndIntegralOperationCheck.cpp -
>> clang-tidy-----------------===//
>> -//
>> -// The LLVM Compiler Infrastructure
>> -//
>> -// This file is distributed under the University of Illinois Open Source
>> -// License. See LICENSE.TXT for details.
>> -//
>> -//===------------------------------------------------------
>> ----------------===//
>> -
>> -#include "PointerAndIntegralOperationCheck.h"
>> -#include "clang/AST/ASTContext.h"
>> -#include "clang/ASTMatchers/ASTMatchFinder.h"
>> -#include "../utils/Matchers.h"
>> -
>> -using namespace clang::ast_matchers;
>> -
>> -namespace clang {
>> -namespace tidy {
>> -namespace misc {
>> -
>> -void PointerAndIntegralOperationCheck::registerMatchers(MatchFinder
>> *Finder) {
>> - const auto PointerExpr = expr(hasType(pointerType()));
>> - const auto BoolExpr = ignoringParenImpCasts(hasType(booleanType()));
>> - const auto CharExpr = ignoringParenImpCasts(hasType(
>> isAnyCharacter()));
>> -
>> - const auto BinOpWithPointerExpr =
>> - binaryOperator(unless(anyOf(hasOperatorName(","),
>> hasOperatorName("="))),
>> - hasEitherOperand(PointerExpr));
>> -
>> - const auto AssignToPointerExpr =
>> - binaryOperator(hasOperatorName("="), hasLHS(PointerExpr));
>> -
>> - const auto CompareToPointerExpr =
>> - binaryOperator(matchers::isRelationalOperator(),
>> - hasEitherOperand(PointerExpr));
>> -
>> - // Detect expression like: ptr = (x != y);
>> - Finder->addMatcher(binaryOperator(AssignToPointerExpr,
>> hasRHS(BoolExpr))
>> - .bind("assign-bool-to-pointer"),
>> - this);
>>
>
This is ill-formed in C and C++. Clang accepts in C as an extension (with
enabled-by-default warning), rejects with -pedantic-errors or
-Werror=int-conversion.
- // Detect expression like: ptr = A[i]; where A is char*.
>> - Finder->addMatcher(binaryOperator(AssignToPointerExpr,
>> hasRHS(CharExpr))
>> - .bind("assign-char-to-pointer"),
>> - this);
>
>
This is ill-formed in C and C++. Clang accepts in C as an extension (with
enabled-by-default warning), rejects with -pedantic-errors or
-Werror=int-conversion.
- // Detect expression like: ptr < false;
>> - Finder->addMatcher(
>> - binaryOperator(BinOpWithPointerExpr,
>> - hasEitherOperand(ignoringPare
>> nImpCasts(cxxBoolLiteral())))
>> - .bind("pointer-and-bool-literal"),
>> - this);
>>
>
This is ill-formed in C (assuming 'false' is the macro from <stdbool.h> and
C++. Clang accepts in C as an extension (with disabled-by-default warning),
rejects with -pedantic-errors. Clang used to accept in C++98 (prior to
applying fix for DR1512), now rejects in all C++ language modes.
> - // Detect expression like: ptr < 'a';
>> - Finder->addMatcher(binaryOperator(BinOpWithPointerExpr,
>> - hasEitherOperand(ignoringParen
>> ImpCasts(
>> - characterLiteral())))
>> - .bind("pointer-and-char-literal"),
>> - this);
>>
>
This is ill-formed in C and C++. Clang accepts in C as an extension (with
enabled-by-default warning), rejects with -pedantic-errors.
> - // Detect expression like: ptr < 0;
>> - Finder->addMatcher(binaryOperator(CompareToPointerExpr,
>> - hasEitherOperand(ignoringParen
>> ImpCasts(
>> - integerLiteral(equals(0)))))
>> - .bind("compare-pointer-to-zero"),
>> - this);
>>
>
This is ill-formed in C and C++. Clang accepts in C as an extension (with
disabled-by-default warning), rejects with -pedantic-errors. Clang used to
accept in all C++ language modes (prior to applying fix for DR1512), now
rejects in all C++ language modes.
> - // Detect expression like: ptr < nullptr;
>> - Finder->addMatcher(binaryOperator(CompareToPointerExpr,
>> - hasEitherOperand(ignoringParen
>> ImpCasts(
>> - cxxNullPtrLiteralExpr())))
>> - .bind("compare-pointer-to-null"),
>> - this);
>>
>
This is ill-formed (in C++; C doesn't have nullptr). Clang used to accept
in all C++ language modes (prior to applying fix for DR1512), now rejects
in all C++ language modes.
> -}
>> -
>> -void PointerAndIntegralOperationCheck::check(
>> - const MatchFinder::MatchResult &Result) {
>> - if (const auto *E =
>> - Result.Nodes.getNodeAs<BinaryOperator>("assign-bool-to-pointer"))
>> {
>> - diag(E->getOperatorLoc(), "suspicious assignment from bool to
>> pointer");
>> - } else if (const auto *E = Result.Nodes.getNodeAs<BinaryOperator>(
>> - "assign-char-to-pointer")) {
>> - diag(E->getOperatorLoc(), "suspicious assignment from char to
>> pointer");
>> - } else if (const auto *E = Result.Nodes.getNodeAs<BinaryOperator>(
>> - "pointer-and-bool-literal")) {
>> - diag(E->getOperatorLoc(),
>> - "suspicious operation between pointer and bool literal");
>> - } else if (const auto *E = Result.Nodes.getNodeAs<BinaryOperator>(
>> - "pointer-and-char-literal")) {
>> - diag(E->getOperatorLoc(),
>> - "suspicious operation between pointer and character literal");
>> - } else if (const auto *E = Result.Nodes.getNodeAs<BinaryOperator>(
>> - "compare-pointer-to-zero")) {
>> - diag(E->getOperatorLoc(), "suspicious comparison of pointer with
>> zero");
>> - } else if (const auto *E = Result.Nodes.getNodeAs<BinaryOperator>(
>> - "compare-pointer-to-null")) {
>> - diag(E->getOperatorLoc(),
>> - "suspicious comparison of pointer with null expression");
>> - }
>> -}
>> -
>> -} // namespace misc
>> -} // namespace tidy
>> -} // namespace clang
>>
>> Removed: clang-tools-extra/trunk/clang-tidy/misc/PointerAndIntegralOp
>> erationCheck.h
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/
>> clang-tidy/misc/PointerAndIntegralOperationCheck.h?rev=284887&view=auto
>> ============================================================
>> ==================
>> --- clang-tools-extra/trunk/clang-tidy/misc/PointerAndIntegralOperationCheck.h
>> (original)
>> +++ clang-tools-extra/trunk/clang-tidy/misc/PointerAndIntegralOperationCheck.h
>> (removed)
>> @@ -1,35 +0,0 @@
>> -//===--- PointerAndIntegralOperationCheck.h - clang-tidy---------*- C++
>> -*-===//
>> -//
>> -// The LLVM Compiler Infrastructure
>> -//
>> -// This file is distributed under the University of Illinois Open Source
>> -// License. See LICENSE.TXT for details.
>> -//
>> -//===------------------------------------------------------
>> ----------------===//
>> -
>> -#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_POINTER_AND_INTEGRAL_
>> OPERATION_H
>> -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_POINTER_AND_INTEGRAL_
>> OPERATION_H
>> -
>> -#include "../ClangTidy.h"
>> -
>> -namespace clang {
>> -namespace tidy {
>> -namespace misc {
>> -
>> -/// Find suspicious expressions involving pointer and integral types.
>> -///
>> -/// For the user-facing documentation see:
>> -/// http://clang.llvm.org/extra/clang-tidy/checks/misc-pointer-
>> and-integral-operation.html
>> -class PointerAndIntegralOperationCheck : public ClangTidyCheck {
>> -public:
>> - PointerAndIntegralOperationCheck(StringRef Name, ClangTidyContext
>> *Context)
>> - : ClangTidyCheck(Name, Context) {}
>> - void registerMatchers(ast_matchers::MatchFinder *Finder) override;
>> - void check(const ast_matchers::MatchFinder::MatchResult &Result)
>> override;
>> -};
>> -
>> -} // namespace misc
>> -} // namespace tidy
>> -} // namespace clang
>> -
>> -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_POINTER_AND_INTEGRAL_
>> OPERATION_H
>>
>> Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/
>> docs/clang-tidy/checks/list.rst?rev=284888&r1=284887&r2=284888&view=diff
>> ============================================================
>> ==================
>> --- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
>> +++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Fri Oct 21
>> 16:50:28 2016
>> @@ -74,7 +74,6 @@ Clang-Tidy Checks
>> misc-new-delete-overloads
>> misc-noexcept-move-constructor
>> misc-non-copyable-objects
>> - misc-pointer-and-integral-operation
>> misc-redundant-expression
>> misc-sizeof-container
>> misc-sizeof-expression
>>
>> Removed: clang-tools-extra/trunk/docs/clang-tidy/checks/misc-pointer-
>> and-integral-operation.rst
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/
>> docs/clang-tidy/checks/misc-pointer-and-integral-operation
>> .rst?rev=284887&view=auto
>> ============================================================
>> ==================
>> --- clang-tools-extra/trunk/docs/clang-tidy/checks/misc-pointer-and-integral-operation.rst
>> (original)
>> +++ clang-tools-extra/trunk/docs/clang-tidy/checks/misc-pointer-and-integral-operation.rst
>> (removed)
>> @@ -1,24 +0,0 @@
>> -.. title:: clang-tidy - misc-pointer-and-integral-operation
>> -
>> -misc-pointer-and-integral-operation
>> -===================================
>> -
>> -Looks for operation involving pointers and integral types. A common
>> mistake is
>> -to forget to dereference a pointer. These errors may be detected when a
>> pointer
>> -object is compare to an object with integral type.
>> -
>> -Examples:
>> -
>> -.. code-block:: c++
>> -
>> - char* ptr;
>> - if ((ptr = malloc(...)) < nullptr) // Pointer comparison with
>> operator '<'
>> - ... // Should probably be '!='
>> -
>> - if (ptr == '\0') // Should probably be *ptr
>> - ...
>> -
>> - void Process(std::string path, bool* error) {
>> - [...]
>> - if (error != false) // Should probably be *error
>> - ...
>>
>> Removed: clang-tools-extra/trunk/test/clang-tidy/misc-pointer-and-int
>> egral-operation-cxx98.cpp
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/
>> test/clang-tidy/misc-pointer-and-integral-operation-cxx98.
>> cpp?rev=284887&view=auto
>> ============================================================
>> ==================
>> --- clang-tools-extra/trunk/test/clang-tidy/misc-pointer-and-integral-operation-cxx98.cpp
>> (original)
>> +++ clang-tools-extra/trunk/test/clang-tidy/misc-pointer-and-integral-operation-cxx98.cpp
>> (removed)
>> @@ -1,45 +0,0 @@
>> -// RUN: %check_clang_tidy %s misc-pointer-and-integral-operation %t --
>> -- -std=c++98
>> -
>> -bool* pb;
>> -char* pc;
>> -int* pi;
>> -
>> -int Test() {
>> - pb = false;
>> - // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: suspicious assignment from
>> bool to pointer [misc-pointer-and-integral-operation]
>> - pc = '\0';
>> - // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: suspicious assignment from
>> char to pointer
>> -
>> - pb = (false?false:false);
>> - // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: suspicious assignment from
>> bool to pointer
>> - pb = (4 != 5?false:false);
>> - // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: suspicious assignment from
>> bool to pointer
>> -
>> - if (pb < false) return 0;
>> - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious operation
>> between pointer and bool literal
>> - if (pb != false) return 0;
>> - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious operation
>> between pointer and bool literal
>> - if (pc < '\0') return 0;
>> - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious operation
>> between pointer and character literal
>> - if (pc != '\0') return 0;
>> - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious operation
>> between pointer and character literal
>> - if (pi < '\0') return 0;
>> - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious operation
>> between pointer and character literal
>> - if (pi != '\0') return 0;
>> - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious operation
>> between pointer and character literal
>> -
>> - return 1;
>> -}
>> -
>> -int Valid() {
>> - *pb = false;
>> - *pc = '\0';
>> -
>> - pb += 0;
>> - pc += 0;
>> - pi += 0;
>> -
>> - pb += (pb != 0);
>> - pc += (pc != 0);
>> - pi += (pi != 0);
>> -}
>>
>> Removed: clang-tools-extra/trunk/test/clang-tidy/misc-pointer-and-int
>> egral-operation.cpp
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/
>> test/clang-tidy/misc-pointer-and-integral-operation.cpp?
>> rev=284887&view=auto
>> ============================================================
>> ==================
>> --- clang-tools-extra/trunk/test/clang-tidy/misc-pointer-and-integral-operation.cpp
>> (original)
>> +++ clang-tools-extra/trunk/test/clang-tidy/misc-pointer-and-integral-operation.cpp
>> (removed)
>> @@ -1,30 +0,0 @@
>> -// RUN: %check_clang_tidy %s misc-pointer-and-integral-operation %t
>> -
>> -bool* pb;
>> -char* pc;
>> -int* pi;
>> -
>> -int Test() {
>> - if (pi < 0) return 0;
>> - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious comparison of
>> pointer with zero [misc-pointer-and-integral-operation]
>> - if (pi <= 0) return 0;
>> - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious comparison of
>> pointer with zero
>> -
>> - if (nullptr <= pb) return 0;
>> - // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: suspicious comparison of
>> pointer with null
>> - if (pc < nullptr) return 0;
>> - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious comparison of
>> pointer with null
>> - if (pi > nullptr) return 0;
>> - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious comparison of
>> pointer with null
>> -
>> - return 1;
>> -}
>> -
>> -int Valid() {
>> - *pb = false;
>> - *pc = '\0';
>> -
>> - pi += (pi != nullptr);
>> - pi -= (pi == nullptr);
>> - pc += (pb != nullptr);
>> -}
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20161024/8c1194d3/attachment-0001.html>
More information about the cfe-commits
mailing list