[clang-tidy] check for __func__/__FUNCTION__ in lambdas

Add a clang-tidy check for using func__/FUNCTION__ inside lambdas. This
evaluates to the string operator(), which is almost never useful and almost
certainly not what the author intended.

Patch by Bryce Liu!

Differential Revision: https://reviews.llvm.org/D33497

llvm-svn: 304570
This commit is contained in:
Alexander Kornienko 2017-06-02 17:36:32 +00:00
parent cdb5dad4cc
commit d9a4491b0a
8 changed files with 228 additions and 0 deletions

View File

@ -4,6 +4,7 @@ add_clang_library(clangTidyMiscModule
ArgumentCommentCheck.cpp
AssertSideEffectCheck.cpp
ForwardingReferenceOverloadCheck.cpp
LambdaFunctionNameCheck.cpp
MisplacedConstCheck.cpp
UnconventionalAssignOperatorCheck.cpp
BoolPointerImplicitConversionCheck.cpp

View File

@ -0,0 +1,99 @@
//===--- LambdaFunctionNameCheck.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 "LambdaFunctionNameCheck.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Lex/MacroInfo.h"
#include "clang/Lex/Preprocessor.h"
using namespace clang::ast_matchers;
namespace clang {
namespace tidy {
namespace misc {
namespace {
// Keep track of macro expansions that contain both __FILE__ and __LINE__. If
// such a macro also uses __func__ or __FUNCTION__, we don't want to issue a
// warning because __FILE__ and __LINE__ may be useful even if __func__ or
// __FUNCTION__ is not, especially if the macro could be used in the context of
// either a function body or a lambda body.
class MacroExpansionsWithFileAndLine : public PPCallbacks {
public:
explicit MacroExpansionsWithFileAndLine(
LambdaFunctionNameCheck::SourceRangeSet *SME)
: SuppressMacroExpansions(SME) {}
void MacroExpands(const Token &MacroNameTok,
const MacroDefinition &MD, SourceRange Range,
const MacroArgs *Args) override {
bool has_file = false;
bool has_line = false;
for (const auto& T : MD.getMacroInfo()->tokens()) {
if (T.is(tok::identifier)) {
StringRef IdentName = T.getIdentifierInfo()->getName();
if (IdentName == "__FILE__") {
has_file = true;
} else if (IdentName == "__LINE__") {
has_line = true;
}
}
}
if (has_file && has_line) {
SuppressMacroExpansions->insert(Range);
}
}
private:
LambdaFunctionNameCheck::SourceRangeSet* SuppressMacroExpansions;
};
} // namespace
void LambdaFunctionNameCheck::registerMatchers(MatchFinder *Finder) {
// Match on PredefinedExprs inside a lambda.
Finder->addMatcher(predefinedExpr(hasAncestor(lambdaExpr())).bind("E"),
this);
}
void LambdaFunctionNameCheck::registerPPCallbacks(CompilerInstance &Compiler) {
Compiler.getPreprocessor().addPPCallbacks(
llvm::make_unique<MacroExpansionsWithFileAndLine>(
&SuppressMacroExpansions));
}
void LambdaFunctionNameCheck::check(const MatchFinder::MatchResult &Result) {
const auto *E = Result.Nodes.getNodeAs<PredefinedExpr>("E");
if (E->getIdentType() != PredefinedExpr::Func &&
E->getIdentType() != PredefinedExpr::Function) {
// We don't care about other PredefinedExprs.
return;
}
if (E->getLocation().isMacroID()) {
auto ER =
Result.SourceManager->getImmediateExpansionRange(E->getLocation());
if (SuppressMacroExpansions.find(SourceRange(ER.first, ER.second)) !=
SuppressMacroExpansions.end()) {
// This is a macro expansion for which we should not warn.
return;
}
}
diag(E->getLocation(),
"inside a lambda, '%0' expands to the name of the function call "
"operator; consider capturing the name of the enclosing function "
"explicitly")
<< PredefinedExpr::getIdentTypeName(E->getIdentType());
}
} // namespace misc
} // namespace tidy
} // namespace clang

View File

@ -0,0 +1,51 @@
//===--- LambdaFunctionNameCheck.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_LAMBDA_FUNCTION_NAME_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_LAMBDA_FUNCTION_NAME_H
#include "../ClangTidy.h"
namespace clang {
namespace tidy {
namespace misc {
/// Detect when __func__ or __FUNCTION__ is being used from within a lambda. In
/// that context, those expressions expand to the name of the call operator
/// (i.e., `operator()`).
///
/// For the user-facing documentation see:
/// http://clang.llvm.org/extra/clang-tidy/checks/misc-lambda-function-name.html
class LambdaFunctionNameCheck : public ClangTidyCheck {
public:
struct SourceRangeLessThan {
bool operator()(const SourceRange &L, const SourceRange &R) {
if (L.getBegin() == R.getBegin()) {
return L.getEnd() < R.getEnd();
}
return L.getBegin() < R.getBegin();
}
};
using SourceRangeSet = std::set<SourceRange, SourceRangeLessThan>;
LambdaFunctionNameCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context) {}
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void registerPPCallbacks(CompilerInstance &Compiler) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
private:
SourceRangeSet SuppressMacroExpansions;
};
} // namespace misc
} // namespace tidy
} // namespace clang
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_LAMBDA_FUNCTION_NAME_H

View File

@ -21,6 +21,7 @@
#include "InaccurateEraseCheck.h"
#include "IncorrectRoundings.h"
#include "InefficientAlgorithmCheck.h"
#include "LambdaFunctionNameCheck.h"
#include "MacroParenthesesCheck.h"
#include "MacroRepeatedSideEffectsCheck.h"
#include "MisplacedConstCheck.h"
@ -68,6 +69,8 @@ public:
"misc-assert-side-effect");
CheckFactories.registerCheck<ForwardingReferenceOverloadCheck>(
"misc-forwarding-reference-overload");
CheckFactories.registerCheck<LambdaFunctionNameCheck>(
"misc-lambda-function-name");
CheckFactories.registerCheck<MisplacedConstCheck>("misc-misplaced-const");
CheckFactories.registerCheck<UnconventionalAssignOperatorCheck>(
"misc-unconventional-assign-operator");

View File

@ -77,6 +77,11 @@ Improvements to clang-tidy
Finds perfect forwarding constructors that can unintentionally hide copy or move constructors.
- New `misc-lambda-function-name
<http://clang.llvm.org/extra/clang-tidy/checks/misc-lambda-function-name.html>`_ check
Finds uses of ``__func__`` or ``__FUNCTION__`` inside lambdas.
- New `modernize-replace-random-shuffle
<http://clang.llvm.org/extra/clang-tidy/checks/modernize-replace-random-shuffle.html>`_ check

View File

@ -82,6 +82,7 @@ Clang-Tidy Checks
misc-inaccurate-erase
misc-incorrect-roundings
misc-inefficient-algorithm
misc-lambda-function-name
misc-macro-parentheses
misc-macro-repeated-side-effects
misc-misplaced-const

View File

@ -0,0 +1,27 @@
.. title:: clang-tidy - misc-lambda-function-name
misc-lambda-function-name
=========================
Checks for attempts to get the name of a function from within a lambda
expression. The name of a lambda is always something like ``operator()``, which
is almost never what was intended.
Example:
.. code-block:: c++
void FancyFunction() {
[] { printf("Called from %s\n", __func__); }();
[] { printf("Now called from %s\n", __FUNCTION__); }();
}
Output::
Called from operator()
Now called from operator()
Likely intended output::
Called from FancyFunction
Now called from FancyFunction

View File

@ -0,0 +1,41 @@
// RUN: %check_clang_tidy %s misc-lambda-function-name %t
void Foo(const char* a, const char* b, int c) {}
#define FUNC_MACRO Foo(__func__, "", 0)
#define FUNCTION_MACRO Foo(__FUNCTION__, "", 0)
#define EMBED_IN_ANOTHER_MACRO1 FUNC_MACRO
void Positives() {
[] { __func__; }();
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__func__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [misc-lambda-function-name]
[] { __FUNCTION__; }();
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__FUNCTION__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [misc-lambda-function-name]
[] { FUNC_MACRO; }();
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__func__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [misc-lambda-function-name]
[] { FUNCTION_MACRO; }();
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__FUNCTION__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [misc-lambda-function-name]
[] { EMBED_IN_ANOTHER_MACRO1; }();
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__func__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [misc-lambda-function-name]
}
#define FUNC_MACRO_WITH_FILE_AND_LINE Foo(__func__, __FILE__, __LINE__)
#define FUNCTION_MACRO_WITH_FILE_AND_LINE Foo(__FUNCTION__, __FILE__, __LINE__)
#define EMBED_IN_ANOTHER_MACRO2 FUNC_MACRO_WITH_FILE_AND_LINE
void Negatives() {
__func__;
__FUNCTION__;
// __PRETTY_FUNCTION__ should not trigger a warning because its value is
// actually potentially useful.
__PRETTY_FUNCTION__;
[] { __PRETTY_FUNCTION__; }();
// Don't warn if __func__/__FUNCTION is used inside a macro that also uses
// __FILE__ and __LINE__, on the assumption that __FILE__ and __LINE__ will
// be useful even if __func__/__FUNCTION__ is not.
[] { FUNC_MACRO_WITH_FILE_AND_LINE; }();
[] { FUNCTION_MACRO_WITH_FILE_AND_LINE; }();
[] { EMBED_IN_ANOTHER_MACRO2; }();
}