diff --git a/clang-tools-extra/clang-tidy/boost/BoostTidyModule.cpp b/clang-tools-extra/clang-tidy/boost/BoostTidyModule.cpp --- a/clang-tools-extra/clang-tidy/boost/BoostTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/boost/BoostTidyModule.cpp @@ -9,6 +9,7 @@ #include "../ClangTidy.h" #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" +#include "UseRangeBasedForLoopCheck.h" #include "UseToStringCheck.h" using namespace clang::ast_matchers; @@ -19,6 +20,8 @@ class BoostModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck( + "boost-use-range-based-for-loop"); CheckFactories.registerCheck("boost-use-to-string"); } }; diff --git a/clang-tools-extra/clang-tidy/boost/CMakeLists.txt b/clang-tools-extra/clang-tidy/boost/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/boost/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/boost/CMakeLists.txt @@ -5,6 +5,7 @@ add_clang_library(clangTidyBoostModule BoostTidyModule.cpp + UseRangeBasedForLoopCheck.cpp UseToStringCheck.cpp LINK_LIBS diff --git a/clang-tools-extra/clang-tidy/boost/UseRangeBasedForLoopCheck.h b/clang-tools-extra/clang-tidy/boost/UseRangeBasedForLoopCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/boost/UseRangeBasedForLoopCheck.h @@ -0,0 +1,34 @@ +//===--- UseRangeBasedForLoopCheck.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_BOOST_USERANGEBASEDFORLOOPCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BOOST_USERANGEBASEDFORLOOPCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace boost { + +/// FIXME: Write a short description. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/boost-use-range-based-for-loop.html +class UseRangeBasedForLoopCheck : public ClangTidyCheck { +public: + using ClangTidyCheck::ClangTidyCheck; + + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) override; +}; + +} // namespace boost +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BOOST_USERANGEBASEDFORLOOPCHECK_H diff --git a/clang-tools-extra/clang-tidy/boost/UseRangeBasedForLoopCheck.cpp b/clang-tools-extra/clang-tidy/boost/UseRangeBasedForLoopCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/boost/UseRangeBasedForLoopCheck.cpp @@ -0,0 +1,84 @@ +//===--- UseRangeBasedForLoopCheck.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 "UseRangeBasedForLoopCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Lex/MacroArgs.h" +#include "clang/Lex/PPCallbacks.h" +#include "clang/Lex/Preprocessor.h" +#include + +namespace clang { +namespace tidy { +namespace boost { + +// Determines whether the macro is a Boost Foreach macro. +static bool isBoostForeachMacro(StringRef MacroName) { + return MacroName == "BOOST_FOREACH"; +} + +namespace { + +class UseRangeBasedForLoopCallback : public PPCallbacks { +public: + explicit UseRangeBasedForLoopCallback(Preprocessor *PP, + UseRangeBasedForLoopCheck *Check) + : PP(PP), Check(Check) {} + + // Detects expansion of the BOOST_FOREACH macro + void MacroExpands(const Token &MacroNameToken, + const MacroDefinition &MacroDefinition, SourceRange Range, + const MacroArgs *Args) override { + IdentifierInfo *NameIdentifierInfo = MacroNameToken.getIdentifierInfo(); + if (!NameIdentifierInfo) + return; + StringRef MacroName = NameIdentifierInfo->getName(); + if (!isBoostForeachMacro(MacroName) || !Args || + Args->getNumMacroArguments() < 2) + return; + const Token *VariableTokens = Args->getUnexpArgument(0); + const Token *ColToken = Args->getUnexpArgument(1); + if (!VariableTokens || !ColToken) + return; + + unsigned VariableNumToks = clang::MacroArgs::getArgLength(VariableTokens); + const char *Divider = ""; + std::string VariableString = std::accumulate( + VariableTokens, VariableTokens + VariableNumToks, std::string(), + [this, &Divider](std::string A, const Token &T) { + return std::move(A) + std::exchange(Divider, " ") + + PP->getSpelling(T); + }); + + auto Loc = MacroNameToken.getLocation(); + auto Diag = Check->diag(Loc, "use range-based for loop instead of %0") + << MacroName; + + Diag << FixItHint::CreateReplacement( + CharSourceRange::getCharRange(MacroNameToken.getLocation(), + ColToken->getLocation()), + (llvm::Twine("for(") + VariableString + " : ").str()); + } + +private: + Preprocessor *PP; + UseRangeBasedForLoopCheck *Check; +}; + +} // namespace + +void UseRangeBasedForLoopCheck::registerPPCallbacks( + const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { + PP->addPPCallbacks(std::make_unique(PP, this)); +} + +} // namespace boost +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -76,6 +76,11 @@ New checks ^^^^^^^^^^ +- New :doc:`boost-use-range-based-for-loop + ` check. + + Suggests converting ``BOOST_FOREACH(..., ...)`` loops to more modern way. + - New :doc:`bugprone-stringview-nullptr ` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/boost-use-range-based-for-loop.rst b/clang-tools-extra/docs/clang-tidy/checks/boost-use-range-based-for-loop.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/boost-use-range-based-for-loop.rst @@ -0,0 +1,65 @@ +.. title:: clang-tidy - boost-use-range-based-for-loop + +boost-use-range-based-for-loop +============================== + +This check converts ``BOOST_FOREACH(..., ...)`` loops to use the new range-based loops in C++11. + +Two kinds of loops can be converted correctly: + +- Loops over arrays. +- Loops over STL containers. + +.. code-block:: c++ + + std::list list_int( /*...*/ ); + BOOST_FOREACH( int i, list_int ) + { + // do something with i + } + + // Will be changed to + std::list list_int( /*...*/ ); + for( int i: list_int ) + { + // do something with i + } + +Known Limitations +----------------- +* Client code that use ``BOOST_FOREACH`` with "``std::pair`` of iterators" or with "Null-terminated strings (``char`` and ``wchar_t``)" or some other sequence types (everything that is not declared in the list "convertible correctly" above) will produce broken code (compilation error). + +.. code-block:: c++ + + const char* cstr = "Hello world"; + BOOST_FOREACH( char c, cstr ) + { + // do something with c + } + + // Will be changed to + const char* cstr = "Hello world"; + for( char c: cstr ) // won't compile + { + // do something with c + } + +* First argument of the ``BOOST_FOREACH`` macro must be only new identifier definition, all other will produce a compilation error after migration. + +.. code-block:: c++ + + std::list list_int( /*...*/ ); + int i; + BOOST_FOREACH( i, list_int ) + { + // do something with i + } + + // Will be changed to + std::list list_int( /*...*/ ); + int i; + for( i: list_int ) // won't compile + { + // do something with i + } + diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -51,6 +51,7 @@ `android-cloexec-pipe2 `_, `android-cloexec-socket `_, `android-comparison-in-temp-failure-retry `_, + `boost-use-range-based-for-loop `_, "Yes" `boost-use-to-string `_, "Yes" `bugprone-argument-comment `_, "Yes" `bugprone-assert-side-effect `_, diff --git a/clang-tools-extra/test/clang-tidy/checkers/boost-use-range-based-for-loop.cpp b/clang-tools-extra/test/clang-tidy/checkers/boost-use-range-based-for-loop.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/boost-use-range-based-for-loop.cpp @@ -0,0 +1,41 @@ +// RUN: %check_clang_tidy %s boost-use-range-based-for-loop %t + +namespace std { +template +class list {}; +} + +#define BOOST_FOREACH(VAR, COL) while(true) +#define BOOST_REVERSE_FOREACH(VAR, COL) while(true) + +void test_range_based_for_loop() +{ + std::list list_int; + BOOST_FOREACH( int i, list_int ) { + // do something with i + } + // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead of BOOST_FOREACH [boost-use-range-based-for-loop] + // CHECK-FIXES: for(int i : list_int ) { + + short array_short[] = {1,2,3}; + BOOST_FOREACH(int + val, array_short) { + /// The short was implicitly converted to an int + } + // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based + // CHECK-FIXES: for(int val : array_short) { +} + +#define foreach_ BOOST_FOREACH +#define foreach_r_ BOOST_REVERSE_FOREACH + +void test_range_based_for_loop_prettier() +{ + std::list list_int; + foreach_( int i, list_int ) { + // do something with i + } + // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based + // CHECK-FIXES: for(int i : list_int ) { +} +