logan-5 updated this revision to Diff 236420.
logan-5 marked 5 inline comments as done.
logan-5 added a comment.
Addressed some formatting stuff.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72282/new/
https://reviews.llvm.org/D72282
Files:
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/clang-tidy/bugprone/UnintendedADLCheck.cpp
clang-tools-extra/clang-tidy/bugprone/UnintendedADLCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone-unintended-adl.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone-unintended-adl.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-unintended-adl.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-unintended-adl.cpp
@@ -0,0 +1,75 @@
+// RUN: %check_clang_tidy %s bugprone-unintended-adl %t
+
+namespace aspace {
+struct A {};
+void func(const A &);
+} // namespace aspace
+
+namespace bspace {
+void func(int);
+void test() {
+ aspace::A a;
+ func(5);
+ func(a);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expression calls 'aspace::func' through ADL [bugprone-unintended-adl]
+}
+} // namespace bspace
+
+namespace ops {
+struct Stream {
+} stream;
+Stream &operator<<(Stream &s, int) {
+ return s;
+}
+void smooth_operator(Stream);
+} // namespace ops
+
+void test() {
+ ops::stream << 5;
+ operator<<(ops::stream, 5);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expression calls 'ops::operator<<' through ADL [bugprone-unintended-adl]
+ smooth_operator(ops::stream);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expression calls 'ops::smooth_operator' through ADL [bugprone-unintended-adl]
+}
+
+namespace ns {
+struct Swappable {};
+void swap(Swappable &a, Swappable &b); // whitelisted by default
+
+void move(Swappable) {} // non-whitelisted
+template <typename T>
+void forward(T) {} // non-whitelisted
+
+struct Swappable2 {};
+} // namespace ns
+
+void move(ns::Swappable2);
+auto ref = [](ns::Swappable2) {};
+
+void test2() {
+ ns::Swappable a, b;
+ using namespace std;
+ swap(a, b);
+ move(a);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expression calls 'ns::move' through ADL [bugprone-unintended-adl]
+ forward(a);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expression calls 'ns::forward' through ADL [bugprone-unintended-adl]
+}
+
+template <typename T>
+void templateFunction(T t) {
+ swap(t, t);
+
+ move(t);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: unqualified call to 'move' may be resolved through ADL [bugprone-unintended-adl]
+
+ ns::move(t);
+ ::move(t);
+
+ ref(t);
+ ::ref(t);
+
+ t << 5;
+ operator<<(t, 5);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: unqualified call to 'operator<<' may be resolved through ADL [bugprone-unintended-adl]
+}
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-unintended-adl.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-unintended-adl.rst
@@ -0,0 +1,47 @@
+.. title:: clang-tidy - bugprone-unintended-adl
+
+bugprone-unintended-adl
+=======================
+
+Finds usages of ADL (argument-dependent lookup), or potential ADL in the case
+of templates, that are not on the provided whitelist.
+
+.. code-block:: c++
+
+ namespace aspace {
+ struct A {};
+ void func(const A &);
+ } // namespace aspace
+
+ namespace bspace {
+ void func(int);
+ void test() {
+ aspace::A a;
+ func(5);
+ func(a); // calls 'aspace::func' through ADL
+ }
+ } // namespace bspace
+
+(Example respectfully borrowed from `Abseil TotW #49 <https://abseil.io/tips/49>`_.)
+
+ADL can be surprising, and can lead to `subtle bugs
+<https://bugs.llvm.org/show_bug.cgi?id=44398>`_ without the utmost attention.
+However, it very is useful for lookup of overloaded operators, and for
+customization points within libraries (e.g. ``swap`` in the C++ standard
+library). As such, this check can be configured to ignore calls to overloaded
+operators as well as other legitimate uses of ADL specified in a whitelist.
+
+This check does not suggest any fixes.
+
+Options
+-------
+
+.. option:: IgnoreOverloadedOperators
+
+ If non-zero, ignores calls to overloaded operators using operator syntax
+ (e.g. ``a + b``), but not function call syntax (e.g. ``operator+(a, b)``).
+ Default is `1`.
+
+.. option:: Whitelist
+
+ Semicolon-separated list of names that the check ignores. Default is `swap`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -104,6 +104,12 @@
Violating the naming rules above results in undefined behavior.
+- New :doc:`bugprone-unintended-adl
+ <clang-tidy/checks/bugprone-unintended-adl>` check.
+
+ Finds usages of ADL (argument-dependent lookup), or potential ADL in the case
+ of templates, that are not on the provided whitelist.
+
- New :doc:`cert-mem57-cpp
<clang-tidy/checks/cert-mem57-cpp>` check.
Index: clang-tools-extra/clang-tidy/bugprone/UnintendedADLCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/UnintendedADLCheck.h
@@ -0,0 +1,40 @@
+//===--- UnintendedADLCheck.h - clang-tidy ----------------------*- 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNINTENDEDADLCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNINTENDEDADLCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include <string>
+#include <vector>
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Finds usages of ADL (argument-dependent lookup), or potential ADL in the
+/// case of templates, that are not on the provided whitelist.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-unintended-adl.html
+class UnintendedADLCheck : public ClangTidyCheck {
+ const bool IgnoreOverloadedOperators;
+ const std::vector<std::string> Whitelist;
+
+public:
+ UnintendedADLCheck(StringRef Name, ClangTidyContext *Context);
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNINTENDEDADLCHECK_H
Index: clang-tools-extra/clang-tidy/bugprone/UnintendedADLCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/UnintendedADLCheck.cpp
@@ -0,0 +1,89 @@
+//===--- UnintendedADLCheck.cpp - clang-tidy ------------------------------===//
+//
+// 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 "UnintendedADLCheck.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include <algorithm>
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, ignoredOperator, bool, IgnoreOverloadedOperators) {
+ return IgnoreOverloadedOperators && isa<CXXOperatorCallExpr>(Node);
+}
+
+AST_MATCHER(UnresolvedLookupExpr, requiresADL) { return Node.requiresADL(); }
+
+AST_MATCHER_P(UnresolvedLookupExpr, isSpelledAsAnyOf, std::vector<StringRef>,
+ Names) {
+ return std::any_of(Names.begin(), Names.end(),
+ [LookupName = Node.getName().getAsString()](
+ StringRef Name) { return Name == LookupName; });
+}
+
+} // namespace
+
+UnintendedADLCheck::UnintendedADLCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ IgnoreOverloadedOperators(Options.get("IgnoreOverloadedOperators", 1)),
+ Whitelist(
+ utils::options::parseStringList(Options.get("Whitelist", "swap"))) {}
+
+void UnintendedADLCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "IgnoreOverloadedOperators", IgnoreOverloadedOperators);
+ Options.store(Opts, "Whitelist",
+ utils::options::serializeStringList(Whitelist));
+}
+
+void UnintendedADLCheck::registerMatchers(MatchFinder *Finder) {
+ const std::vector<StringRef> WhitelistRefs(Whitelist.begin(),
+ Whitelist.end());
+ Finder->addMatcher(
+ callExpr(usesADL(), unless(ignoredOperator(IgnoreOverloadedOperators)),
+ callee(functionDecl(unless(hasAnyName(WhitelistRefs)))
+ .bind("ADLcallee")))
+ .bind("ADLcall"),
+ this);
+
+ Finder->addMatcher(
+ callExpr(unless(ignoredOperator(IgnoreOverloadedOperators)),
+ hasDescendant(
+ unresolvedLookupExpr(requiresADL(),
+ unless(isSpelledAsAnyOf(WhitelistRefs)))
+ .bind("templateADLexpr")))
+ .bind("templateADLcall"),
+ this);
+}
+
+void UnintendedADLCheck::check(const MatchFinder::MatchResult &Result) {
+ if (const auto *Call = Result.Nodes.getNodeAs<CallExpr>("ADLcall")) {
+ const auto *Callee = Result.Nodes.getNodeAs<FunctionDecl>("ADLcallee");
+ diag(Call->getBeginLoc(), "expression calls '%0' through ADL")
+ << Callee->getQualifiedNameAsString();
+ } else {
+ Call = Result.Nodes.getNodeAs<CallExpr>("templateADLcall");
+ assert(Call);
+ const auto *Lookup =
+ Result.Nodes.getNodeAs<UnresolvedLookupExpr>("templateADLexpr");
+ diag(Call->getBeginLoc(),
+ "unqualified call to '%0' may be resolved through ADL")
+ << Lookup->getName().getAsString();
+ }
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -46,6 +46,7 @@
UndefinedMemoryManipulationCheck.cpp
UndelegatedConstructorCheck.cpp
UnhandledSelfAssignmentCheck.cpp
+ UnintendedADLCheck.cpp
UnusedRaiiCheck.cpp
UnusedReturnValueCheck.cpp
UseAfterMoveCheck.cpp
Index: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -54,6 +54,7 @@
#include "UndefinedMemoryManipulationCheck.h"
#include "UndelegatedConstructorCheck.h"
#include "UnhandledSelfAssignmentCheck.h"
+#include "UnintendedADLCheck.h"
#include "UnusedRaiiCheck.h"
#include "UnusedReturnValueCheck.h"
#include "UseAfterMoveCheck.h"
@@ -156,6 +157,8 @@
"bugprone-undelegated-constructor");
CheckFactories.registerCheck<UnhandledSelfAssignmentCheck>(
"bugprone-unhandled-self-assignment");
+ CheckFactories.registerCheck<UnintendedADLCheck>(
+ "bugprone-unintended-adl");
CheckFactories.registerCheck<UnusedRaiiCheck>(
"bugprone-unused-raii");
CheckFactories.registerCheck<UnusedReturnValueCheck>(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
