[clang-tools-extra] r212540 - [clang-tidy] Address review comments for the Twine checker.
Benjamin Kramer
benny.kra at gmail.com
Tue Jul 8 09:04:43 PDT 2014
On Tue, Jul 8, 2014 at 5:58 PM, Alexander Kornienko <alexfh at google.com> wrote:
> 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"?
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
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
More information about the cfe-commits
mailing list