[clang-tools-extra] r212540 - [clang-tidy] Address review comments for the Twine checker.

[Alexander Kornienko]

Thanks! See one nit below.

On Tue, Jul 8, 2014 at 4:41 PM, Benjamin Kramer <benny.kra at googlemail.com>
wrote:

> Author: d0k
> Date: Tue Jul  8 10:41:20 2014
> New Revision: 212540
>
> URL: http://llvm.org/viewvc/llvm-project?rev=212540&view=rev
> Log:
> [clang-tidy] Address review comments for the Twine checker.
>
> - Remove unused includes.
> - Minor wording fix.
> - Added support to check for clang-tidy messages to check_clang_tidy_fix.sh
> = Updated test case.
>
> Modified:
>     clang-tools-extra/trunk/clang-tidy/llvm/TwineLocalCheck.cpp
>     clang-tools-extra/trunk/clang-tidy/llvm/TwineLocalCheck.h
>     clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy_fix.sh
>     clang-tools-extra/trunk/test/clang-tidy/llvm-twine-local.cpp
>
> Modified: clang-tools-extra/trunk/clang-tidy/llvm/TwineLocalCheck.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/llvm/TwineLocalCheck.cpp?rev=212540&r1=212539&r2=212540&view=diff
>
> ==============================================================================
> --- clang-tools-extra/trunk/clang-tidy/llvm/TwineLocalCheck.cpp (original)
> +++ clang-tools-extra/trunk/clang-tidy/llvm/TwineLocalCheck.cpp Tue Jul  8
> 10:41:20 2014
> @@ -11,7 +11,6 @@
>  #include "clang/AST/ASTContext.h"
>  #include "clang/ASTMatchers/ASTMatchers.h"
>  #include "clang/Lex/Lexer.h"
> -#include "llvm/Support/raw_ostream.h"
>
>  using namespace clang::ast_matchers;
>
> @@ -29,7 +28,7 @@ void TwineLocalCheck::registerMatchers(M
>  void TwineLocalCheck::check(const MatchFinder::MatchResult &Result) {
>    const VarDecl *VD = Result.Nodes.getNodeAs<VarDecl>("variable");
>    auto Diag = diag(VD->getLocation(),
> -                   "twine variables are prone to use after free bugs");
> +                   "twine variables are prone to use-after-free bugs");
>
>    // If this VarDecl has an initializer try to fix it.
>    if (VD->hasInit()) {
>
> Modified: clang-tools-extra/trunk/clang-tidy/llvm/TwineLocalCheck.h
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/llvm/TwineLocalCheck.h?rev=212540&r1=212539&r2=212540&view=diff
>
> ==============================================================================
> --- clang-tools-extra/trunk/clang-tidy/llvm/TwineLocalCheck.h (original)
> +++ clang-tools-extra/trunk/clang-tidy/llvm/TwineLocalCheck.h Tue Jul  8
> 10:41:20 2014
> @@ -11,7 +11,6 @@
>  #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_TWINE_LOCAL_CHECK_H
>
>  #include "../ClangTidy.h"
> -#include "llvm/Support/Regex.h"
>
>  namespace clang {
>  namespace tidy {
>
> Modified: clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy_fix.sh
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy_fix.sh?rev=212540&r1=212539&r2=212540&view=diff
>
> ==============================================================================
> --- clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy_fix.sh
> (original)
> +++ clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy_fix.sh Tue
> Jul  8 10:41:20 2014
> @@ -7,5 +7,6 @@ CHECK_TO_RUN=$2
>  TEMPORARY_FILE=$3.cpp
>
>  grep -Ev "// *[A-Z-]+:" ${INPUT_FILE} > ${TEMPORARY_FILE}
> -clang-tidy ${TEMPORARY_FILE} -fix --checks="-*,${CHECK_TO_RUN}" --
> --std=c++11
> +clang-tidy ${TEMPORARY_FILE} -fix --checks="-*,${CHECK_TO_RUN}" --
> --std=c++11 > ${TEMPORARY_FILE}.msg 2>&1
>  FileCheck -input-file=${TEMPORARY_FILE} ${INPUT_FILE} -strict-whitespace
> +not grep CHECK-MESSAGES ${INPUT_FILE} || FileCheck
> -input-file=${TEMPORARY_FILE}.msg ${INPUT_FILE} -check-prefix=CHECK-MESSAGES
>

Any reason to use "not X || Y" instead of "X && Y" or even "if X ; then Y ;
fi"?


>
> Modified: clang-tools-extra/trunk/test/clang-tidy/llvm-twine-local.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/llvm-twine-local.cpp?rev=212540&r1=212539&r2=212540&view=diff
>
> ==============================================================================
> --- clang-tools-extra/trunk/test/clang-tidy/llvm-twine-local.cpp (original)
> +++ clang-tools-extra/trunk/test/clang-tidy/llvm-twine-local.cpp Tue Jul
>  8 10:41:20 2014
> @@ -1,7 +1,5 @@
> -// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
> -// RUN: clang-tidy %t.cpp -checks='-*,llvm-twine-local' -fix -- > %t.msg
> 2>&1
> -// RUN: FileCheck -input-file=%t.cpp %s
> -// RUN: FileCheck -input-file=%t.msg -check-prefix=CHECK-MESSAGES %s
> +// RUN: $(dirname %s)/check_clang_tidy_fix.sh %s llvm-twine-local %t
> +// REQUIRES: shell
>
>  namespace llvm {
>  class Twine {
> @@ -17,19 +15,19 @@ using namespace llvm;
>  void foo(const Twine &x);
>
>  static Twine Moo = Twine("bark") + "bah";
> -// CHECK-MASSAGES: twine variables are prone to use after free bugs
> +// CHECK-MASSAGES: twine variables are prone to use-after-free bugs
>  // CHECK-MESSAGES: note: FIX-IT applied suggested code changes
>  // CHECK: static std::string Moo = (Twine("bark") + "bah").str();
>
>  int main() {
>    const Twine t = Twine("a") + "b" + Twine(42);
> -// CHECK-MASSAGES: twine variables are prone to use after free bugs
> +// CHECK-MASSAGES: twine variables are prone to use-after-free bugs
>  // CHECK-MESSAGES: note: FIX-IT applied suggested code changes
>  // CHECK: std::string t = (Twine("a") + "b" + Twine(42)).str();
>    foo(Twine("a") + "b");
>
>    Twine Prefix = false ? "__INT_FAST" : "__UINT_FAST";
> -// CHECK-MASSAGES: twine variables are prone to use after free bugs
> +// CHECK-MASSAGES: twine variables are prone to use-after-free bugs
>  // CHECK-MESSAGES: note: FIX-IT applied suggested code changes
>  // CHECK: const char * Prefix = false ? "__INT_FAST" : "__UINT_FAST";
>  }
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140708/0e3dca3c/attachment.html>