On Tue, Jul 8, 2014 at 5:04 PM, Benjamin Kramer <[email protected]> wrote:
> On Tue, Jul 8, 2014 at 5:58 PM, Alexander Kornienko <[email protected]> > wrote: > > Thanks! See one nit below. > > > > On Tue, Jul 8, 2014 at 4:41 PM, Benjamin Kramer < > [email protected]> > > 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"? > > I care about the return value. You're right. But it seems like we've started ignoring the return value of the first FileCheck. We either need to use "bash -e" or add "|| exit $?". > X && Y would return failure if X > failed, I want it to succeed if X failed or Y passed so the other > tests keep functioning. > > I guess an explicit if-then would be doable. Have to check how return > values work when doing that. > They should work fine in ifs. In any case you could use explicit "exit". > > - Ben > > > > >> > >> > >> 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 > >> [email protected] > >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > > > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
