[Clang-tidy] Comparison Function Address

[xazax.hun]

Summary

The checker warns in case the address of a function is compared

[Eugene.Zelenko]

Please mention this check in docs/ReleaseNotes.rst (in alphabetical order).

If I'm not mistaken, GCC or Clang has warning that result of comparison is always true for such situations.

clang-tidy/misc/ComparisonFunctionAddressCheck.h

19

Please lowercase Checker. Add add spaces across == below.

docs/clang-tidy/checks/misc-comparison-function-address.rst

6

Please lowercase Checker.

7

Please add space across ==. Please also run Clang-format over examples code.

18

Please highlight nullptr with ``.

test/clang-tidy/misc-comparison-function-address.cpp

16

Unnecessary empty line.

30

Unnecessary empty line.

[Eugene.Zelenko]

Clang-tidy terminology use checks, not checkers.

[Eugene.Zelenko]

This check looks like specific case of D23427. May be they should be merged?

[hokein]

This check looks like specific case of D23427. May be they should be merged?

+1, I think this check can be merged there.

test/clang-tidy/misc-comparison-function-address.cpp

20

How about the case if (getc == func)?

[xazax.hun]

Hi Benedek, could you do the merge or should anybody commandeer these revisions?

[xazax.hun]

-Wtautological-pointer-compare already covers this case.

clang-tidy/misc/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyMiscModule
   ArgumentCommentCheck.cpp
   AssertSideEffectCheck.cpp
   MisplacedConstCheck.cpp
   UnconventionalAssignOperatorCheck.cpp
   BoolPointerImplicitConversionCheck.cpp
+  ComparisonFunctionAddressCheck.cpp
   DanglingHandleCheck.cpp
   DefinitionsInHeadersCheck.cpp
   FoldInitTypeCheck.cpp
   ForwardDeclarationNamespaceCheck.cpp
   InaccurateEraseCheck.cpp
   IncorrectRoundings.cpp
   InefficientAlgorithmCheck.cpp
   MacroParenthesesCheck.cpp

clang-tidy/misc/ComparisonFunctionAddressCheck.h

+//===--- ComparisonFunctionAddressCheck.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_COMPARISON_FUNCTION_ADDRESS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_COMPARISON_FUNCTION_ADDRESS_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// This Checker gives a warning if address of a function is compared.
+/// For example: the programmer wants to write getc()==0 but writes getc==0.
+/// As a general rule, function pointers can be compared to other function
+/// pointers, function, 0, nullptr, functions can be compared only against
+/// function pointers
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-comparison-function-address.html
+class ComparisonFunctionAddressCheck : public ClangTidyCheck {
+public:
+  ComparisonFunctionAddressCheck(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_COMPARISON_FUNCTION_ADDRESS_H

clang-tidy/misc/ComparisonFunctionAddressCheck.cpp

+//===--- ComparisonFunctionAddressCheck.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 "ComparisonFunctionAddressCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+void ComparisonFunctionAddressCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      binaryOperator(
+          hasEitherOperand(hasType(pointerType(pointee(functionType())))),
+          unless(hasEitherOperand(ignoringImpCasts(
+              declRefExpr(hasDeclaration(varDecl(hasType(pointerType()))))))))
+          .bind("funcAdd"),
+      this);
+}
+
+void ComparisonFunctionAddressCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *MatchedDecl = Result.Nodes.getNodeAs<BinaryOperator>("funcAdd");
+  diag(MatchedDecl->getOperatorLoc(), "Address of function is compared");
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+

clang-tidy/misc/MiscTidyModule.cpp

 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "ArgumentCommentCheck.h"
 #include "AssertSideEffectCheck.h"
 #include "MisplacedConstCheck.h"
 #include "UnconventionalAssignOperatorCheck.h"
 #include "BoolPointerImplicitConversionCheck.h"
+#include "ComparisonFunctionAddressCheck.h"
 #include "DanglingHandleCheck.h"
 #include "DefinitionsInHeadersCheck.h"
 #include "FoldInitTypeCheck.h"
 #include "ForwardDeclarationNamespaceCheck.h"
 #include "InaccurateEraseCheck.h"
 #include "IncorrectRoundings.h"
 #include "InefficientAlgorithmCheck.h"
 #include "MacroParenthesesCheck.h"
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<AssertSideEffectCheck>(
         "misc-assert-side-effect");
     CheckFactories.registerCheck<MisplacedConstCheck>(
         "misc-misplaced-const");
     CheckFactories.registerCheck<UnconventionalAssignOperatorCheck>(
         "misc-unconventional-assign-operator");
     CheckFactories.registerCheck<BoolPointerImplicitConversionCheck>(
         "misc-bool-pointer-implicit-conversion");
+    CheckFactories.registerCheck<ComparisonFunctionAddressCheck>(
+        "misc-comparison-function-address");
     CheckFactories.registerCheck<DanglingHandleCheck>(
         "misc-dangling-handle");
     CheckFactories.registerCheck<DefinitionsInHeadersCheck>(
         "misc-definitions-in-headers");
     CheckFactories.registerCheck<FoldInitTypeCheck>(
         "misc-fold-init-type");
     CheckFactories.registerCheck<ForwardDeclarationNamespaceCheck>(
         "misc-forward-declaration-namespace");

docs/clang-tidy/checks/list.rst

.. toctree::
    google-runtime-references
    llvm-header-guard
    llvm-include-order
    llvm-namespace-comment
    llvm-twine-local
    misc-argument-comment
    misc-assert-side-effect
    misc-bool-pointer-implicit-conversion
+   misc-comparison-function-address
    misc-dangling-handle
    misc-definitions-in-headers
    misc-fold-init-type
    misc-forward-declaration-namespace
    misc-inaccurate-erase
    misc-incorrect-roundings
    misc-inefficient-algorithm
    misc-macro-parentheses

docs/clang-tidy/checks/misc-comparison-function-address.rst

+.. title:: clang-tidy - misc-comparison-function-address
+
+misc-comparison-function-address
+================================
+
+This Checker gives a warning if address of a function is compared.
+For example: the programmer wants to write ``getc()==0`` but writes ``getc==0``
+
+.. code-block:: c++ 
+  if (getc== 0); // warning: Address of strlen is compared
+
+but comparison to function pointer should be allowed
+
+.. code-block:: c++
+  int (* myfp)(char const * ) = 0;
+  if (getc ==myfp); // should be ok
+
+as a general rule, function pointers can be compared to other function pointers, function, 0, nullptr
+functions can be compared only against function pointers

test/clang-tidy/misc-comparison-function-address.cpp

+// RUN: %check_clang_tidy %s misc-comparison-function-address %t
+
+#define nullptr 0
+#define NULL    0
+
+struct __sFile {
+  int unused;
+};
+  
+typedef struct __sFILE FILE;
+
+int getc(FILE * stream);
+char func();
+char * func2();
+
+
+void test_function_warning() {
+  if (getc == 0); 
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: Address of function is compared [misc-comparison-function-address]
+  
+  if (0 == getc);
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: Address of function is compared [misc-comparison-function-address]  
+ 
+  if (getc == nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: Address of function is compared [misc-comparison-function-address]
+
+  if (NULL == func);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: Address of function is compared [misc-comparison-function-address]
+}
+
+
+void test_function_awsome() {
+  FILE * pFile;
+  int (*myfp)(FILE *) = 0;
+  char (*myfp2)();
+  
+  if (getc == myfp);
+
+  if (0 == myfp);
+
+  if (getc(pFile) == 0);
+
+  if (myfp == getc);
+
+  if (myfp == 0);
+
+  if (myfp == NULL);
+
+  if (func == myfp2);
+}
+