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. 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. - 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
