Re: r277923 - [ASTReader] Use real move semantics instead of emulating them in the copy ctor.
The members are references, pointers and a bool. None of them requires
moving as they're trivially copyable. We actually have a clang-tidy
check to remove std::move in those cases.
On Mon, Aug 8, 2016 at 3:18 AM, Piotr Padlewski
wrote:
>
>
> 2016-08-06 5:45 GMT-07:00 Benjamin Kramer via cfe-commits
> :
>>
>> Author: d0k
>> Date: Sat Aug 6 07:45:16 2016
>> New Revision: 277923
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=277923&view=rev
>> Log:
>> [ASTReader] Use real move semantics instead of emulating them in the copy
>> ctor.
>>
>> No functionality change intended.
>>
>> Modified:
>> cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
>>
>> Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=277923&r1=277922&r2=277923&view=diff
>>
>> ==
>> --- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
>> +++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Sat Aug 6 07:45:16 2016
>> @@ -170,12 +170,12 @@ namespace clang {
>>ASTReader &Reader;
>>NamedDecl *New;
>>NamedDecl *Existing;
>> - mutable bool AddResult;
>> + bool AddResult;
>>
>>unsigned AnonymousDeclNumber;
>>IdentifierInfo *TypedefNameForLinkage;
>>
>> - void operator=(FindExistingResult&) = delete;
>> + void operator=(FindExistingResult &&) = delete;
>>
>> public:
>>FindExistingResult(ASTReader &Reader)
>> @@ -189,7 +189,7 @@ namespace clang {
>> AnonymousDeclNumber(AnonymousDeclNumber),
>> TypedefNameForLinkage(TypedefNameForLinkage) {}
>>
>> - FindExistingResult(const FindExistingResult &Other)
>> + FindExistingResult(FindExistingResult &&Other)
>>: Reader(Other.Reader), New(Other.New),
>> Existing(Other.Existing),
>> AddResult(Other.AddResult),
>> AnonymousDeclNumber(Other.AnonymousDeclNumber),
>>
> Shouldn't these lines have std::move() everywhere to make them real move
> ctors?
>
>
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23060: [analyzer] Show enabled checker list
xazax.hun updated this revision to Diff 67129.
xazax.hun marked an inline comment as done.
xazax.hun added a comment.
- Changes according to the review comments.
https://reviews.llvm.org/D23060
Files:
include/clang/Driver/CC1Options.td
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
include/clang/StaticAnalyzer/Core/CheckerRegistry.h
include/clang/StaticAnalyzer/Frontend/FrontendActions.h
lib/Frontend/CompilerInvocation.cpp
lib/FrontendTool/ExecuteCompilerInvocation.cpp
lib/StaticAnalyzer/Core/CheckerRegistry.cpp
lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
test/Analysis/analyzer-enabled-checkers.c
Index: test/Analysis/analyzer-enabled-checkers.c
===
--- /dev/null
+++ test/Analysis/analyzer-enabled-checkers.c
@@ -0,0 +1,20 @@
+// RUN: %clang -target x86_64-apple-darwin10 --analyze %s -o /dev/null -Xclang -analyzer-checker=core -Xclang -analyzer-list-enabled-checkers > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+
+// CHECK: OVERVIEW: Clang Static Analyzer Enabled Checkers List
+// CHECK: core.CallAndMessage
+// CHECK: core.DivideZero
+// CHECK: core.DynamicTypePropagation
+// CHECK: core.NonNullParamChecker
+// CHECK: core.NullDereference
+// CHECK: core.StackAddressEscape
+// CHECK: core.UndefinedBinaryOperatorResult
+// CHECK: core.VLASize
+// CHECK: core.builtin.BuiltinFunctions
+// CHECK: core.builtin.NoReturnFunctions
+// CHECK: core.uninitialized.ArraySubscript
+// CHECK: core.uninitialized.Assign
+// CHECK: core.uninitialized.Branch
+// CHECK: core.uninitialized.CapturedBlockVariable
+// CHECK: core.uninitialized.UndefReturn
+
Index: lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
===
--- lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
+++ lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
@@ -101,18 +101,24 @@
<< pluginAPIVersion;
}
+static SmallVector
+getCheckerOptList(const AnalyzerOptions &opts) {
+ SmallVector checkerOpts;
+ for (unsigned i = 0, e = opts.CheckersControlList.size(); i != e; ++i) {
+const std::pair &opt = opts.CheckersControlList[i];
+checkerOpts.push_back(CheckerOptInfo(opt.first.c_str(), opt.second));
+ }
+ return checkerOpts;
+}
+
std::unique_ptr
ento::createCheckerManager(AnalyzerOptions &opts, const LangOptions &langOpts,
ArrayRef plugins,
DiagnosticsEngine &diags) {
std::unique_ptr checkerMgr(
new CheckerManager(langOpts, &opts));
- SmallVector checkerOpts;
- for (unsigned i = 0, e = opts.CheckersControlList.size(); i != e; ++i) {
-const std::pair &opt = opts.CheckersControlList[i];
-checkerOpts.push_back(CheckerOptInfo(opt.first.c_str(), opt.second));
- }
+ SmallVector checkerOpts = getCheckerOptList(opts);
ClangCheckerRegistry allCheckers(plugins, &diags);
allCheckers.initializeManager(*checkerMgr, checkerOpts);
@@ -137,3 +143,12 @@
ClangCheckerRegistry(plugins).printHelp(out);
}
+
+void ento::printEnabledCheckerList(raw_ostream &out,
+ ArrayRef plugins,
+ const AnalyzerOptions &opts) {
+ out << "OVERVIEW: Clang Static Analyzer Enabled Checkers List\n\n";
+
+ SmallVector checkerOpts = getCheckerOptList(opts);
+ ClangCheckerRegistry(plugins).printList(out, checkerOpts);
+}
Index: lib/StaticAnalyzer/Core/CheckerRegistry.cpp
===
--- lib/StaticAnalyzer/Core/CheckerRegistry.cpp
+++ lib/StaticAnalyzer/Core/CheckerRegistry.cpp
@@ -175,3 +175,22 @@
out << '\n';
}
}
+
+void CheckerRegistry::printList(
+raw_ostream &out, SmallVectorImpl &opts) const {
+ std::sort(Checkers.begin(), Checkers.end(), checkerNameLT);
+
+ // Collect checkers enabled by the options.
+ CheckerInfoSet enabledCheckers;
+ for (SmallVectorImpl::iterator i = opts.begin(),
+ e = opts.end();
+ i != e; ++i) {
+collectCheckers(Checkers, Packages, *i, enabledCheckers);
+ }
+
+ for (CheckerInfoSet::const_iterator i = enabledCheckers.begin(),
+ e = enabledCheckers.end();
+ i != e; ++i) {
+out << (*i)->FullName << '\n';
+ }
+}
Index: lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -229,6 +229,11 @@
ento::printCheckerHelp(llvm::outs(), Clang->getFrontendOpts().Plugins);
return true;
}
+ if (Clang->getAnalyzerOpts()->ShowEnabledCheckerList) {
+ento::printEnabledCheckerList(llvm::outs(),
+ Clang->getFrontendOpts().Plugins,
+ *Clang->getAnalyzerOpts());
+ }
#endif
// If there were errors in processing arguments, don't
Re: [PATCH] D23198: clang-rename rename-all: support reading old/newname pairs from a YAML file
omtcyfz added a comment. Ah, and yes, it's better to move `*.yaml` to `extra/test/clang-rename/Inputs` in order to prevent confusion as every other file in `extra/test/clang-rename` is a test. https://reviews.llvm.org/D23198 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r277985 - Fix two bugs for musl-libc on ARM
Author: rovka
Date: Mon Aug 8 03:27:36 2016
New Revision: 277985
URL: http://llvm.org/viewvc/llvm-project?rev=277985&view=rev
Log:
Fix two bugs for musl-libc on ARM
Bug 1: triples like armv7-pc-linux-musl use the wrong linker name
ld-musl-armv7.so.1; the right name should be ld-musl-arm.so.1, disregarding the
subarch field.
Bug 2: when compiler option -mhard-float is used, we should use the "hardfloat"
linker, no matter whether the triple itself mentions "hardfloat".
Patch by Lei Zhang!
Differential Revision: https://reviews.llvm.org/D22904
Modified:
cfe/trunk/lib/Driver/ToolChains.cpp
cfe/trunk/test/Driver/linux-ld.c
Modified: cfe/trunk/lib/Driver/ToolChains.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.cpp?rev=277985&r1=277984&r2=277985&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains.cpp Mon Aug 8 03:27:36 2016
@@ -4274,19 +4274,28 @@ std::string Linux::getDynamicLinker(cons
if (Triple.isAndroid())
return Triple.isArch64Bit() ? "/system/bin/linker64" :
"/system/bin/linker";
- else if (Triple.isMusl()) {
+
+ if (Triple.isMusl()) {
std::string ArchName;
+bool IsArm = false;
+
switch (Arch) {
+case llvm::Triple::arm:
case llvm::Triple::thumb:
ArchName = "arm";
+ IsArm = true;
break;
+case llvm::Triple::armeb:
case llvm::Triple::thumbeb:
ArchName = "armeb";
+ IsArm = true;
break;
default:
ArchName = Triple.getArchName().str();
}
-if (Triple.getEnvironment() == llvm::Triple::MuslEABIHF)
+if (IsArm &&
+(Triple.getEnvironment() == llvm::Triple::MuslEABIHF ||
+ tools::arm::getARMFloatABI(*this, Args) ==
tools::arm::FloatABI::Hard))
ArchName += "hf";
return "/lib/ld-musl-" + ArchName + ".so.1";
Modified: cfe/trunk/test/Driver/linux-ld.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/linux-ld.c?rev=277985&r1=277984&r2=277985&view=diff
==
--- cfe/trunk/test/Driver/linux-ld.c (original)
+++ cfe/trunk/test/Driver/linux-ld.c Mon Aug 8 03:27:36 2016
@@ -1613,24 +1613,36 @@
// RUN: --target=thumb-pc-linux-musleabihf \
// RUN: | FileCheck --check-prefix=CHECK-MUSL-ARMHF %s
// RUN: %clang %s -### -o %t.o 2>&1 \
+// RUN: --target=thumbv7-pc-linux-musleabi -mhard-float \
+// RUN: | FileCheck --check-prefix=CHECK-MUSL-ARMHF %s
+// RUN: %clang %s -### -o %t.o 2>&1 \
// RUN: --target=thumbeb-pc-linux-musleabi \
// RUN: | FileCheck --check-prefix=CHECK-MUSL-ARMEB %s
// RUN: %clang %s -### -o %t.o 2>&1 \
// RUN: --target=thumbeb-pc-linux-musleabihf \
// RUN: | FileCheck --check-prefix=CHECK-MUSL-ARMEBHF %s
// RUN: %clang %s -### -o %t.o 2>&1 \
+// RUN: --target=thumbv7eb-pc-linux-musleabi -mhard-float \
+// RUN: | FileCheck --check-prefix=CHECK-MUSL-ARMEBHF %s
+// RUN: %clang %s -### -o %t.o 2>&1 \
// RUN: --target=arm-pc-linux-musleabi \
// RUN: | FileCheck --check-prefix=CHECK-MUSL-ARM %s
// RUN: %clang %s -### -o %t.o 2>&1 \
// RUN: --target=arm-pc-linux-musleabihf \
// RUN: | FileCheck --check-prefix=CHECK-MUSL-ARMHF %s
// RUN: %clang %s -### -o %t.o 2>&1 \
+// RUN: --target=armv7-pc-linux-musleabi -mhard-float \
+// RUN: | FileCheck --check-prefix=CHECK-MUSL-ARMHF %s
+// RUN: %clang %s -### -o %t.o 2>&1 \
// RUN: --target=armeb-pc-linux-musleabi \
// RUN: | FileCheck --check-prefix=CHECK-MUSL-ARMEB %s
// RUN: %clang %s -### -o %t.o 2>&1 \
// RUN: --target=armeb-pc-linux-musleabihf \
// RUN: | FileCheck --check-prefix=CHECK-MUSL-ARMEBHF %s
// RUN: %clang %s -### -o %t.o 2>&1 \
+// RUN: --target=armv7eb-pc-linux-musleabi -mhard-float \
+// RUN: | FileCheck --check-prefix=CHECK-MUSL-ARMEBHF %s
+// RUN: %clang %s -### -o %t.o 2>&1 \
// RUN: --target=aarch64-pc-linux-musleabi \
// RUN: | FileCheck --check-prefix=CHECK-MUSL-AARCH64 %s
// RUN: %clang %s -### -o %t.o 2>&1 \
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22904: Fix two bugs for musl-libc on ARM
This revision was automatically updated to reflect the committed changes.
Closed by commit rL277985: Fix two bugs for musl-libc on ARM (authored by
rovka).
Changed prior to commit:
https://reviews.llvm.org/D22904?vs=66074&id=67131#toc
Repository:
rL LLVM
https://reviews.llvm.org/D22904
Files:
cfe/trunk/lib/Driver/ToolChains.cpp
cfe/trunk/test/Driver/linux-ld.c
Index: cfe/trunk/test/Driver/linux-ld.c
===
--- cfe/trunk/test/Driver/linux-ld.c
+++ cfe/trunk/test/Driver/linux-ld.c
@@ -1613,24 +1613,36 @@
// RUN: --target=thumb-pc-linux-musleabihf \
// RUN: | FileCheck --check-prefix=CHECK-MUSL-ARMHF %s
// RUN: %clang %s -### -o %t.o 2>&1 \
+// RUN: --target=thumbv7-pc-linux-musleabi -mhard-float \
+// RUN: | FileCheck --check-prefix=CHECK-MUSL-ARMHF %s
+// RUN: %clang %s -### -o %t.o 2>&1 \
// RUN: --target=thumbeb-pc-linux-musleabi \
// RUN: | FileCheck --check-prefix=CHECK-MUSL-ARMEB %s
// RUN: %clang %s -### -o %t.o 2>&1 \
// RUN: --target=thumbeb-pc-linux-musleabihf \
// RUN: | FileCheck --check-prefix=CHECK-MUSL-ARMEBHF %s
// RUN: %clang %s -### -o %t.o 2>&1 \
+// RUN: --target=thumbv7eb-pc-linux-musleabi -mhard-float \
+// RUN: | FileCheck --check-prefix=CHECK-MUSL-ARMEBHF %s
+// RUN: %clang %s -### -o %t.o 2>&1 \
// RUN: --target=arm-pc-linux-musleabi \
// RUN: | FileCheck --check-prefix=CHECK-MUSL-ARM %s
// RUN: %clang %s -### -o %t.o 2>&1 \
// RUN: --target=arm-pc-linux-musleabihf \
// RUN: | FileCheck --check-prefix=CHECK-MUSL-ARMHF %s
// RUN: %clang %s -### -o %t.o 2>&1 \
+// RUN: --target=armv7-pc-linux-musleabi -mhard-float \
+// RUN: | FileCheck --check-prefix=CHECK-MUSL-ARMHF %s
+// RUN: %clang %s -### -o %t.o 2>&1 \
// RUN: --target=armeb-pc-linux-musleabi \
// RUN: | FileCheck --check-prefix=CHECK-MUSL-ARMEB %s
// RUN: %clang %s -### -o %t.o 2>&1 \
// RUN: --target=armeb-pc-linux-musleabihf \
// RUN: | FileCheck --check-prefix=CHECK-MUSL-ARMEBHF %s
// RUN: %clang %s -### -o %t.o 2>&1 \
+// RUN: --target=armv7eb-pc-linux-musleabi -mhard-float \
+// RUN: | FileCheck --check-prefix=CHECK-MUSL-ARMEBHF %s
+// RUN: %clang %s -### -o %t.o 2>&1 \
// RUN: --target=aarch64-pc-linux-musleabi \
// RUN: | FileCheck --check-prefix=CHECK-MUSL-AARCH64 %s
// RUN: %clang %s -### -o %t.o 2>&1 \
Index: cfe/trunk/lib/Driver/ToolChains.cpp
===
--- cfe/trunk/lib/Driver/ToolChains.cpp
+++ cfe/trunk/lib/Driver/ToolChains.cpp
@@ -4274,19 +4274,28 @@
if (Triple.isAndroid())
return Triple.isArch64Bit() ? "/system/bin/linker64" :
"/system/bin/linker";
- else if (Triple.isMusl()) {
+
+ if (Triple.isMusl()) {
std::string ArchName;
+bool IsArm = false;
+
switch (Arch) {
+case llvm::Triple::arm:
case llvm::Triple::thumb:
ArchName = "arm";
+ IsArm = true;
break;
+case llvm::Triple::armeb:
case llvm::Triple::thumbeb:
ArchName = "armeb";
+ IsArm = true;
break;
default:
ArchName = Triple.getArchName().str();
}
-if (Triple.getEnvironment() == llvm::Triple::MuslEABIHF)
+if (IsArm &&
+(Triple.getEnvironment() == llvm::Triple::MuslEABIHF ||
+ tools::arm::getARMFloatABI(*this, Args) ==
tools::arm::FloatABI::Hard))
ArchName += "hf";
return "/lib/ld-musl-" + ArchName + ".so.1";
Index: cfe/trunk/test/Driver/linux-ld.c
===
--- cfe/trunk/test/Driver/linux-ld.c
+++ cfe/trunk/test/Driver/linux-ld.c
@@ -1613,24 +1613,36 @@
// RUN: --target=thumb-pc-linux-musleabihf \
// RUN: | FileCheck --check-prefix=CHECK-MUSL-ARMHF %s
// RUN: %clang %s -### -o %t.o 2>&1 \
+// RUN: --target=thumbv7-pc-linux-musleabi -mhard-float \
+// RUN: | FileCheck --check-prefix=CHECK-MUSL-ARMHF %s
+// RUN: %clang %s -### -o %t.o 2>&1 \
// RUN: --target=thumbeb-pc-linux-musleabi \
// RUN: | FileCheck --check-prefix=CHECK-MUSL-ARMEB %s
// RUN: %clang %s -### -o %t.o 2>&1 \
// RUN: --target=thumbeb-pc-linux-musleabihf \
// RUN: | FileCheck --check-prefix=CHECK-MUSL-ARMEBHF %s
// RUN: %clang %s -### -o %t.o 2>&1 \
+// RUN: --target=thumbv7eb-pc-linux-musleabi -mhard-float \
+// RUN: | FileCheck --check-prefix=CHECK-MUSL-ARMEBHF %s
+// RUN: %clang %s -### -o %t.o 2>&1 \
// RUN: --target=arm-pc-linux-musleabi \
// RUN: | FileCheck --check-prefix=CHECK-MUSL-ARM %s
// RUN: %clang %s -### -o %t.o 2>&1 \
// RUN: --target=arm-pc-linux-musleabihf \
// RUN: | FileCheck --check-prefix=CHECK-MUSL-ARMHF %s
// RUN: %clang %s -### -o %t.o 2>&1 \
+// RUN: --target=armv7-pc-linux-musleabi -mhard-float \
+// RUN: | FileCheck --check-prefix=CHECK-MUSL-ARMHF %s
+// RUN: %clang %s -### -o %t.o 2>&1 \
// RUN: --target=armeb-pc-linux-musleabi \
// RUN: | FileCheck --check-prefix=CHECK-MUSL-A
Re: [PATCH] D22904: Fix two bugs for musl-libc on ARM
rovka added a subscriber: rovka. rovka added a comment. Hi Lei, Renato is on vacation, so I committed this for you. Regards, Diana Repository: rL LLVM https://reviews.llvm.org/D22904 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23112: [analyzer] Correctly add assumptions based on array bounds.
xazax.hun updated this revision to Diff 67135.
xazax.hun added a comment.
- Address review comments.
https://reviews.llvm.org/D23112
Files:
lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
test/Analysis/out-of-bounds.c
Index: test/Analysis/out-of-bounds.c
===
--- test/Analysis/out-of-bounds.c
+++ test/Analysis/out-of-bounds.c
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -Wno-array-bounds -analyze
-analyzer-checker=core,alpha.security.ArrayBoundV2 -verify %s
+// RUN: %clang_cc1 -Wno-array-bounds -analyze
-analyzer-checker=core,alpha.security.ArrayBoundV2,debug.ExprInspection -verify
%s
+
+void clang_analyzer_eval(int);
// Tests doing an out-of-bounds access after the end of an array using:
// - constant integer index
@@ -146,6 +148,14 @@
buf[x] = 1;
}
+// *** FIXME ***
+// The result is unknown for the same reason as above.
+void test_asume_after_access(unsigned long x) {
+ int buf[100];
+ buf[x] = 1;
+ clang_analyzer_eval(x <= 99); // expected-warning{{UNKNOWN}}
+}
+
// Don't warn when indexing below the start of a symbolic region's whose
// base extent we don't know.
int *get_symbolic();
@@ -166,3 +176,9 @@
p[1] = 42; // no-warning
}
+void test_asume_after_access2(unsigned long x) {
+ char buf[100];
+ buf[x] = 1;
+ clang_analyzer_eval(x <= 99); // expected-warning{{TRUE}}
+}
+
Index: lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
===
--- lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -157,13 +157,13 @@
// If we are under constrained and the index variables are tainted, report.
if (state_exceedsUpperBound && state_withinUpperBound) {
- if (state->isTainted(rawOffset.getByteOffset()))
+ if (state->isTainted(rawOffset.getByteOffset())) {
reportOOB(checkerContext, state_exceedsUpperBound, OOB_Tainted);
return;
-}
-
-// If we are constrained enough to definitely exceed the upper bound,
report.
-if (state_exceedsUpperBound) {
+ }
+} else if (state_exceedsUpperBound) {
+ // If we are constrained enough to definitely exceed the upper bound,
+ // report.
assert(!state_withinUpperBound);
reportOOB(checkerContext, state_exceedsUpperBound, OOB_Excedes);
return;
Index: test/Analysis/out-of-bounds.c
===
--- test/Analysis/out-of-bounds.c
+++ test/Analysis/out-of-bounds.c
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -Wno-array-bounds -analyze -analyzer-checker=core,alpha.security.ArrayBoundV2 -verify %s
+// RUN: %clang_cc1 -Wno-array-bounds -analyze -analyzer-checker=core,alpha.security.ArrayBoundV2,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
// Tests doing an out-of-bounds access after the end of an array using:
// - constant integer index
@@ -146,6 +148,14 @@
buf[x] = 1;
}
+// *** FIXME ***
+// The result is unknown for the same reason as above.
+void test_asume_after_access(unsigned long x) {
+ int buf[100];
+ buf[x] = 1;
+ clang_analyzer_eval(x <= 99); // expected-warning{{UNKNOWN}}
+}
+
// Don't warn when indexing below the start of a symbolic region's whose
// base extent we don't know.
int *get_symbolic();
@@ -166,3 +176,9 @@
p[1] = 42; // no-warning
}
+void test_asume_after_access2(unsigned long x) {
+ char buf[100];
+ buf[x] = 1;
+ clang_analyzer_eval(x <= 99); // expected-warning{{TRUE}}
+}
+
Index: lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
===
--- lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -157,13 +157,13 @@
// If we are under constrained and the index variables are tainted, report.
if (state_exceedsUpperBound && state_withinUpperBound) {
- if (state->isTainted(rawOffset.getByteOffset()))
+ if (state->isTainted(rawOffset.getByteOffset())) {
reportOOB(checkerContext, state_exceedsUpperBound, OOB_Tainted);
return;
-}
-
-// If we are constrained enough to definitely exceed the upper bound, report.
-if (state_exceedsUpperBound) {
+ }
+} else if (state_exceedsUpperBound) {
+ // If we are constrained enough to definitely exceed the upper bound,
+ // report.
assert(!state_withinUpperBound);
reportOOB(checkerContext, state_exceedsUpperBound, OOB_Excedes);
return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23112: [analyzer] Correctly add assumptions based on array bounds.
xazax.hun added a comment. I am not sure that the checker is the appropriate way to fix the remaining issue with this checker. I think generating simpler constraints without loosing any generality is non trivial. https://reviews.llvm.org/D23112 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23014: [analyzer] Model base to derived casts more precisely.
xazax.hun added a comment. What do you think about escaping pointers that gone through conservatively evaluated casts? https://reviews.llvm.org/D23014 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r277989 - [analyzer] Model base to derived casts more precisely.
Author: xazax
Date: Mon Aug 8 04:22:59 2016
New Revision: 277989
URL: http://llvm.org/viewvc/llvm-project?rev=277989&view=rev
Log:
[analyzer] Model base to derived casts more precisely.
Dynamic casts are handled relatively well by the static analyzer.
BaseToDerived casts however are treated conservatively. This can cause some
false positives with the NewDeleteLeaks checker.
This patch alters the behavior of BaseToDerived casts. In case a dynamic cast
would succeed use the same semantics. Otherwise fall back to the conservative
approach.
Differential Revision: https://reviews.llvm.org/D23014
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp
cfe/trunk/test/Analysis/NewDelete-checker-test.cpp
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h?rev=277989&r1=277988&r2=277989&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h Mon Aug
8 04:22:59 2016
@@ -123,15 +123,18 @@ public:
SVal evalDerivedToBase(SVal Derived, QualType DerivedPtrType,
bool IsVirtual);
- /// \brief Evaluates C++ dynamic_cast cast.
+ /// \brief Attempts to do a down cast. Used to model BaseToDerived and C++
+ ///dynamic_cast.
/// The callback may result in the following 3 scenarios:
/// - Successful cast (ex: derived is subclass of base).
/// - Failed cast (ex: derived is definitely not a subclass of base).
+ ///The distinction of this case from the next one is necessary to model
+ ///dynamic_cast.
/// - We don't know (base is a symbolic region and we don't have
///enough info to determine if the cast will succeed at run time).
/// The function returns an SVal representing the derived class; it's
/// valid only if Failed flag is set to false.
- SVal evalDynamicCast(SVal Base, QualType DerivedPtrType, bool &Failed);
+ SVal attemptDownCast(SVal Base, QualType DerivedPtrType, bool &Failed);
const ElementRegion *GetElementZeroRegion(const MemRegion *R, QualType T);
Modified: cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp?rev=277989&r1=277988&r2=277989&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp Mon Aug 8 04:22:59 2016
@@ -552,7 +552,7 @@ void CXXInstanceCall::getInitialStackFra
// FIXME: CallEvent maybe shouldn't be directly accessing StoreManager.
bool Failed;
- ThisVal = StateMgr.getStoreManager().evalDynamicCast(ThisVal, Ty,
Failed);
+ ThisVal = StateMgr.getStoreManager().attemptDownCast(ThisVal, Ty,
Failed);
assert(!Failed && "Calling an incorrectly devirtualized method");
}
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp?rev=277989&r1=277988&r2=277989&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp Mon Aug 8 04:22:59 2016
@@ -386,7 +386,7 @@ void ExprEngine::VisitCast(const CastExp
Failed = true;
// Else, evaluate the cast.
else
- val = getStoreManager().evalDynamicCast(val, T, Failed);
+ val = getStoreManager().attemptDownCast(val, T, Failed);
if (Failed) {
if (T->isReferenceType()) {
@@ -412,6 +412,28 @@ void ExprEngine::VisitCast(const CastExp
Bldr.generateNode(CastE, Pred, state);
continue;
}
+ case CK_BaseToDerived: {
+SVal val = state->getSVal(Ex, LCtx);
+QualType resultType = CastE->getType();
+if (CastE->isGLValue())
+ resultType = getContext().getPointerType(resultType);
+
+bool Failed = false;
+
+if (!val.isConstant()) {
+ val = getStoreManager().attemptDownCast(val, T, Failed);
+}
+
+// Failed to cast or the result is unknown, fall back to conservative.
+if (Failed || val.isUnknown()) {
+ val =
+svalBuilder.conjureSymbolVal(nullptr, CastE, LCtx, resultType,
+ currBldrCtx->blockCount());
+}
+state = state->BindExpr(CastE, LCtx, val);
+Bldr.generateNode(CastE, Pred, state);
+continue;
+ }
case CK_NullToM
Re: [PATCH] D23014: [analyzer] Model base to derived casts more precisely.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL277989: [analyzer] Model base to derived casts more
precisely. (authored by xazax).
Changed prior to commit:
https://reviews.llvm.org/D23014?vs=66632&id=67136#toc
Repository:
rL LLVM
https://reviews.llvm.org/D23014
Files:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp
cfe/trunk/test/Analysis/NewDelete-checker-test.cpp
Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
@@ -123,15 +123,18 @@
SVal evalDerivedToBase(SVal Derived, QualType DerivedPtrType,
bool IsVirtual);
- /// \brief Evaluates C++ dynamic_cast cast.
+ /// \brief Attempts to do a down cast. Used to model BaseToDerived and C++
+ ///dynamic_cast.
/// The callback may result in the following 3 scenarios:
/// - Successful cast (ex: derived is subclass of base).
/// - Failed cast (ex: derived is definitely not a subclass of base).
+ ///The distinction of this case from the next one is necessary to model
+ ///dynamic_cast.
/// - We don't know (base is a symbolic region and we don't have
///enough info to determine if the cast will succeed at run time).
/// The function returns an SVal representing the derived class; it's
/// valid only if Failed flag is set to false.
- SVal evalDynamicCast(SVal Base, QualType DerivedPtrType, bool &Failed);
+ SVal attemptDownCast(SVal Base, QualType DerivedPtrType, bool &Failed);
const ElementRegion *GetElementZeroRegion(const MemRegion *R, QualType T);
Index: cfe/trunk/test/Analysis/NewDelete-checker-test.cpp
===
--- cfe/trunk/test/Analysis/NewDelete-checker-test.cpp
+++ cfe/trunk/test/Analysis/NewDelete-checker-test.cpp
@@ -377,3 +377,19 @@
delete foo;
delete foo; // expected-warning {{Attempt to delete released memory}}
}
+
+struct Base {
+ virtual ~Base() {}
+};
+
+struct Derived : Base {
+};
+
+Base *allocate() {
+ return new Derived;
+}
+
+void shouldNotReportLeak() {
+ Derived *p = (Derived *)allocate();
+ delete p;
+}
Index: cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -552,7 +552,7 @@
// FIXME: CallEvent maybe shouldn't be directly accessing StoreManager.
bool Failed;
- ThisVal = StateMgr.getStoreManager().evalDynamicCast(ThisVal, Ty, Failed);
+ ThisVal = StateMgr.getStoreManager().attemptDownCast(ThisVal, Ty, Failed);
assert(!Failed && "Calling an incorrectly devirtualized method");
}
Index: cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp
@@ -292,7 +292,7 @@
return nullptr;
}
-SVal StoreManager::evalDynamicCast(SVal Base, QualType TargetType,
+SVal StoreManager::attemptDownCast(SVal Base, QualType TargetType,
bool &Failed) {
Failed = false;
Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -386,7 +386,7 @@
Failed = true;
// Else, evaluate the cast.
else
- val = getStoreManager().evalDynamicCast(val, T, Failed);
+ val = getStoreManager().attemptDownCast(val, T, Failed);
if (Failed) {
if (T->isReferenceType()) {
@@ -412,6 +412,28 @@
Bldr.generateNode(CastE, Pred, state);
continue;
}
+ case CK_BaseToDerived: {
+SVal val = state->getSVal(Ex, LCtx);
+QualType resultType = CastE->getType();
+if (CastE->isGLValue())
+ resultType = getContext().getPointerType(resultType);
+
+bool Failed = false;
+
+if (!val.isConstant()) {
+ val = getStoreManager().attemptDownCast(val, T, Failed);
+}
+
+// Failed to cast or the result is unknown, fall back to conservative.
+if (Failed || val.isUnknown()) {
+ val =
+svalBuilder.conjureSymbolVal(nullptr, CastE, LCtx, resultType,
+ currBldrCtx->blockCount());
+}
+state = state->BindExpr(CastE, LCtx, val);
+Bldr.generateNode(CastE, Pred, state);
+continue;
+
[PATCH] D23257: Fix clang-tidy crash when a single fix is applied on multiple files.
ioeric created this revision.
ioeric added reviewers: alexfh, hokein.
ioeric added a subscriber: cfe-commits.
tooling::Replacements only holds replacements for a single file, so
this patch makes Fix a map from file paths to tooling::Replacements so that it
can be applied on multiple files.
https://reviews.llvm.org/D23257
Files:
clang-tidy/ClangTidy.cpp
clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tidy/ClangTidyDiagnosticConsumer.h
test/clang-tidy/Inputs/modernize-pass-by-value/header-with-fix.h
test/clang-tidy/modernize-pass-by-value-multi-fixes.cpp
unittests/clang-tidy/ClangTidyTest.h
Index: unittests/clang-tidy/ClangTidyTest.h
===
--- unittests/clang-tidy/ClangTidyTest.h
+++ unittests/clang-tidy/ClangTidyTest.h
@@ -119,14 +119,15 @@
DiagConsumer.finish();
tooling::Replacements Fixes;
for (const ClangTidyError &Error : Context.getErrors())
-for (const auto &Fix : Error.Fix) {
- auto Err = Fixes.add(Fix);
- // FIXME: better error handling. Keep the behavior for now.
- if (Err) {
-llvm::errs() << llvm::toString(std::move(Err)) << "\n";
-return "";
+for (const auto &FileAndFixes : Error.Fix)
+ for (const auto &Fix : FileAndFixes.second) {
+auto Err = Fixes.add(Fix);
+// FIXME: better error handling. Keep the behavior for now.
+if (Err) {
+ llvm::errs() << llvm::toString(std::move(Err)) << "\n";
+ return "";
+}
}
-}
if (Errors)
*Errors = Context.getErrors();
auto Result = tooling::applyAllReplacements(Code, Fixes);
Index: test/clang-tidy/modernize-pass-by-value-multi-fixes.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-pass-by-value-multi-fixes.cpp
@@ -0,0 +1,12 @@
+// RUN: cp %S/Inputs/modernize-pass-by-value/header-with-fix.h %T/pass-by-value-header-with-fix.h
+// RUN: sed -e 's#//.*$##' %s > %t.cpp
+// RUN: clang-tidy %t.cpp -checks='-*,modernize-pass-by-value' -header-filter='.*' -fix -- -std=c++11 -I %T | FileCheck %s -check-prefix=CHECK-MESSAGES -implicit-check-not="{{warning|error}}:"
+// RUN: FileCheck -input-file=%t.cpp %s -check-prefix=CHECK-FIXES
+// RUN: FileCheck -input-file=%T/pass-by-value-header-with-fix.h %s -check-prefix=CHECK-HEADER-FIXES
+
+#include "pass-by-value-header-with-fix.h"
+// CHECK-HEADER-FIXES: Foo(std::string s);
+Foo::Foo(const std::string &s) : s(s) {}
+// CHECK-MESSAGES: :9:10: warning: pass by value and use std::move [modernize-pass-by-value]
+// CHECK-FIXES: #include
+// CHECK-FIXES: Foo::Foo(std::string s) : s(std::move(s)) {}
Index: test/clang-tidy/Inputs/modernize-pass-by-value/header-with-fix.h
===
--- /dev/null
+++ test/clang-tidy/Inputs/modernize-pass-by-value/header-with-fix.h
@@ -0,0 +1,6 @@
+#include
+
+struct Foo {
+ Foo(const std::string &s);
+ std::string s;
+};
Index: clang-tidy/ClangTidyDiagnosticConsumer.h
===
--- clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -62,7 +62,8 @@
std::string CheckName;
ClangTidyMessage Message;
- tooling::Replacements Fix;
+ // Fixes grouped by file path.
+ std::map Fix;
SmallVector Notes;
// A build directory of the diagnostic source file.
Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -77,8 +77,8 @@
assert(Range.getBegin().isFileID() && Range.getEnd().isFileID() &&
"Only file locations supported in fix-it hints.");
- auto Err =
- Error.Fix.add(tooling::Replacement(SM, Range, FixIt.CodeToInsert));
+ tooling::Replacement replace(SM, Range, FixIt.CodeToInsert);
+ auto Err = Error.Fix[replace.getFilePath()].add(replace);
// FIXME: better error handling.
if (Err) {
llvm::errs() << "Fix conflicts with existing fix! "
@@ -495,27 +495,28 @@
std::vector Sizes;
for (const auto &Error : Errors) {
int Size = 0;
-for (const auto &Replace : Error.Fix)
- Size += Replace.getLength();
+for (const auto &FileAndReplaces : Error.Fix)
+ for (const auto &Replace : FileAndReplaces.second)
+Size += Replace.getLength();
Sizes.push_back(Size);
}
// Build events from error intervals.
std::map> FileEvents;
- for (unsigned I = 0; I < Errors.size(); ++I) {
-for (const auto &Replace : Errors[I].Fix) {
- unsigned Begin = Replace.getOffset();
- unsigned End = Begin + Replace.getLength();
- const std::string &FilePath = Replace.getFilePath();
- // FIXME: Handle empty intervals, such as those from insertions.
- if (Begin == End)
-continue;
- FileEvents[FilePath].push_back
Re: [PATCH] D23086: [OpenCL] Generate concrete struct type for ndrange_t
Anastasia added a comment. In https://reviews.llvm.org/D23086#507360, @yaxunl wrote: > In https://reviews.llvm.org/D23086#507203, @yaxunl wrote: > > > How about assuming ndrange_t is a struct type defined by user and identify > > it by struct type name in Clang? This gives user freedom of implementing it > > differently than SPIR. In opencl-c.h define it as a struct type as SPIR > > required. > > > Anastasia, are you OK with this approach? Thanks. Sure. It seems there is no better solution to this. Repository: rL LLVM https://reviews.llvm.org/D23086 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23257: Fix clang-tidy crash when a single fix is applied on multiple files.
hokein added inline comments. Comment at: clang-tidy/ClangTidyDiagnosticConsumer.h:66 @@ -66,1 +65,3 @@ + // Fixes grouped by file path. + std::map Fix; SmallVector Notes; Use llvm::StringMap here? https://reviews.llvm.org/D23257 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector values
vbyakovl set the repository for this revision to rL LLVM.
vbyakovl updated this revision to Diff 67140.
Repository:
rL LLVM
https://reviews.llvm.org/D21678
Files:
llvm/tools/clang/lib/Sema/SemaExpr.cpp
llvm/tools/clang/test/Sema/shift.c
Index: llvm/tools/clang/lib/Sema/SemaExpr.cpp
===
--- llvm/tools/clang/lib/Sema/SemaExpr.cpp
+++ llvm/tools/clang/lib/Sema/SemaExpr.cpp
@@ -8592,11 +8592,10 @@
<< RHS.get()->getSourceRange();
}
-/// \brief Return the resulting type when an OpenCL vector is shifted
+/// \brief Return the resulting type when a vector is shifted
///by a scalar or vector shift amount.
-static QualType checkOpenCLVectorShift(Sema &S,
- ExprResult &LHS, ExprResult &RHS,
- SourceLocation Loc, bool IsCompAssign) {
+static QualType checkVectorShift(Sema &S, ExprResult &LHS, ExprResult &RHS,
+ SourceLocation Loc, bool IsCompAssign) {
// OpenCL v1.1 s6.3.j says RHS can be a vector only if LHS is a vector.
if (!LHS.get()->getType()->isVectorType()) {
S.Diag(Loc, diag::err_shift_rhs_only_vector)
@@ -8664,23 +8663,18 @@
// Vector shifts promote their scalar inputs to vector type.
if (LHS.get()->getType()->isVectorType() ||
RHS.get()->getType()->isVectorType()) {
-if (LangOpts.OpenCL)
- return checkOpenCLVectorShift(*this, LHS, RHS, Loc, IsCompAssign);
if (LangOpts.ZVector) {
// The shift operators for the z vector extensions work basically
- // like OpenCL shifts, except that neither the LHS nor the RHS is
+ // like general shifts, except that neither the LHS nor the RHS is
// allowed to be a "vector bool".
if (auto LHSVecType = LHS.get()->getType()->getAs())
if (LHSVecType->getVectorKind() == VectorType::AltiVecBool)
return InvalidOperands(Loc, LHS, RHS);
if (auto RHSVecType = RHS.get()->getType()->getAs())
if (RHSVecType->getVectorKind() == VectorType::AltiVecBool)
return InvalidOperands(Loc, LHS, RHS);
- return checkOpenCLVectorShift(*this, LHS, RHS, Loc, IsCompAssign);
}
-return CheckVectorOperands(LHS, RHS, Loc, IsCompAssign,
- /*AllowBothBool*/true,
- /*AllowBoolConversions*/false);
+return checkVectorShift(*this, LHS, RHS, Loc, IsCompAssign);
}
// Shifts don't perform usual arithmetic conversions, they just do integer
Index: llvm/tools/clang/test/Sema/shift.c
===
--- llvm/tools/clang/test/Sema/shift.c
+++ llvm/tools/clang/test/Sema/shift.c
@@ -67,3 +67,14 @@
(void) (x >> 80); // no-warning
(void) (x >> 80); // expected-warning {{shift count >= width of type}}
}
+
+typedef unsigned vec16 __attribute__((vector_size(16)));
+typedef unsigned vec8 __attribute__((vector_size(8)));
+
+void vect_shift_1(vec16 *x) { *x = *x << 4; }
+
+void vect_shift_2(vec16 *x, vec16 y) { *x = *x << y; }
+
+void vect_shift_3(vec16 *x, vec8 y) {
+ *x = *x << y; // expected-error {{vector operands do not have the same
number of elements}}
+}
Index: llvm/tools/clang/lib/Sema/SemaExpr.cpp
===
--- llvm/tools/clang/lib/Sema/SemaExpr.cpp
+++ llvm/tools/clang/lib/Sema/SemaExpr.cpp
@@ -8592,11 +8592,10 @@
<< RHS.get()->getSourceRange();
}
-/// \brief Return the resulting type when an OpenCL vector is shifted
+/// \brief Return the resulting type when a vector is shifted
///by a scalar or vector shift amount.
-static QualType checkOpenCLVectorShift(Sema &S,
- ExprResult &LHS, ExprResult &RHS,
- SourceLocation Loc, bool IsCompAssign) {
+static QualType checkVectorShift(Sema &S, ExprResult &LHS, ExprResult &RHS,
+ SourceLocation Loc, bool IsCompAssign) {
// OpenCL v1.1 s6.3.j says RHS can be a vector only if LHS is a vector.
if (!LHS.get()->getType()->isVectorType()) {
S.Diag(Loc, diag::err_shift_rhs_only_vector)
@@ -8664,23 +8663,18 @@
// Vector shifts promote their scalar inputs to vector type.
if (LHS.get()->getType()->isVectorType() ||
RHS.get()->getType()->isVectorType()) {
-if (LangOpts.OpenCL)
- return checkOpenCLVectorShift(*this, LHS, RHS, Loc, IsCompAssign);
if (LangOpts.ZVector) {
// The shift operators for the z vector extensions work basically
- // like OpenCL shifts, except that neither the LHS nor the RHS is
+ // like general shifts, except that neither the LHS nor the RHS is
// allowed to be a "vector bool".
if (auto LHSVecType = LHS.get()->getType()->getAs())
if (LHSVecType->getVectorKind() == VectorType::AltiVecBool)
return InvalidOperands(Loc, LHS, RHS);
if (auto RHS
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
danielmarjamaki added inline comments.
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:43
@@ +42,3 @@
+
+static StringRef getAsString(const MatchFinder::MatchResult &Result,
+ const Expr *E) {
alexfh wrote:
> Looks like this repeats getText from clang/Tooling/FixIt.h.
Yes indeed..
https://reviews.llvm.org/D21134
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
danielmarjamaki set the repository for this revision to rL LLVM.
danielmarjamaki updated this revision to Diff 67144.
danielmarjamaki added a comment.
Fixed review comments.
Repository:
rL LLVM
https://reviews.llvm.org/D21134
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/MisplacedArrayIndexCheck.cpp
clang-tidy/readability/MisplacedArrayIndexCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-misplaced-array-index.rst
test/clang-tidy/readability-misplaced-array-index.cpp
Index: test/clang-tidy/readability-misplaced-array-index.cpp
===
--- test/clang-tidy/readability-misplaced-array-index.cpp
+++ test/clang-tidy/readability-misplaced-array-index.cpp
@@ -0,0 +1,32 @@
+// RUN: %check_clang_tidy %s readability-misplaced-array-index %t
+
+#define ABC "abc"
+
+struct XY { int *x; int *y; };
+
+void dostuff(int);
+
+void unusualSyntax(int *x, struct XY *xy)
+{
+ 10[x] = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unusual array index syntax, usually the index is inside the []
+ // CHECK-FIXES: x[10] = 0;
+
+ 10[xy->x] = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unusual array index syntax, usually the index is inside the []
+ // CHECK-FIXES: xy->x[10] = 0;
+
+ dostuff(1["abc"]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: unusual array index syntax, usually the index is inside the []
+ // CHECK-FIXES: dostuff("abc"[1]);
+
+ dostuff(1[ABC]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: unusual array index syntax, usually the index is inside the []
+ // No fixit, we don't want to replace with "abc"[1]
+}
+
+void normalSyntax(int *x)
+{
+ x[10] = 0;
+ dostuff(0[0 + ABC]);
+}
Index: docs/clang-tidy/checks/readability-misplaced-array-index.rst
===
--- docs/clang-tidy/checks/readability-misplaced-array-index.rst
+++ docs/clang-tidy/checks/readability-misplaced-array-index.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - readability-misplaced-array-index
+
+readability-misplaced-array-index
+=
+
+This check warns for unusual array index syntax.
+
+The following code has unusual array index syntax:
+
+.. code:: c++
+
+ void f(int *x, int y)
+ {
+y[x] = 0;
+ }
+
+becomes
+
+.. code:: c++
+
+ void f(int *x, int y)
+ {
+x[y] = 0;
+ }
+
+The check warns about such unusual syntax for readability reasons:
+ * There are programmers that are not familiar with this unusual syntax.
+ * It is possible that variables are mixed up.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -126,6 +126,7 @@
readability-identifier-naming
readability-implicit-bool-cast
readability-inconsistent-declaration-parameter-name
+ readability-misplaced-array-index
readability-named-parameter
readability-redundant-control-flow
readability-redundant-smartptr-get
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -75,6 +75,10 @@
This check warns about the performance overhead arising from concatenating
strings using the ``operator+``, instead of ``operator+=``.
+- New `readability-misplaced-array-index
+ `_ check
+
+ Warns when there is array index before the [] instead of inside it.
Improvements to include-fixer
-
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -19,6 +19,7 @@
#include "IdentifierNamingCheck.h"
#include "ImplicitBoolCastCheck.h"
#include "InconsistentDeclarationParameterNameCheck.h"
+#include "MisplacedArrayIndexCheck.h"
#include "NamedParameterCheck.h"
#include "RedundantControlFlowCheck.h"
#include "RedundantSmartptrGetCheck.h"
@@ -53,6 +54,8 @@
"readability-implicit-bool-cast");
CheckFactories.registerCheck(
"readability-inconsistent-declaration-parameter-name");
+CheckFactories.registerCheck(
+"readability-misplaced-array-index");
CheckFactories.registerCheck(
"readability-static-definition-in-anonymous-namespace");
CheckFactories.registerCheck(
Index: clang-tidy/readability/MisplacedArrayIndexCheck.h
===
--- clang-tidy/readability/MisplacedArrayIndexCheck.h
+++ clang-tidy/readability/MisplacedArrayIndexCheck.h
@@ -0,0 +1,36 @@
+//===--- MisplacedArrayIndexCheck.h - clang-tidy-*- C++ -*-===//
+//
+//
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
danielmarjamaki removed rL LLVM as the repository for this revision.
danielmarjamaki updated this revision to Diff 67145.
danielmarjamaki added a comment.
Cleanup my previous commit. Ran clang-format.
https://reviews.llvm.org/D21134
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/MisplacedArrayIndexCheck.cpp
clang-tidy/readability/MisplacedArrayIndexCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-misplaced-array-index.rst
test/clang-tidy/readability-misplaced-array-index.cpp
Index: test/clang-tidy/readability-misplaced-array-index.cpp
===
--- test/clang-tidy/readability-misplaced-array-index.cpp
+++ test/clang-tidy/readability-misplaced-array-index.cpp
@@ -0,0 +1,32 @@
+// RUN: %check_clang_tidy %s readability-misplaced-array-index %t
+
+#define ABC "abc"
+
+struct XY { int *x; int *y; };
+
+void dostuff(int);
+
+void unusualSyntax(int *x, struct XY *xy)
+{
+ 10[x] = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unusual array index syntax, usually the index is inside the []
+ // CHECK-FIXES: x[10] = 0;
+
+ 10[xy->x] = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unusual array index syntax, usually the index is inside the []
+ // CHECK-FIXES: xy->x[10] = 0;
+
+ dostuff(1["abc"]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: unusual array index syntax, usually the index is inside the []
+ // CHECK-FIXES: dostuff("abc"[1]);
+
+ dostuff(1[ABC]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: unusual array index syntax, usually the index is inside the []
+ // No fixit, we don't want to replace with "abc"[1]
+}
+
+void normalSyntax(int *x)
+{
+ x[10] = 0;
+ dostuff(0[0 + ABC]);
+}
Index: docs/clang-tidy/checks/readability-misplaced-array-index.rst
===
--- docs/clang-tidy/checks/readability-misplaced-array-index.rst
+++ docs/clang-tidy/checks/readability-misplaced-array-index.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - readability-misplaced-array-index
+
+readability-misplaced-array-index
+=
+
+This check warns for unusual array index syntax.
+
+The following code has unusual array index syntax:
+
+.. code:: c++
+
+ void f(int *x, int y)
+ {
+y[x] = 0;
+ }
+
+becomes
+
+.. code:: c++
+
+ void f(int *x, int y)
+ {
+x[y] = 0;
+ }
+
+The check warns about such unusual syntax for readability reasons:
+ * There are programmers that are not familiar with this unusual syntax.
+ * It is possible that variables are mixed up.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -126,6 +126,7 @@
readability-identifier-naming
readability-implicit-bool-cast
readability-inconsistent-declaration-parameter-name
+ readability-misplaced-array-index
readability-named-parameter
readability-redundant-control-flow
readability-redundant-smartptr-get
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -75,6 +75,10 @@
This check warns about the performance overhead arising from concatenating
strings using the ``operator+``, instead of ``operator+=``.
+- New `readability-misplaced-array-index
+ `_ check
+
+ Warns when there is array index before the [] instead of inside it.
Improvements to include-fixer
-
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -19,6 +19,7 @@
#include "IdentifierNamingCheck.h"
#include "ImplicitBoolCastCheck.h"
#include "InconsistentDeclarationParameterNameCheck.h"
+#include "MisplacedArrayIndexCheck.h"
#include "NamedParameterCheck.h"
#include "RedundantControlFlowCheck.h"
#include "RedundantSmartptrGetCheck.h"
@@ -53,6 +54,8 @@
"readability-implicit-bool-cast");
CheckFactories.registerCheck(
"readability-inconsistent-declaration-parameter-name");
+CheckFactories.registerCheck(
+"readability-misplaced-array-index");
CheckFactories.registerCheck(
"readability-static-definition-in-anonymous-namespace");
CheckFactories.registerCheck(
Index: clang-tidy/readability/MisplacedArrayIndexCheck.h
===
--- clang-tidy/readability/MisplacedArrayIndexCheck.h
+++ clang-tidy/readability/MisplacedArrayIndexCheck.h
@@ -0,0 +1,36 @@
+//===--- MisplacedArrayIndexCheck.h - clang-tidy-*- C++ -*-===//
+//
+
Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector values
vbyakovl added inline comments.
Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8597
@@ -8596,4 +8596,3 @@
///by a scalar or vector shift amount.
-static QualType checkOpenCLVectorShift(Sema &S,
- ExprResult &LHS, ExprResult &RHS,
- SourceLocation Loc, bool IsCompAssign) {
+static QualType checkVectorShift(Sema &S, ExprResult &LHS, ExprResult &RHS,
+ SourceLocation Loc, bool IsCompAssign) {
aaron.ballman wrote:
> Why does this drop the mention of OpenCL in the function name?
This routine has common applying rather than for OpenCL only.
Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8638
@@ -8638,4 +8637,3 @@
if (RHSVecTy) {
-// OpenCL v1.1 s6.3.j says that for vector types, the operators
-// are applied component-wise. So if RHS is a vector, then ensure
-// that the number of elements is the same as LHS...
+// For vector types, the operators are applied component-wise. So if RHS is
+// a vector, then ensure that the number of elements is the same as LHS...
aaron.ballman wrote:
> It's good to keep language references in the comments, so I would leave the
> reference in even though this is being expanded for non-OpenCL vector types
> (unless the reference is incorrect).
I'll restore the language reference.
Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8681-8683
@@ -8680,5 +8675,3 @@
}
-return CheckVectorOperands(LHS, RHS, Loc, IsCompAssign,
- /*AllowBothBool*/true,
- /*AllowBoolConversions*/false);
}
aaron.ballman wrote:
> Why is it correct to remove semantic checking for vector operands?
This routine targets for checking operands of operations like plus, mul ...,
but not shifts. The tests vect_shift_1 and vect_shift_2 (I added) are examples
which were compiled with error.
Comment at: llvm/tools/clang/test/Sema/shift.c:75
@@ +74,3 @@
+void
+vect_shift_1 (vec16 *x)
+{
aaron.ballman wrote:
> Please clang-format the test case.
Ok.
Repository:
rL LLVM
https://reviews.llvm.org/D21678
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22725: [clang-tidy] Add check 'modernize-use-algorithm'
aaron.ballman added a comment. In https://reviews.llvm.org/D22725#506058, @JDevlieghere wrote: > - Added function pointer test case > - Used placeholders for diagnostics > > I extended the matchers to include `::memcpy` and `::memset` as well > because the check otherwise does not work on my mac because the `cstring` > header that ships with Xcode is implemented as shown below. > > ``` _LIBCPP_BEGIN_NAMESPACE_STD using ::size_t; using ::memcpy; using > ::memset; ... _LIBCPP_END_NAMESPACE_STD ``` You should add a similar approach as a test case to ensure we're covering this usage. > This is annoying as I have no way to discriminate functions that have the > same signature. Unless there's a better solution, this seems like a > reasonable trade-off to me. Of course I'm open to suggestions! Given that you also have to handle the case where C++ code includes string.h (rather than cstring) to call memcpy, I think this is the only approach you can really take. Either you are getting a global declaration or one in the std namespace, and both should be handled when the signatures are correct. > > > In https://reviews.llvm.org/D22725#505739, @aaron.ballman wrote: > > > The tests added mostly cover them -- I elaborated on the function pointer > > case, which I don't *think* will go wrong with this check, but should have > > a pathological test case for just to be sure. > > > I added the test case. The call is indeed replaced, which I guess is fine? Yes, that's the behavior I would expect. > > > In https://reviews.llvm.org/D22725#505476, @Prazek wrote: > > > Did you manage to see what was wrong in the transformation that you did on > > LLVM? > > > Not yet, thought it seemed to be related to `std::copy` rather than > `std::fill`. I'm still trying to pinpoint the culprit but it's extremely time > consuming as I have to recompile LLVM completely in order to run the tests... Repository: rL LLVM https://reviews.llvm.org/D22725 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23004: [ASTMatchers] Add matchers canReferToDecl() and hasUnderlyingDecl()
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Assuming others are fine with hasAnyDeclaration() vs canReferToDecl(), LGTM! If others feel strongly about canReferToDecl() instead, it still LGTM. https://reviews.llvm.org/D23004 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22955: [MSVC] Improved late parsing of template functions.
ABataev added a comment. Ping https://reviews.llvm.org/D22955 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D23264: Fixes calculateRangesAfterReplacements crash when Replacements is empty.
ioeric created this revision.
ioeric added reviewers: klimek, djasper.
ioeric added a subscriber: cfe-commits.
Herald added a subscriber: klimek.
https://reviews.llvm.org/D23264
Files:
lib/Tooling/Core/Replacement.cpp
unittests/Tooling/RefactoringTest.cpp
Index: unittests/Tooling/RefactoringTest.cpp
===
--- unittests/Tooling/RefactoringTest.cpp
+++ unittests/Tooling/RefactoringTest.cpp
@@ -506,6 +506,13 @@
EXPECT_TRUE(Ranges[1].getLength() == 22);
}
+TEST(Range, RangesAfterEmptyReplacements) {
+ std::vector Ranges = {Range(5, 6), Range(10, 5)};
+ Replacements Replaces;
+ std::vector Expected = {Range(5, 10)};
+ EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces, Ranges));
+}
+
TEST(Range, RangesAfterReplacements) {
std::vector Ranges = {Range(5, 2), Range(10, 5)};
Replacements Replaces = toReplacements({Replacement("foo", 0, 2, "1234")});
Index: lib/Tooling/Core/Replacement.cpp
===
--- lib/Tooling/Core/Replacement.cpp
+++ lib/Tooling/Core/Replacement.cpp
@@ -361,6 +361,8 @@
// - Merge with \p Replaces.
// - The new ranges will be the affected ranges of the merged replacements.
auto MergedRanges = combineAndSortRanges(Ranges);
+ if (Replaces.empty())
+return MergedRanges;
tooling::Replacements FakeReplaces;
for (const auto &R : MergedRanges) {
auto Err = FakeReplaces.add(Replacement(Replaces.begin()->getFilePath(),
Index: unittests/Tooling/RefactoringTest.cpp
===
--- unittests/Tooling/RefactoringTest.cpp
+++ unittests/Tooling/RefactoringTest.cpp
@@ -506,6 +506,13 @@
EXPECT_TRUE(Ranges[1].getLength() == 22);
}
+TEST(Range, RangesAfterEmptyReplacements) {
+ std::vector Ranges = {Range(5, 6), Range(10, 5)};
+ Replacements Replaces;
+ std::vector Expected = {Range(5, 10)};
+ EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces, Ranges));
+}
+
TEST(Range, RangesAfterReplacements) {
std::vector Ranges = {Range(5, 2), Range(10, 5)};
Replacements Replaces = toReplacements({Replacement("foo", 0, 2, "1234")});
Index: lib/Tooling/Core/Replacement.cpp
===
--- lib/Tooling/Core/Replacement.cpp
+++ lib/Tooling/Core/Replacement.cpp
@@ -361,6 +361,8 @@
// - Merge with \p Replaces.
// - The new ranges will be the affected ranges of the merged replacements.
auto MergedRanges = combineAndSortRanges(Ranges);
+ if (Replaces.empty())
+return MergedRanges;
tooling::Replacements FakeReplaces;
for (const auto &R : MergedRanges) {
auto Err = FakeReplaces.add(Replacement(Replaces.begin()->getFilePath(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23264: Fixes calculateRangesAfterReplacements crash when Replacements is empty.
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. https://reviews.llvm.org/D23264 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r278006 - [analyzer] Command line option to show enabled checker list.
Author: xazax
Date: Mon Aug 8 08:41:04 2016
New Revision: 278006
URL: http://llvm.org/viewvc/llvm-project?rev=278006&view=rev
Log:
[analyzer] Command line option to show enabled checker list.
This patch adds a command line option to list the checkers that were enabled
by analyzer-checker and not disabled by -analyzer-disable-checker.
It can be very useful to debug long command lines when it is not immediately
apparent which checkers are turned on and which checkers are turned off.
Differential Revision: https://reviews.llvm.org/D23060
Added:
cfe/trunk/test/Analysis/analyzer-enabled-checkers.c
Modified:
cfe/trunk/include/clang/Driver/CC1Options.td
cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerRegistry.h
cfe/trunk/include/clang/StaticAnalyzer/Frontend/FrontendActions.h
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
cfe/trunk/lib/StaticAnalyzer/Core/CheckerRegistry.cpp
cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
Modified: cfe/trunk/include/clang/Driver/CC1Options.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=278006&r1=278005&r2=278006&view=diff
==
--- cfe/trunk/include/clang/Driver/CC1Options.td (original)
+++ cfe/trunk/include/clang/Driver/CC1Options.td Mon Aug 8 08:41:04 2016
@@ -114,6 +114,9 @@ def analyzer_disable_all_checks : Flag<[
def analyzer_checker_help : Flag<["-"], "analyzer-checker-help">,
HelpText<"Display the list of analyzer checkers that are available">;
+def analyzer_list_enabled_checkers : Flag<["-"],
"analyzer-list-enabled-checkers">,
+ HelpText<"Display the list of enabled analyzer checkers">;
+
def analyzer_config : Separate<["-"], "analyzer-config">,
HelpText<"Choose analyzer options to enable">;
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h?rev=278006&r1=278005&r2=278006&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h Mon Aug 8
08:41:04 2016
@@ -149,6 +149,7 @@ public:
unsigned DisableAllChecks : 1;
unsigned ShowCheckerHelp : 1;
+ unsigned ShowEnabledCheckerList : 1;
unsigned AnalyzeAll : 1;
unsigned AnalyzerDisplayProgress : 1;
unsigned AnalyzeNestedBlocks : 1;
@@ -541,6 +542,7 @@ public:
AnalysisPurgeOpt(PurgeStmt),
DisableAllChecks(0),
ShowCheckerHelp(0),
+ShowEnabledCheckerList(0),
AnalyzeAll(0),
AnalyzerDisplayProgress(0),
AnalyzeNestedBlocks(0),
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerRegistry.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerRegistry.h?rev=278006&r1=278005&r2=278006&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerRegistry.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerRegistry.h Mon Aug 8
08:41:04 2016
@@ -127,7 +127,9 @@ public:
/// Prints the name and description of all checkers in this registry.
/// This output is not intended to be machine-parseable.
- void printHelp(raw_ostream &out, size_t maxNameChars = 30) const ;
+ void printHelp(raw_ostream &out, size_t maxNameChars = 30) const;
+ void printList(raw_ostream &out,
+ SmallVectorImpl &opts) const;
private:
mutable CheckerInfoList Checkers;
Modified: cfe/trunk/include/clang/StaticAnalyzer/Frontend/FrontendActions.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Frontend/FrontendActions.h?rev=278006&r1=278005&r2=278006&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Frontend/FrontendActions.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Frontend/FrontendActions.h Mon Aug
8 08:41:04 2016
@@ -17,6 +17,7 @@
namespace clang {
class Stmt;
+class AnalyzerOptions;
namespace ento {
@@ -52,6 +53,8 @@ private:
};
void printCheckerHelp(raw_ostream &OS, ArrayRef plugins);
+void printEnabledCheckerList(raw_ostream &OS, ArrayRef plugins,
+ const AnalyzerOptions &opts);
} // end GR namespace
Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=278006&r1=278005&r2=278006&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation
r278004 - Fixes calculateRangesAfterReplacements crash when Replacements is empty.
Author: ioeric
Date: Mon Aug 8 08:37:39 2016
New Revision: 278004
URL: http://llvm.org/viewvc/llvm-project?rev=278004&view=rev
Log:
Fixes calculateRangesAfterReplacements crash when Replacements is empty.
Reviewers: klimek, djasper
Subscribers: cfe-commits, klimek
Differential Revision: https://reviews.llvm.org/D23264
Modified:
cfe/trunk/lib/Tooling/Core/Replacement.cpp
cfe/trunk/unittests/Tooling/RefactoringTest.cpp
Modified: cfe/trunk/lib/Tooling/Core/Replacement.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Core/Replacement.cpp?rev=278004&r1=278003&r2=278004&view=diff
==
--- cfe/trunk/lib/Tooling/Core/Replacement.cpp (original)
+++ cfe/trunk/lib/Tooling/Core/Replacement.cpp Mon Aug 8 08:37:39 2016
@@ -361,6 +361,8 @@ calculateRangesAfterReplacements(const R
// - Merge with \p Replaces.
// - The new ranges will be the affected ranges of the merged replacements.
auto MergedRanges = combineAndSortRanges(Ranges);
+ if (Replaces.empty())
+return MergedRanges;
tooling::Replacements FakeReplaces;
for (const auto &R : MergedRanges) {
auto Err = FakeReplaces.add(Replacement(Replaces.begin()->getFilePath(),
Modified: cfe/trunk/unittests/Tooling/RefactoringTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/RefactoringTest.cpp?rev=278004&r1=278003&r2=278004&view=diff
==
--- cfe/trunk/unittests/Tooling/RefactoringTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/RefactoringTest.cpp Mon Aug 8 08:37:39 2016
@@ -506,6 +506,13 @@ TEST(Range, CalculateRangesOfReplacement
EXPECT_TRUE(Ranges[1].getLength() == 22);
}
+TEST(Range, RangesAfterEmptyReplacements) {
+ std::vector Ranges = {Range(5, 6), Range(10, 5)};
+ Replacements Replaces;
+ std::vector Expected = {Range(5, 10)};
+ EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces, Ranges));
+}
+
TEST(Range, RangesAfterReplacements) {
std::vector Ranges = {Range(5, 2), Range(10, 5)};
Replacements Replaces = toReplacements({Replacement("foo", 0, 2, "1234")});
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23264: Fixes calculateRangesAfterReplacements crash when Replacements is empty.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL278004: Fixes calculateRangesAfterReplacements crash when
Replacements is empty. (authored by ioeric).
Changed prior to commit:
https://reviews.llvm.org/D23264?vs=67155&id=67156#toc
Repository:
rL LLVM
https://reviews.llvm.org/D23264
Files:
cfe/trunk/lib/Tooling/Core/Replacement.cpp
cfe/trunk/unittests/Tooling/RefactoringTest.cpp
Index: cfe/trunk/lib/Tooling/Core/Replacement.cpp
===
--- cfe/trunk/lib/Tooling/Core/Replacement.cpp
+++ cfe/trunk/lib/Tooling/Core/Replacement.cpp
@@ -361,6 +361,8 @@
// - Merge with \p Replaces.
// - The new ranges will be the affected ranges of the merged replacements.
auto MergedRanges = combineAndSortRanges(Ranges);
+ if (Replaces.empty())
+return MergedRanges;
tooling::Replacements FakeReplaces;
for (const auto &R : MergedRanges) {
auto Err = FakeReplaces.add(Replacement(Replaces.begin()->getFilePath(),
Index: cfe/trunk/unittests/Tooling/RefactoringTest.cpp
===
--- cfe/trunk/unittests/Tooling/RefactoringTest.cpp
+++ cfe/trunk/unittests/Tooling/RefactoringTest.cpp
@@ -506,6 +506,13 @@
EXPECT_TRUE(Ranges[1].getLength() == 22);
}
+TEST(Range, RangesAfterEmptyReplacements) {
+ std::vector Ranges = {Range(5, 6), Range(10, 5)};
+ Replacements Replaces;
+ std::vector Expected = {Range(5, 10)};
+ EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces, Ranges));
+}
+
TEST(Range, RangesAfterReplacements) {
std::vector Ranges = {Range(5, 2), Range(10, 5)};
Replacements Replaces = toReplacements({Replacement("foo", 0, 2, "1234")});
Index: cfe/trunk/lib/Tooling/Core/Replacement.cpp
===
--- cfe/trunk/lib/Tooling/Core/Replacement.cpp
+++ cfe/trunk/lib/Tooling/Core/Replacement.cpp
@@ -361,6 +361,8 @@
// - Merge with \p Replaces.
// - The new ranges will be the affected ranges of the merged replacements.
auto MergedRanges = combineAndSortRanges(Ranges);
+ if (Replaces.empty())
+return MergedRanges;
tooling::Replacements FakeReplaces;
for (const auto &R : MergedRanges) {
auto Err = FakeReplaces.add(Replacement(Replaces.begin()->getFilePath(),
Index: cfe/trunk/unittests/Tooling/RefactoringTest.cpp
===
--- cfe/trunk/unittests/Tooling/RefactoringTest.cpp
+++ cfe/trunk/unittests/Tooling/RefactoringTest.cpp
@@ -506,6 +506,13 @@
EXPECT_TRUE(Ranges[1].getLength() == 22);
}
+TEST(Range, RangesAfterEmptyReplacements) {
+ std::vector Ranges = {Range(5, 6), Range(10, 5)};
+ Replacements Replaces;
+ std::vector Expected = {Range(5, 10)};
+ EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces, Ranges));
+}
+
TEST(Range, RangesAfterReplacements) {
std::vector Ranges = {Range(5, 2), Range(10, 5)};
Replacements Replaces = toReplacements({Replacement("foo", 0, 2, "1234")});
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23060: [analyzer] Show enabled checker list
This revision was automatically updated to reflect the committed changes.
Closed by commit rL278006: [analyzer] Command line option to show enabled
checker list. (authored by xazax).
Changed prior to commit:
https://reviews.llvm.org/D23060?vs=67129&id=67157#toc
Repository:
rL LLVM
https://reviews.llvm.org/D23060
Files:
cfe/trunk/include/clang/Driver/CC1Options.td
cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerRegistry.h
cfe/trunk/include/clang/StaticAnalyzer/Frontend/FrontendActions.h
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
cfe/trunk/lib/StaticAnalyzer/Core/CheckerRegistry.cpp
cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
cfe/trunk/test/Analysis/analyzer-enabled-checkers.c
Index: cfe/trunk/test/Analysis/analyzer-enabled-checkers.c
===
--- cfe/trunk/test/Analysis/analyzer-enabled-checkers.c
+++ cfe/trunk/test/Analysis/analyzer-enabled-checkers.c
@@ -0,0 +1,20 @@
+// RUN: %clang -target x86_64-apple-darwin10 --analyze %s -o /dev/null -Xclang -analyzer-checker=core -Xclang -analyzer-list-enabled-checkers > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+
+// CHECK: OVERVIEW: Clang Static Analyzer Enabled Checkers List
+// CHECK: core.CallAndMessage
+// CHECK: core.DivideZero
+// CHECK: core.DynamicTypePropagation
+// CHECK: core.NonNullParamChecker
+// CHECK: core.NullDereference
+// CHECK: core.StackAddressEscape
+// CHECK: core.UndefinedBinaryOperatorResult
+// CHECK: core.VLASize
+// CHECK: core.builtin.BuiltinFunctions
+// CHECK: core.builtin.NoReturnFunctions
+// CHECK: core.uninitialized.ArraySubscript
+// CHECK: core.uninitialized.Assign
+// CHECK: core.uninitialized.Branch
+// CHECK: core.uninitialized.CapturedBlockVariable
+// CHECK: core.uninitialized.UndefReturn
+
Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp
@@ -238,6 +238,7 @@
}
Opts.ShowCheckerHelp = Args.hasArg(OPT_analyzer_checker_help);
+ Opts.ShowEnabledCheckerList = Args.hasArg(OPT_analyzer_list_enabled_checkers);
Opts.DisableAllChecks = Args.hasArg(OPT_analyzer_disable_all_checks);
Opts.visualizeExplodedGraphWithGraphViz =
Index: cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -229,6 +229,11 @@
ento::printCheckerHelp(llvm::outs(), Clang->getFrontendOpts().Plugins);
return true;
}
+ if (Clang->getAnalyzerOpts()->ShowEnabledCheckerList) {
+ento::printEnabledCheckerList(llvm::outs(),
+ Clang->getFrontendOpts().Plugins,
+ *Clang->getAnalyzerOpts());
+ }
#endif
// If there were errors in processing arguments, don't do anything else.
Index: cfe/trunk/lib/StaticAnalyzer/Core/CheckerRegistry.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/CheckerRegistry.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/CheckerRegistry.cpp
@@ -175,3 +175,22 @@
out << '\n';
}
}
+
+void CheckerRegistry::printList(
+raw_ostream &out, SmallVectorImpl &opts) const {
+ std::sort(Checkers.begin(), Checkers.end(), checkerNameLT);
+
+ // Collect checkers enabled by the options.
+ CheckerInfoSet enabledCheckers;
+ for (SmallVectorImpl::iterator i = opts.begin(),
+ e = opts.end();
+ i != e; ++i) {
+collectCheckers(Checkers, Packages, *i, enabledCheckers);
+ }
+
+ for (CheckerInfoSet::const_iterator i = enabledCheckers.begin(),
+ e = enabledCheckers.end();
+ i != e; ++i) {
+out << (*i)->FullName << '\n';
+ }
+}
Index: cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
@@ -101,18 +101,24 @@
<< pluginAPIVersion;
}
+static SmallVector
+getCheckerOptList(const AnalyzerOptions &opts) {
+ SmallVector checkerOpts;
+ for (unsigned i = 0, e = opts.CheckersControlList.size(); i != e; ++i) {
+const std::pair &opt = opts.CheckersControlList[i];
+checkerOpts.push_back(CheckerOptInfo(opt.first.c_str(), opt.second));
+ }
+ return checkerOpts;
+}
+
std::unique_ptr
ento::createCheckerManager(AnalyzerOptions &opts, const LangOptions &langOpts,
ArrayRef plugins,
DiagnosticsEngine &diags) {
std::unique_ptr checkerMgr(
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
alexfh added a comment.
Thanks, that's better. Still a couple of comments.
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:50
@@ +49,3 @@
+ diag(ArraySubscriptE->getLocStart(),
+ "unusual array index syntax, usually the index is inside the []");
+
I'd say "confusing" instead of "unusual". This would also help avoiding the
repetition ("unusual ..., usually ..."):
"confusing array index syntax; usually, the index is inside the brackets"
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:54
@@ +53,3 @@
+ // a fixit for.
+ if (hasMacroID(ArraySubscriptE) ||
+
!Result.SourceManager->isWrittenInSameFile(ArraySubscriptE->getLocStart(),
What exactly is the recursive `hasMacroID` function trying to prevent? Do you
have a test that breaks if you just check that the start location of the
expression is not a macro?
In most cases, it's enough to check that the whole range is on the same macro
expansion level using `Lexer::makeFileCharRange` in order to prevent most
problems with macros, while allowing fixes in safe cases, e.g. replacements in
parameters of `EXPECT_*` macros from gtest.
https://reviews.llvm.org/D21134
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
aaron.ballman added a subscriber: aaron.ballman.
aaron.ballman added a reviewer: aaron.ballman.
aaron.ballman added a comment.
Is there a reason we don't want this check to be a part of the clang frontend,
rather than as a clang-tidy check?
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:27-28
@@ +26,4 @@
+ hasRHS(ignoringParenImpCasts(
+ anyOf(stringLiteral(),
declRefExpr(hasType(pointsTo(qualType(,
+memberExpr(hasType(pointsTo(qualType(
+ .bind("expr"),
Can this use `hasType(pointerType())` instead of
`hasType(pointsTo(qualType()))` ?
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:48
@@ +47,3 @@
+
+ auto D =
+ diag(ArraySubscriptE->getLocStart(),
Should not use `auto` here because the type is not spelled out in the
initialization.
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:50
@@ +49,3 @@
+ diag(ArraySubscriptE->getLocStart(),
+ "unusual array index syntax, usually the index is inside the []");
+
alexfh wrote:
> I'd say "confusing" instead of "unusual". This would also help avoiding the
> repetition ("unusual ..., usually ..."):
>
> "confusing array index syntax; usually, the index is inside the brackets"
Instead of "array index syntax", perhaps "array subscript expression"?
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:52
@@ +51,3 @@
+
+ // Don't even try to resolve macro or include contraptions. Not worth
emitting
+ // a fixit for.
What does "contraptions" mean in this context?
https://reviews.llvm.org/D21134
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23257: Fix clang-tidy crash when a single fix is applied on multiple files.
alexfh requested changes to this revision.
This revision now requires changes to proceed.
Comment at: clang-tidy/ClangTidy.cpp:129
@@ -128,22 +128,3 @@
<< Message.Message << Name;
- for (const tooling::Replacement &Fix : Error.Fix) {
-// Retrieve the source range for applicable fixes. Macro definitions
-// on the command line have locations in a virtual buffer and don't
-// have valid file paths and are therefore not applicable.
-SourceRange Range;
-SourceLocation FixLoc;
-if (Fix.isApplicable()) {
- SmallString<128> FixAbsoluteFilePath = Fix.getFilePath();
- Files.makeAbsolutePath(FixAbsoluteFilePath);
- FixLoc = getLocation(FixAbsoluteFilePath, Fix.getOffset());
- SourceLocation FixEndLoc = FixLoc.getLocWithOffset(Fix.getLength());
- Range = SourceRange(FixLoc, FixEndLoc);
- Diag << FixItHint::CreateReplacement(Range,
Fix.getReplacementText());
-}
-
-++TotalFixes;
-if (ApplyFixes) {
- bool Success = Fix.isApplicable() && Fix.apply(Rewrite);
- if (Success)
-++AppliedFixes;
- FixLocations.push_back(std::make_pair(FixLoc, Success));
+ for (const auto &FileAndFixes : Error.Fix) {
+for (const auto &Fix : FileAndFixes.second) {
s/Fixes/Replacements/
Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:80
@@ -79,3 +79,3 @@
- auto Err =
- Error.Fix.add(tooling::Replacement(SM, Range, FixIt.CodeToInsert));
+ tooling::Replacement replace(SM, Range, FixIt.CodeToInsert);
+ auto Err = Error.Fix[replace.getFilePath()].add(replace);
nit: 1. `replace` is a verb; 2. variable names should start with a capital.
`Replacement` would be better.
Comment at: test/clang-tidy/modernize-pass-by-value-multi-fixes.cpp:1
@@ +1,2 @@
+// RUN: cp %S/Inputs/modernize-pass-by-value/header-with-fix.h
%T/pass-by-value-header-with-fix.h
+// RUN: sed -e 's#//.*$##' %s > %t.cpp
I think, `cat x > y` is preferred over `cp x y` in lit tests as a more
platform-agnostic way of copying files.
https://reviews.llvm.org/D23257
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
jroelofs added a subscriber: jroelofs. jroelofs added a comment. I think the replacement is wrong for something like: int *arr; int offs1, offs2; offs1[arr + offs2] = 42; which I think would give: int *arr; int offs1, offs2; arr + offs2[offs1] = 42; If the precedence of the "indexing" argument is less than that of the subscript operator, then you need to insert parens: int *arr; int offs1, offs2; (arr + offs2)[offs1] = 42; https://reviews.llvm.org/D21134 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23193: [clang-rename] fix bug with initializer lists
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Wait, please add a test. https://reviews.llvm.org/D23193 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23193: [clang-rename] fix bug with initializer lists
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
LG with a nit.
Comment at: clang-rename/USRFinder.cpp:100
@@ +99,3 @@
+ if (const clang::FieldDecl *FieldDecl = Initializer->getMember()) {
+const auto InitBeginLoc = Initializer->getSourceLocation();
+const auto InitEndLoc = Lexer::getLocForEndOfToken(
It might be not clear from the context what `auto` means here, I'd use the
explicit type.
https://reviews.llvm.org/D23193
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D23265: [clang-tidy] enhance readability-else-after-return
omtcyfz created this revision.
omtcyfz added a reviewer: alexfh.
omtcyfz added a subscriber: cfe-commits.
`readability-else-after-return` only warns about `return` calls, but [[
http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return |
Coding Stadnards ]] mention `throw`, `continue`, `goto`, etc.
https://reviews.llvm.org/D23265
Files:
clang-tidy/readability/ElseAfterReturnCheck.cpp
test/clang-tidy/readability-else-after-return.cpp
Index: test/clang-tidy/readability-else-after-return.cpp
===
--- test/clang-tidy/readability-else-after-return.cpp
+++ test/clang-tidy/readability-else-after-return.cpp
@@ -4,15 +4,15 @@
if (a > 0)
return;
else // comment
-// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: don't use else after return
-// CHECK-FIXES: {{^}} // comment
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'else' or 'else if' after something that interrupts control flow - like return, break, throw, continue, goto, etc
+ // CHECK-FIXES: // comment
return;
if (a > 0) {
return;
} else { // comment
-// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: don't use else after return
-// CHECK-FIXES: } // comment
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' or 'else if' after something that interrupts control flow - like return, break, throw, continue, goto, etc
+ // CHECK-FIXES: // comment
return;
}
@@ -28,7 +28,43 @@
f(0);
else if (a > 10)
return;
- else
+ else // comment
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'else' or 'else if' after something that interrupts control flow - like return, break, throw, continue, goto, etc
+ // CHECK-FIXES: // comment
f(0);
}
+void foo() {
+label:
+ for (unsigned x = 0; x < 42; ++x) {
+if (x) {
+ continue;
+} else { // comment
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' or 'else if' after something that interrupts control flow - like return, break, throw, continue, goto, etc
+// CHECK-FIXES: // comment
+ x++;
+}
+if (x) {
+ break;
+} else { // comment
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' or 'else if' after something that interrupts control flow - like return, break, throw, continue, goto, etc
+// CHECK-FIXES: // comment
+ x++;
+}
+if (x) {
+ throw 42;
+} else { // comment
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' or 'else if' after something that interrupts control flow - like return, break, throw, continue, goto, etc
+// CHECK-FIXES: // comment
+ x++;
+}
+if (x) {
+ goto label;
+} else { // comment
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' or 'else if' after something that interrupts control flow - like return, break, throw, continue, goto, etc
+// CHECK-FIXES: // comment
+ x++;
+}
+
+ }
+}
Index: clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -19,20 +19,26 @@
namespace readability {
void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) {
- // FIXME: Support continue, break and throw.
Finder->addMatcher(
- compoundStmt(
- forEach(ifStmt(hasThen(stmt(anyOf(returnStmt(),
-compoundStmt(has(returnStmt()),
- hasElse(stmt().bind("else")))
- .bind("if"))),
+ stmt(forEach(
+ ifStmt(hasThen(stmt(
+ anyOf(returnStmt(), compoundStmt(has(returnStmt())),
+ continueStmt(), compoundStmt(has(continueStmt())),
+ breakStmt(), compoundStmt(has(breakStmt())),
+ gotoStmt(), compoundStmt(has(gotoStmt())),
+ cxxThrowExpr(), compoundStmt(has(cxxThrowExpr()),
+ hasElse(stmt().bind("else")))
+ .bind("if"))),
this);
}
void ElseAfterReturnCheck::check(const MatchFinder::MatchResult &Result) {
const auto *If = Result.Nodes.getNodeAs("if");
SourceLocation ElseLoc = If->getElseLoc();
- DiagnosticBuilder Diag = diag(ElseLoc, "don't use else after return");
+ StringRef DiagMessage = "do not use 'else' or 'else if' after something that "
+ "interrupts control flow - like return, break, "
+ "throw, continue, goto, etc";
+ DiagnosticBuilder Diag = diag(ElseLoc, DiagMessage);
Diag << tooling::fixit::createRemoval(ElseLoc);
// FIXME: Removing the braces isn't always safe. Do a more careful analysis.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23265: [clang-tidy] enhance readability-else-after-return
aaron.ballman added a subscriber: aaron.ballman.
aaron.ballman added a reviewer: aaron.ballman.
Comment at: clang-tidy/readability/ElseAfterReturnCheck.cpp:38-40
@@ -34,2 +37,5 @@
SourceLocation ElseLoc = If->getElseLoc();
- DiagnosticBuilder Diag = diag(ElseLoc, "don't use else after return");
+ StringRef DiagMessage = "do not use 'else' or 'else if' after something that
"
+ "interrupts control flow - like return, break, "
+ "throw, continue, goto, etc";
+ DiagnosticBuilder Diag = diag(ElseLoc, DiagMessage);
I think it might be better to specifically identify the flow control construct
used instead of a laundry list of possibilities. Then it can be "do not use
'%select{else|else if}0' after '%1'".
https://reviews.llvm.org/D23265
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23243: [clang-tidy] enhance modernize-use-bool-literals to check ternary operator
alexfh requested changes to this revision.
This revision now requires changes to proceed.
Comment at: clang-tidy/modernize/UseBoolLiteralsCheck.cpp:41
@@ +40,3 @@
+ // using anyOf(BINDLITERAL(), anything()).
+ anyOf(hasTrueExpression(ignoringParenImpCasts(
+integerLiteral().bind("trueBranchLiteral"))),
I think, Aaron was talking about `eachOf` matcher that will actually generate a
separate match for each of its submatchers.
https://reviews.llvm.org/D23243
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23257: Fix clang-tidy crash when a single fix is applied on multiple files.
ioeric updated this revision to Diff 67164.
ioeric marked 4 inline comments as done.
ioeric added a comment.
- Addressed reviewers' comments.
https://reviews.llvm.org/D23257
Files:
clang-tidy/ClangTidy.cpp
clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tidy/ClangTidyDiagnosticConsumer.h
test/clang-tidy/Inputs/modernize-pass-by-value/header-with-fix.h
test/clang-tidy/modernize-pass-by-value-multi-fixes.cpp
unittests/clang-tidy/ClangTidyTest.h
Index: unittests/clang-tidy/ClangTidyTest.h
===
--- unittests/clang-tidy/ClangTidyTest.h
+++ unittests/clang-tidy/ClangTidyTest.h
@@ -119,14 +119,15 @@
DiagConsumer.finish();
tooling::Replacements Fixes;
for (const ClangTidyError &Error : Context.getErrors())
-for (const auto &Fix : Error.Fix) {
- auto Err = Fixes.add(Fix);
- // FIXME: better error handling. Keep the behavior for now.
- if (Err) {
-llvm::errs() << llvm::toString(std::move(Err)) << "\n";
-return "";
+for (const auto &FileAndFixes : Error.Fix)
+ for (const auto &Fix : FileAndFixes.second) {
+auto Err = Fixes.add(Fix);
+// FIXME: better error handling. Keep the behavior for now.
+if (Err) {
+ llvm::errs() << llvm::toString(std::move(Err)) << "\n";
+ return "";
+}
}
-}
if (Errors)
*Errors = Context.getErrors();
auto Result = tooling::applyAllReplacements(Code, Fixes);
Index: test/clang-tidy/modernize-pass-by-value-multi-fixes.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-pass-by-value-multi-fixes.cpp
@@ -0,0 +1,12 @@
+// RUN: cat %S/Inputs/modernize-pass-by-value/header-with-fix.h > %T/pass-by-value-header-with-fix.h
+// RUN: sed -e 's#//.*$##' %s > %t.cpp
+// RUN: clang-tidy %t.cpp -checks='-*,modernize-pass-by-value' -header-filter='.*' -fix -- -std=c++11 -I %T | FileCheck %s -check-prefix=CHECK-MESSAGES -implicit-check-not="{{warning|error}}:"
+// RUN: FileCheck -input-file=%t.cpp %s -check-prefix=CHECK-FIXES
+// RUN: FileCheck -input-file=%T/pass-by-value-header-with-fix.h %s -check-prefix=CHECK-HEADER-FIXES
+
+#include "pass-by-value-header-with-fix.h"
+// CHECK-HEADER-FIXES: Foo(std::string s);
+Foo::Foo(const std::string &s) : s(s) {}
+// CHECK-MESSAGES: :9:10: warning: pass by value and use std::move [modernize-pass-by-value]
+// CHECK-FIXES: #include
+// CHECK-FIXES: Foo::Foo(std::string s) : s(std::move(s)) {}
Index: test/clang-tidy/Inputs/modernize-pass-by-value/header-with-fix.h
===
--- /dev/null
+++ test/clang-tidy/Inputs/modernize-pass-by-value/header-with-fix.h
@@ -0,0 +1,6 @@
+#include
+
+struct Foo {
+ Foo(const std::string &s);
+ std::string s;
+};
Index: clang-tidy/ClangTidyDiagnosticConsumer.h
===
--- clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -62,7 +62,8 @@
std::string CheckName;
ClangTidyMessage Message;
- tooling::Replacements Fix;
+ // Fixes grouped by file path.
+ llvm::StringMap Fix;
SmallVector Notes;
// A build directory of the diagnostic source file.
Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -77,8 +77,8 @@
assert(Range.getBegin().isFileID() && Range.getEnd().isFileID() &&
"Only file locations supported in fix-it hints.");
- auto Err =
- Error.Fix.add(tooling::Replacement(SM, Range, FixIt.CodeToInsert));
+ tooling::Replacement Replacement(SM, Range, FixIt.CodeToInsert);
+ auto Err = Error.Fix[Replacement.getFilePath()].add(Replacement);
// FIXME: better error handling.
if (Err) {
llvm::errs() << "Fix conflicts with existing fix! "
@@ -495,27 +495,28 @@
std::vector Sizes;
for (const auto &Error : Errors) {
int Size = 0;
-for (const auto &Replace : Error.Fix)
- Size += Replace.getLength();
+for (const auto &FileAndReplaces : Error.Fix)
+ for (const auto &Replace : FileAndReplaces.second)
+Size += Replace.getLength();
Sizes.push_back(Size);
}
// Build events from error intervals.
std::map> FileEvents;
- for (unsigned I = 0; I < Errors.size(); ++I) {
-for (const auto &Replace : Errors[I].Fix) {
- unsigned Begin = Replace.getOffset();
- unsigned End = Begin + Replace.getLength();
- const std::string &FilePath = Replace.getFilePath();
- // FIXME: Handle empty intervals, such as those from insertions.
- if (Begin == End)
-continue;
- FileEvents[FilePath].push_back(
- Event(Begin, End, Event::ET_Begin, I, Sizes[I]));
- FileEvents[FilePath].push_back(
- Event(Begin,
Re: [PATCH] D23243: [clang-tidy] enhance modernize-use-bool-literals to check ternary operator
aaron.ballman added inline comments.
Comment at: clang-tidy/modernize/UseBoolLiteralsCheck.cpp:41
@@ +40,3 @@
+ // using anyOf(BINDLITERAL(), anything()).
+ anyOf(hasTrueExpression(ignoringParenImpCasts(
+integerLiteral().bind("trueBranchLiteral"))),
alexfh wrote:
> I think, Aaron was talking about `eachOf` matcher that will actually generate
> a separate match for each of its submatchers.
I *knew* we had something that would do this! Thank you for the reminder, Alex.
:-)
https://reviews.llvm.org/D23243
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23193: [clang-rename] fix bug with initializer lists
omtcyfz updated this revision to Diff 67165.
omtcyfz added a comment.
Do these symbolic pointers seem reasonable?
https://reviews.llvm.org/D23193
Files:
clang-rename/USRFinder.cpp
clang-rename/USRLocFinder.cpp
test/clang-rename/Field.cpp
Index: test/clang-rename/Field.cpp
===
--- test/clang-rename/Field.cpp
+++ test/clang-rename/Field.cpp
@@ -1,14 +1,12 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=148 -new-name=Bar %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
class Baz {
int Foo; // CHECK: Bar;
+//^ first offset
public:
Baz();
};
Baz::Baz() : Foo(0) {} // CHECK: Baz::Baz() : Bar(0) {}
+// ^ second offset
-// Use grep -FUbo 'Foo' to get the correct offset of foo when changing
-// this file.
+// RUN: clang-rename -offset=18 -new-name=Bar %s -- | sed 's,//.*,,' |
FileCheck %s
+// RUN: clang-rename -offset=106 -new-name=Bar %s -- | sed 's,//.*,,' |
FileCheck %s
Index: clang-rename/USRLocFinder.cpp
===
--- clang-rename/USRLocFinder.cpp
+++ clang-rename/USRLocFinder.cpp
@@ -48,18 +48,9 @@
// Ignore implicit initializers.
continue;
}
- if (const clang::FieldDecl *FieldDecl = Initializer->getAnyMember()) {
+ if (const clang::FieldDecl *FieldDecl = Initializer->getMember()) {
if (USRSet.find(getUSRForDecl(FieldDecl)) != USRSet.end()) {
- // The initializer refers to a field that is to be renamed.
- SourceLocation Location = Initializer->getSourceLocation();
- StringRef TokenName = Lexer::getSourceText(
- CharSourceRange::getTokenRange(Location),
- Context.getSourceManager(), Context.getLangOpts());
- if (TokenName == PrevName) {
-// The token of the source location we find actually has the old
-// name.
-LocationsFound.push_back(Initializer->getSourceLocation());
- }
+ LocationsFound.push_back(Initializer->getSourceLocation());
}
}
}
Index: clang-rename/USRFinder.cpp
===
--- clang-rename/USRFinder.cpp
+++ clang-rename/USRFinder.cpp
@@ -90,6 +90,24 @@
TypeEndLoc);
}
+ bool VisitCXXConstructorDecl(clang::CXXConstructorDecl *ConstructorDecl) {
+for (auto &Initializer : ConstructorDecl->inits()) {
+ if (Initializer->getSourceOrder() == -1) {
+// Ignore implicit initializers.
+continue;
+ }
+ if (const clang::FieldDecl *FieldDecl = Initializer->getMember()) {
+const auto InitBeginLoc = Initializer->getSourceLocation();
+const auto InitEndLoc = Lexer::getLocForEndOfToken(
+InitBeginLoc, 0, Context.getSourceManager(),
Context.getLangOpts());
+if (!setResult(FieldDecl, InitBeginLoc, InitEndLoc)) {
+ return false;
+}
+ }
+}
+return true;
+ }
+
// Other:
const NamedDecl *getNamedDecl() { return Result; }
Index: test/clang-rename/Field.cpp
===
--- test/clang-rename/Field.cpp
+++ test/clang-rename/Field.cpp
@@ -1,14 +1,12 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=148 -new-name=Bar %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
class Baz {
int Foo; // CHECK: Bar;
+//^ first offset
public:
Baz();
};
Baz::Baz() : Foo(0) {} // CHECK: Baz::Baz() : Bar(0) {}
+// ^ second offset
-// Use grep -FUbo 'Foo' to get the correct offset of foo when changing
-// this file.
+// RUN: clang-rename -offset=18 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// RUN: clang-rename -offset=106 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
Index: clang-rename/USRLocFinder.cpp
===
--- clang-rename/USRLocFinder.cpp
+++ clang-rename/USRLocFinder.cpp
@@ -48,18 +48,9 @@
// Ignore implicit initializers.
continue;
}
- if (const clang::FieldDecl *FieldDecl = Initializer->getAnyMember()) {
+ if (const clang::FieldDecl *FieldDecl = Initializer->getMember()) {
if (USRSet.find(getUSRForDecl(FieldDecl)) != USRSet.end()) {
- // The initializer refers to a field that is to be renamed.
- SourceLocation Location = Initializer->getSourceLocation();
- StringRef TokenName = Lexer::getSourceText(
- CharSourceRange::getTokenRange(Location),
- Context.getSourceManager(), Context.getLangOpts());
- if (TokenName == PrevName) {
-// The token of the source location we find actually has the old
-// name.
-LocationsFound.push_back(Initializer->getSourceLocation());
- }
+ LocationsFound.push_back(Initializer->getSourceLocation())
[PATCH] D23266: [include-fixer] Support processing multiple files in one run.
hokein created this revision.
hokein added a reviewer: bkramer.
hokein added a subscriber: cfe-commits.
Previously, if we pass multiple files or a file pattern (e.g. /path/to/*.cc) to
include-fixer, include-fixer will apply all replacements to the first argument,
which probably causes crashes.
With this patch, include-fixer can process multiple files now.
Vim and Emacs integration are tested manually.
https://reviews.llvm.org/D23266
Files:
include-fixer/IncludeFixer.cpp
include-fixer/IncludeFixer.h
include-fixer/IncludeFixerContext.cpp
include-fixer/IncludeFixerContext.h
include-fixer/tool/ClangIncludeFixer.cpp
include-fixer/tool/clang-include-fixer.py
test/include-fixer/commandline_options.cpp
unittests/include-fixer/IncludeFixerTest.cpp
Index: unittests/include-fixer/IncludeFixerTest.cpp
===
--- unittests/include-fixer/IncludeFixerTest.cpp
+++ unittests/include-fixer/IncludeFixerTest.cpp
@@ -88,15 +88,15 @@
SymbolIndexMgr->addSymbolIndex(
llvm::make_unique(Symbols));
- IncludeFixerContext FixerContext;
- IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContext, "llvm");
-
+ std::vector FixerContexts;
+ IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContexts, "llvm");
std::string FakeFileName = "input.cc";
runOnCode(&Factory, Code, FakeFileName, ExtraArgs);
- if (FixerContext.getHeaderInfos().empty())
+ assert(FixerContexts.size() == 1);
+ if (FixerContexts.front().getHeaderInfos().empty())
return Code;
auto Replaces = clang::include_fixer::createIncludeFixerReplacements(
- Code, FakeFileName, FixerContext);
+ Code, FixerContexts.front());
EXPECT_TRUE(static_cast(Replaces))
<< llvm::toString(Replaces.takeError()) << "\n";
if (!Replaces)
Index: test/include-fixer/commandline_options.cpp
===
--- test/include-fixer/commandline_options.cpp
+++ test/include-fixer/commandline_options.cpp
@@ -1,9 +1,9 @@
// REQUIRES: shell
// RUN: echo "foo f;" > %t.cpp
// RUN: clang-include-fixer -db=fixed -input='foo= "foo.h","bar.h"' -output-headers %t.cpp -- | FileCheck %s
-// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"}]}' %t.cpp | FileCheck %s -check-prefix=CHECK-CODE
-// RUN: cat %t.cpp | not clang-include-fixer -stdin -insert-header='{QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"},{Header: "\"foo2.h\"", QualifiedName: "foo"}]}' %t.cpp
-// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "a:foo"},{Header: "\"foo.h\"", QualifiedName: "b:foo"}]}' %t.cpp
+// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{FilePath: %t.cpp, QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"}]}' %t.cpp | FileCheck %s -check-prefix=CHECK-CODE
+// RUN: cat %t.cpp | not clang-include-fixer -stdin -insert-header='{FilePath: %t.cpp, QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"},{Header: "\"foo2.h\"", QualifiedName: "foo"}]}' %t.cpp
+// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{FilePath: %t.cpp, QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "a:foo"},{Header: "\"foo.h\"", QualifiedName: "b:foo"}]}' %t.cpp
//
// CHECK: "HeaderInfos": [
// CHECK-NEXT: {"Header": "\"foo.h\"",
Index: include-fixer/tool/clang-include-fixer.py
===
--- include-fixer/tool/clang-include-fixer.py
+++ include-fixer/tool/clang-include-fixer.py
@@ -149,21 +149,16 @@
return
try:
-# If there is only one suggested header, insert it directly.
-if len(unique_headers) == 1 or maximum_suggested_headers == 1:
- InsertHeaderToVimBuffer({"QuerySymbolInfos": query_symbol_infos,
- "HeaderInfos": header_infos}, text)
- print "Added #include {0} for {1}.".format(unique_headers[0], symbol)
- return
-
-selected = GetUserSelection("choose a header file for {0}.".format(symbol),
-unique_headers, maximum_suggested_headers)
-selected_header_infos = [
+inserted_header_infos = header_infos
+if len(unique_headers) > 1:
+ selected = GetUserSelection(
+ "choose a header file for {0}.".format(symbol),
+ unique_headers, maximum_suggested_headers)
+ inserted_header_infos = [
header for header in header_infos if header["Header"] == selected]
Re: [PATCH] D23243: [clang-tidy] enhance modernize-use-bool-literals to check ternary operator
omtcyfz updated this revision to Diff 67169.
omtcyfz marked an inline comment as done.
omtcyfz added a comment.
Use shiny `eachOf` and bind everything to `"literal"` value.
https://reviews.llvm.org/D23243
Files:
clang-tidy/modernize/UseBoolLiteralsCheck.cpp
docs/clang-tidy/checks/modernize-use-bool-literals.rst
test/clang-tidy/modernize-use-bool-literals.cpp
Index: test/clang-tidy/modernize-use-bool-literals.cpp
===
--- test/clang-tidy/modernize-use-bool-literals.cpp
+++ test/clang-tidy/modernize-use-bool-literals.cpp
@@ -116,3 +116,33 @@
// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: {{.*}}
// CHECK-FIXES: {{^ *}}IntToTrue = true;{{$}}
}
+
+static int Value = 1;
+
+bool Function1() {
+ bool Result = Value == 1 ? 1 : 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: {{.*}}
+ // CHECK-MESSAGES: :[[@LINE-2]]:34: {{.*}}
+ // CHECK-FIXES: {{^ *}}bool Result = Value == 1 ? true : false;{{$}}
+ return Result;
+}
+
+bool Function2() {
+ return Value == 1 ? 1 : 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: {{.*}}
+ // CHECK-MESSAGES: :[[@LINE-2]]:27: {{.*}}
+ // CHECK-FIXES: {{^ *}}return Value == 1 ? true : false;{{$}}
+}
+
+void foo() {
+ bool Result;
+ Result = Value == 1 ? true : 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:32: {{.*}}
+ // CHECK-FIXES: {{^ *}}Result = Value == 1 ? true : false;{{$}}
+ Result = Value == 1 ? false : bool(0);
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: {{.*}}
+ // CHECK-FIXES: {{^ *}}Result = Value == 1 ? false : false;{{$}}
+ Result = Value == 1 ? (bool)0 : false;
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: {{.*}}
+ // CHECK-FIXES: {{^ *}}Result = Value == 1 ? false : false;{{$}}
+}
Index: docs/clang-tidy/checks/modernize-use-bool-literals.rst
===
--- docs/clang-tidy/checks/modernize-use-bool-literals.rst
+++ docs/clang-tidy/checks/modernize-use-bool-literals.rst
@@ -10,9 +10,11 @@
bool p = 1;
bool f = static_cast(1);
std::ios_base::sync_with_stdio(0);
+ bool x = p ? 1 : 0;
// transforms to
bool p = true;
bool f = true;
std::ios_base::sync_with_stdio(false);
+ bool x = p ? true : false;
Index: clang-tidy/modernize/UseBoolLiteralsCheck.cpp
===
--- clang-tidy/modernize/UseBoolLiteralsCheck.cpp
+++ clang-tidy/modernize/UseBoolLiteralsCheck.cpp
@@ -29,10 +29,31 @@
unless(isInTemplateInstantiation()),
anyOf(hasParent(explicitCastExpr().bind("cast")), anything())),
this);
+
+ Finder->addMatcher(
+ conditionalOperator(
+ hasParent(implicitCastExpr(
+ hasImplicitDestinationType(qualType(booleanType())),
+ unless(isInTemplateInstantiation(,
+ eachOf(
+ // anyOf() is not evaluating second argument if first is
evaluated
+ // as
+ // 'true'. We'd like to bind both literals if they are presents.
+ // Thus,
+ // using anyOf(BINDLITERAL(), anything()).
+ anyOf(hasTrueExpression(ignoringParenImpCasts(
+integerLiteral().bind("literal"))),
+anything()),
+ anyOf(hasFalseExpression(ignoringParenImpCasts(
+integerLiteral().bind("literal"))),
+anything(,
+ this);
}
void UseBoolLiteralsCheck::check(const MatchFinder::MatchResult &Result) {
const auto *Literal = Result.Nodes.getNodeAs("literal");
+ if (!Literal)
+return;
const auto *Cast = Result.Nodes.getNodeAs("cast");
bool LiteralBooleanValue = Literal->getValue().getBoolValue();
Index: test/clang-tidy/modernize-use-bool-literals.cpp
===
--- test/clang-tidy/modernize-use-bool-literals.cpp
+++ test/clang-tidy/modernize-use-bool-literals.cpp
@@ -116,3 +116,33 @@
// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: {{.*}}
// CHECK-FIXES: {{^ *}}IntToTrue = true;{{$}}
}
+
+static int Value = 1;
+
+bool Function1() {
+ bool Result = Value == 1 ? 1 : 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: {{.*}}
+ // CHECK-MESSAGES: :[[@LINE-2]]:34: {{.*}}
+ // CHECK-FIXES: {{^ *}}bool Result = Value == 1 ? true : false;{{$}}
+ return Result;
+}
+
+bool Function2() {
+ return Value == 1 ? 1 : 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: {{.*}}
+ // CHECK-MESSAGES: :[[@LINE-2]]:27: {{.*}}
+ // CHECK-FIXES: {{^ *}}return Value == 1 ? true : false;{{$}}
+}
+
+void foo() {
+ bool Result;
+ Result = Value == 1 ? true : 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:32: {{.*}}
+ // CHECK-FIXES: {{^ *}}Result = Value == 1 ? true : false;{{$}}
+ Result = Value == 1 ? false : bool(0);
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: {{.*}}
+ // CHECK-FIXES: {{^ *}}Result = Value == 1 ? false : false;{{$}}
+ Result = Value == 1 ? (bool)0 : false;
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: {{.*}}
+ // CHECK-FIXES: {{^ *}}Resu
Re: [PATCH] D23243: [clang-tidy] enhance modernize-use-bool-literals to check ternary operator
omtcyfz added inline comments.
Comment at: clang-tidy/modernize/UseBoolLiteralsCheck.cpp:41
@@ +40,3 @@
+ // using anyOf(BINDLITERAL(), anything()).
+ anyOf(hasTrueExpression(ignoringParenImpCasts(
+integerLiteral().bind("trueBranchLiteral"))),
aaron.ballman wrote:
> alexfh wrote:
> > I think, Aaron was talking about `eachOf` matcher that will actually
> > generate a separate match for each of its submatchers.
> I *knew* we had something that would do this! Thank you for the reminder,
> Alex. :-)
Yup. Seems like I learned about something, too :)
https://reviews.llvm.org/D23243
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23266: [include-fixer] Support processing multiple files in one run.
hokein updated this revision to Diff 67171.
hokein added a comment.
Remove unneeded header.
https://reviews.llvm.org/D23266
Files:
include-fixer/IncludeFixer.cpp
include-fixer/IncludeFixer.h
include-fixer/IncludeFixerContext.cpp
include-fixer/IncludeFixerContext.h
include-fixer/tool/ClangIncludeFixer.cpp
include-fixer/tool/clang-include-fixer.py
test/include-fixer/commandline_options.cpp
unittests/include-fixer/IncludeFixerTest.cpp
Index: unittests/include-fixer/IncludeFixerTest.cpp
===
--- unittests/include-fixer/IncludeFixerTest.cpp
+++ unittests/include-fixer/IncludeFixerTest.cpp
@@ -88,15 +88,15 @@
SymbolIndexMgr->addSymbolIndex(
llvm::make_unique(Symbols));
- IncludeFixerContext FixerContext;
- IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContext, "llvm");
-
+ std::vector FixerContexts;
+ IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContexts, "llvm");
std::string FakeFileName = "input.cc";
runOnCode(&Factory, Code, FakeFileName, ExtraArgs);
- if (FixerContext.getHeaderInfos().empty())
+ assert(FixerContexts.size() == 1);
+ if (FixerContexts.front().getHeaderInfos().empty())
return Code;
auto Replaces = clang::include_fixer::createIncludeFixerReplacements(
- Code, FakeFileName, FixerContext);
+ Code, FixerContexts.front());
EXPECT_TRUE(static_cast(Replaces))
<< llvm::toString(Replaces.takeError()) << "\n";
if (!Replaces)
Index: test/include-fixer/commandline_options.cpp
===
--- test/include-fixer/commandline_options.cpp
+++ test/include-fixer/commandline_options.cpp
@@ -1,9 +1,9 @@
// REQUIRES: shell
// RUN: echo "foo f;" > %t.cpp
// RUN: clang-include-fixer -db=fixed -input='foo= "foo.h","bar.h"' -output-headers %t.cpp -- | FileCheck %s
-// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"}]}' %t.cpp | FileCheck %s -check-prefix=CHECK-CODE
-// RUN: cat %t.cpp | not clang-include-fixer -stdin -insert-header='{QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"},{Header: "\"foo2.h\"", QualifiedName: "foo"}]}' %t.cpp
-// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "a:foo"},{Header: "\"foo.h\"", QualifiedName: "b:foo"}]}' %t.cpp
+// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{FilePath: %t.cpp, QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"}]}' %t.cpp | FileCheck %s -check-prefix=CHECK-CODE
+// RUN: cat %t.cpp | not clang-include-fixer -stdin -insert-header='{FilePath: %t.cpp, QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"},{Header: "\"foo2.h\"", QualifiedName: "foo"}]}' %t.cpp
+// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{FilePath: %t.cpp, QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "a:foo"},{Header: "\"foo.h\"", QualifiedName: "b:foo"}]}' %t.cpp
//
// CHECK: "HeaderInfos": [
// CHECK-NEXT: {"Header": "\"foo.h\"",
Index: include-fixer/tool/clang-include-fixer.py
===
--- include-fixer/tool/clang-include-fixer.py
+++ include-fixer/tool/clang-include-fixer.py
@@ -149,21 +149,16 @@
return
try:
-# If there is only one suggested header, insert it directly.
-if len(unique_headers) == 1 or maximum_suggested_headers == 1:
- InsertHeaderToVimBuffer({"QuerySymbolInfos": query_symbol_infos,
- "HeaderInfos": header_infos}, text)
- print "Added #include {0} for {1}.".format(unique_headers[0], symbol)
- return
-
-selected = GetUserSelection("choose a header file for {0}.".format(symbol),
-unique_headers, maximum_suggested_headers)
-selected_header_infos = [
+inserted_header_infos = header_infos
+if len(unique_headers) > 1:
+ selected = GetUserSelection(
+ "choose a header file for {0}.".format(symbol),
+ unique_headers, maximum_suggested_headers)
+ inserted_header_infos = [
header for header in header_infos if header["Header"] == selected]
+include_fixer_context["HeaderInfos"] = inserted_header_infos
-# Insert a selected header.
-InsertHeaderToVimBuffer({"QuerySymbolInfos": query_symbol_infos,
- "HeaderInfos": selected_header_infos}, text)
+InsertHeaderToVimBuffer(include_fixer_context, text)
prin
Re: [PATCH] D23266: [include-fixer] Support processing multiple files in one run.
hokein updated this revision to Diff 67172.
hokein added a comment.
Add missing tests.
https://reviews.llvm.org/D23266
Files:
include-fixer/IncludeFixer.cpp
include-fixer/IncludeFixer.h
include-fixer/IncludeFixerContext.cpp
include-fixer/IncludeFixerContext.h
include-fixer/tool/ClangIncludeFixer.cpp
include-fixer/tool/clang-include-fixer.py
test/include-fixer/commandline_options.cpp
test/include-fixer/multiple_fixes.cpp
unittests/include-fixer/IncludeFixerTest.cpp
Index: unittests/include-fixer/IncludeFixerTest.cpp
===
--- unittests/include-fixer/IncludeFixerTest.cpp
+++ unittests/include-fixer/IncludeFixerTest.cpp
@@ -88,15 +88,15 @@
SymbolIndexMgr->addSymbolIndex(
llvm::make_unique(Symbols));
- IncludeFixerContext FixerContext;
- IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContext, "llvm");
-
+ std::vector FixerContexts;
+ IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContexts, "llvm");
std::string FakeFileName = "input.cc";
runOnCode(&Factory, Code, FakeFileName, ExtraArgs);
- if (FixerContext.getHeaderInfos().empty())
+ assert(FixerContexts.size() == 1);
+ if (FixerContexts.front().getHeaderInfos().empty())
return Code;
auto Replaces = clang::include_fixer::createIncludeFixerReplacements(
- Code, FakeFileName, FixerContext);
+ Code, FixerContexts.front());
EXPECT_TRUE(static_cast(Replaces))
<< llvm::toString(Replaces.takeError()) << "\n";
if (!Replaces)
Index: test/include-fixer/multiple_fixes.cpp
===
--- /dev/null
+++ test/include-fixer/multiple_fixes.cpp
@@ -0,0 +1,13 @@
+// REQUIRES: shell
+// RUN: sed -e 's#//.*$##' %s > %t.cpp
+// RUN: mkdir -p %T/include-fixer/multiple-fixes
+// RUN: echo 'foo f;' > %T/include-fixer/multiple-fixes/foo.cpp
+// RUN: echo 'bar b;' > %T/include-fixer/multiple-fixes/bar.cpp
+// RUN: clang-include-fixer -db=fixed -input='foo= "foo.h";bar= "bar.h"' %T/include-fixer/multiple-fixes/*.cpp --
+// RUN: FileCheck -input-file=%T/include-fixer/multiple-fixes/bar.cpp %s -check-prefix=CHECK-BAR
+// RUN: FileCheck -input-file=%T/include-fixer/multiple-fixes/foo.cpp %s -check-prefix=CHECK-FOO
+//
+// CHECK-FOO: #include "foo.h"
+// CHECK-FOO: foo f;
+// CHECK-BAR: #include "bar.h"
+// CHECK-BAR: bar b;
Index: test/include-fixer/commandline_options.cpp
===
--- test/include-fixer/commandline_options.cpp
+++ test/include-fixer/commandline_options.cpp
@@ -1,9 +1,9 @@
// REQUIRES: shell
// RUN: echo "foo f;" > %t.cpp
// RUN: clang-include-fixer -db=fixed -input='foo= "foo.h","bar.h"' -output-headers %t.cpp -- | FileCheck %s
-// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"}]}' %t.cpp | FileCheck %s -check-prefix=CHECK-CODE
-// RUN: cat %t.cpp | not clang-include-fixer -stdin -insert-header='{QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"},{Header: "\"foo2.h\"", QualifiedName: "foo"}]}' %t.cpp
-// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "a:foo"},{Header: "\"foo.h\"", QualifiedName: "b:foo"}]}' %t.cpp
+// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{FilePath: %t.cpp, QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"}]}' %t.cpp | FileCheck %s -check-prefix=CHECK-CODE
+// RUN: cat %t.cpp | not clang-include-fixer -stdin -insert-header='{FilePath: %t.cpp, QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"},{Header: "\"foo2.h\"", QualifiedName: "foo"}]}' %t.cpp
+// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{FilePath: %t.cpp, QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "a:foo"},{Header: "\"foo.h\"", QualifiedName: "b:foo"}]}' %t.cpp
//
// CHECK: "HeaderInfos": [
// CHECK-NEXT: {"Header": "\"foo.h\"",
Index: include-fixer/tool/clang-include-fixer.py
===
--- include-fixer/tool/clang-include-fixer.py
+++ include-fixer/tool/clang-include-fixer.py
@@ -149,21 +149,16 @@
return
try:
-# If there is only one suggested header, insert it directly.
-if len(unique_headers) == 1 or maximum_suggested_headers == 1:
- InsertHeaderToVimBuffer({"QuerySymbolInfos": query_symbol_infos,
- "HeaderInfos": header_infos}, text)
- print "Added #include {0} for {1}.
Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector values
aaron.ballman added inline comments. Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8681-8683 @@ -8680,5 +8676,3 @@ } -return CheckVectorOperands(LHS, RHS, Loc, IsCompAssign, - /*AllowBothBool*/true, - /*AllowBoolConversions*/false); } Are you saying that calling `CheckVectorOperands` was always incorrect? Or that it's no longer required because it should be fully covered by `checkVectorShift`? Because the two methods do considerably different checking, and I would have expected there to be more behavioral differences in the tests by removing the call to `CheckVectorOperands` that suggests there are tests missing. Repository: rL LLVM https://reviews.llvm.org/D21678 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22725: [clang-tidy] Add check 'modernize-use-algorithm'
alexfh requested changes to this revision.
This revision now requires changes to proceed.
Comment at: clang-tidy/modernize/ModernizeTidyModule.cpp:59
@@ -57,1 +58,3 @@
"modernize-use-bool-literals");
+CheckFactories.registerCheck(
+"modernize-use-algorithm");
Entries should be sorted alphabetically.
Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:50
@@ +49,3 @@
+
+static std::string getReplacement(const StringRef Function,
+ const StringRef Arg0, const StringRef Arg1,
I don't think this function pulls its weight. With just two callers and a
trivial implementation, it seems that it's better to inline it.
Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:59-61
@@ +58,5 @@
+ IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
+ Options.get("IncludeStyle", "llvm"))) {
+
+ for (const auto &KeyValue :
+ {std::make_pair("memcpy", "std::copy"), {"memset", "std::fill"}}) {
I'm not sure this works on MSVC2013. Could someone try this before submitting?
Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:86
@@ +85,3 @@
+ // benign.
+ if (getLangOpts().CPlusPlus) {
+Inserter = llvm::make_unique(
For consistency, I'd prefer the early return style (`if (!...) return;`).
Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:102
@@ +101,3 @@
+ assert(it != Replacements.end() &&
+ "Replacements list does not match list of registered matcher names");
+ const std::string ReplacedName = it->second;
Notes are treated completely differently - each note has to be attached to a
warning.
Clang-tidy can only deal with two severity levels of diagnostics: "warning" and
"error", but it's better to let them be controlled by the user via
`-warnings-as-errors=` command-line option or the `WarningsAsErrors`
configuration file option.
Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:103
@@ +102,3 @@
+ "Replacements list does not match list of registered matcher names");
+ const std::string ReplacedName = it->second;
+
This should be a StringRef instead. No need to allocate a string.
Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:131
@@ +130,3 @@
+
+ const StringRef Arg0Text = getText(MatchedExpr->getArg(0), SM, LangOptions);
+ const StringRef Arg1Text = getText(MatchedExpr->getArg(1), SM, LangOptions);
Should `clang::tooling::fixit::getText` be used instead?
Comment at: clang-tidy/modernize/UseAlgorithmCheck.h:20
@@ +19,3 @@
+
+/// Replaces std::memcpy and std::memset with std::copy and std::fill,
+/// respectively.
Please enclose inline code snippets in backquotes.
Repository:
rL LLVM
https://reviews.llvm.org/D22725
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23265: [clang-tidy] enhance readability-else-after-return
alexfh requested changes to this revision.
This revision now requires changes to proceed.
Comment at: clang-tidy/readability/ElseAfterReturnCheck.cpp:38-40
@@ -34,2 +37,5 @@
SourceLocation ElseLoc = If->getElseLoc();
- DiagnosticBuilder Diag = diag(ElseLoc, "don't use else after return");
+ StringRef DiagMessage = "do not use 'else' or 'else if' after something that
"
+ "interrupts control flow - like return, break, "
+ "throw, continue, goto, etc";
+ DiagnosticBuilder Diag = diag(ElseLoc, DiagMessage);
aaron.ballman wrote:
> I think it might be better to specifically identify the flow control
> construct used instead of a laundry list of possibilities. Then it can be "do
> not use '%select{else|else if}0' after '%1'".
The wording is actually good. For documentation (currently,
http://clang.llvm.org/extra/clang-tidy/checks/readability-else-after-return.html
is not extremely verbose ;)).
Otherwise, I agree with Aaron: the warning should be precise to avoid confusion.
Also, no need for a separate variable to keep the message, IMO.
https://reviews.llvm.org/D23265
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23266: [include-fixer] Support processing multiple files in one run.
bkramer added a comment. Can you add a lit test for this? We should've added that earlier :| Comment at: include-fixer/IncludeFixer.h:53 @@ -51,2 +52,3 @@ /// The context that contains all information about the symbol being queried. + std::vector &Contexts; This comment is now outdated, there are multiple contexts. Comment at: include-fixer/IncludeFixerContext.h:21 @@ -20,2 +20,3 @@ -/// \brief A context for the symbol being queried. +/// \brief A context for the file being processed. It includes all query +/// information, e.g. symbols being queried in database, all header candiates. A context for **a** file being processed. Comment at: include-fixer/IncludeFixerContext.h:78 @@ -72,1 +77,3 @@ + /// \brief The absolute path to the file being processed. + std::string FilePath; Does it have to be absolute? https://reviews.llvm.org/D23266 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22782: Added 'inline' attribute to __init to inline the basic_string's constructor
I'm still (as per another similar thread) a bit concerned this is working around a compiler optimizer bug - I'd love to see a standalone example of this behavior (that adding the inline keyword to an already inline/available definition is what's causing the inlining) so we can look at what the compiler might be doing wrong (& consider fixing that as the root cause instead of having to change every instance of this problem) - preferably an example without std::string/headers/etc (so, preprocessed and reduced). But up to you folks maintaining libc++ whether this goes in of course, just my 2c. On Wed, Aug 3, 2016 at 5:23 AM Aditya Kumar via cfe-commits < cfe-commits@lists.llvm.org> wrote: > hiraditya added a comment. > > In https://reviews.llvm.org/D22782#504416, @EricWF wrote: > > > The change itself LGTM, although we probably want to inline the > forward/input iterator __init's as well. > > > > However I would like to see a small benchmark that demonstrates the > performance change. Please try and write the benchmark using Google > Benchmark. > > Some helpful links: > > > > - http://libcxx.llvm.org/docs/TestingLibcxx.html#building-benchmarks > > - http://github.com/google/benchmark > > > Sure, > We'll come up with a synthetic benchmark to expose performance > improvements. > > Thanks, > > > https://reviews.llvm.org/D22782 > > > > ___ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22725: [clang-tidy] Add check 'modernize-use-algorithm'
aaron.ballman added inline comments.
Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:59-61
@@ +58,5 @@
+ IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
+ Options.get("IncludeStyle", "llvm"))) {
+
+ for (const auto &KeyValue :
+ {std::make_pair("memcpy", "std::copy"), {"memset", "std::fill"}}) {
alexfh wrote:
> I'm not sure this works on MSVC2013. Could someone try this before submitting?
It does not work in MSVC 2013.
Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:102
@@ +101,3 @@
+ assert(it != Replacements.end() &&
+ "Replacements list does not match list of registered matcher names");
+ const std::string ReplacedName = it->second;
alexfh wrote:
> Notes are treated completely differently - each note has to be attached to a
> warning.
>
> Clang-tidy can only deal with two severity levels of diagnostics: "warning"
> and "error", but it's better to let them be controlled by the user via
> `-warnings-as-errors=` command-line option or the `WarningsAsErrors`
> configuration file option.
Drat. I am not keen on this being a warning (let alone an error) because it
really doesn't warn the user against anything bad (the type safety argument is
tenuous at best), and it's arguable whether this actually modernizes code. Do
you think there's sufficient utility to this check to warrant it being
default-enabled as part of the modernize suite? For instance, we have 368
instances of memcpy() and 171 instances of std::copy() in the LLVM code base,
which is an example of a very modern C++ code base. I'm not opposed to the
check, just worried that it will drown out diagnostics that have more impact to
the user.
Repository:
rL LLVM
https://reviews.llvm.org/D22725
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22729: MPIBufferDerefCheck for Clang-Tidy
hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D22729 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23130: Add a check for definitions in the global namespace.
This seems to have a lot of overlap with -Wmissing-prototype, really - what do you think of the overlap/distinction between the two? On Wed, Aug 3, 2016 at 1:25 PM Eugene Zelenko via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Eugene.Zelenko added a subscriber: Eugene.Zelenko. > Eugene.Zelenko added a comment. > > Please mention this check in docs/ReleaseNotes.rst (in alphabetical order). > > > https://reviews.llvm.org/D23130 > > > > ___ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [clang-tools-extra] r277677 - [clang-tidy] Inefficient string operation
On Wed, Aug 3, 2016 at 4:13 PM Alexander Kornienko via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Author: alexfh
> Date: Wed Aug 3 18:06:03 2016
> New Revision: 277677
>
> URL: http://llvm.org/viewvc/llvm-project?rev=277677&view=rev
> Log:
> [clang-tidy] Inefficient string operation
>
A more detailed commit message might be nice. The code review included some
information that might've been a good start:
"This checker is to warn about the performance overhead caused by
concatenating strings using the operator+ instead of basic_string's append
or operator+=. It is configurable. In strict mode it matches every instance
of a supposed inefficient concatenation, in non-strict mode it matches only
those which are inside a loop."
>
> Patch by Bittner Barni!
>
> Differential revision: https://reviews.llvm.org/D20196
>
> Added:
>
> clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.cpp
>
> clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.h
>
> clang-tools-extra/trunk/docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
>
> clang-tools-extra/trunk/test/clang-tidy/performance-inefficient-string-concatenation.cpp
> Modified:
> clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt
>
> clang-tools-extra/trunk/clang-tidy/performance/PerformanceTidyModule.cpp
> clang-tools-extra/trunk/docs/ReleaseNotes.rst
> clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
>
> Modified: clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt?rev=277677&r1=277676&r2=277677&view=diff
>
> ==
> --- clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt
> (original)
> +++ clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt Wed Aug
> 3 18:06:03 2016
> @@ -4,6 +4,7 @@ add_clang_library(clangTidyPerformanceMo
>FasterStringFindCheck.cpp
>ForRangeCopyCheck.cpp
>ImplicitCastInLoopCheck.cpp
> + InefficientStringConcatenationCheck.cpp
>PerformanceTidyModule.cpp
>UnnecessaryCopyInitialization.cpp
>UnnecessaryValueParamCheck.cpp
>
> Added:
> clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.cpp?rev=277677&view=auto
>
> ==
> ---
> clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.cpp
> (added)
> +++
> clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.cpp
> Wed Aug 3 18:06:03 2016
> @@ -0,0 +1,86 @@
> +//===--- InefficientStringConcatenationCheck.cpp -
> clang-tidy--===//
> +//
> +// The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
>
> +//===--===//
> +
> +#include "InefficientStringConcatenationCheck.h"
> +#include "clang/AST/ASTContext.h"
> +#include "clang/ASTMatchers/ASTMatchFinder.h"
> +
> +using namespace clang::ast_matchers;
> +
> +namespace clang {
> +namespace tidy {
> +namespace performance {
> +
> +void InefficientStringConcatenationCheck::storeOptions(
> +ClangTidyOptions::OptionMap &Opts) {
> + Options.store(Opts, "StrictMode", StrictMode);
> +}
> +
> +InefficientStringConcatenationCheck::InefficientStringConcatenationCheck(
> +StringRef Name, ClangTidyContext *Context)
> +: ClangTidyCheck(Name, Context), StrictMode(Options.get("StrictMode",
> 0)) {}
> +
> +void InefficientStringConcatenationCheck::registerMatchers(
> +MatchFinder *Finder) {
> + if (!getLangOpts().CPlusPlus)
> +return;
> +
> + const auto BasicStringType =
> + hasType(cxxRecordDecl(hasName("::std::basic_string")));
> +
> + const auto BasicStringPlusOperator = cxxOperatorCallExpr(
> + hasOverloadedOperatorName("+"),
> + hasAnyArgument(ignoringImpCasts(declRefExpr(BasicStringType;
> +
> + const auto PlusOperator =
> + cxxOperatorCallExpr(
> + hasOverloadedOperatorName("+"),
> + hasAnyArgument(ignoringImpCasts(declRefExpr(BasicStringType))),
> + hasDescendant(BasicStringPlusOperator))
> + .bind("plusOperator");
> +
> + const auto AssignOperator = cxxOperatorCallExpr(
> + hasOverloadedOperatorName("="),
> + hasArgument(0, declRefExpr(BasicStringType,
> + hasDeclaration(decl().bind("lhsStrT")))
> + .bind("lhsStr")),
> + hasArgument(1, stmt(hasDescendant(declRefExpr(
> +
> hasDeclaration(decl(equalsBoundNode("lhsStrT"))),
> + hasDescendant(BasicStringPlusO
r278016 - [ARM] Command-line options for embedded position-independent code
Author: olista01
Date: Mon Aug 8 10:28:40 2016
New Revision: 278016
URL: http://llvm.org/viewvc/llvm-project?rev=278016&view=rev
Log:
[ARM] Command-line options for embedded position-independent code
This patch (with the corresponding ARM backend patch) adds support for
some new relocation models:
* Read-only position independence (ROPI): Code and read-only data is accessed
PC-relative. The offsets between all code and RO data sections are known at
static link time.
* Read-write position independence (RWPI): Read-write data is accessed relative
to a static base register. The offsets between all writeable data sections
are known at static link time.
These two modes are independent (they specify how different objects
should be addressed), so they can be used individually or together.
These modes are intended for bare-metal systems or systems with small
real-time operating systems. They are designed to avoid the need for a
dynamic linker, the only initialisation required is setting the static
base register to an appropriate value for RWPI code.
There is one C construct not currently supported by these modes: global
variables initialised to the address of another global variable or
function, where that address is not known at static-link time. There are
a few possible ways to solve this:
* Disallow this, and require the user to write their own initialisation
function if they need variables like this.
* Emit dynamic initialisers for these variables in the compiler, called from
the .init_array section (as is currently done for C++ dynamic initialisers).
We have a patch to do this, described in my original RFC email
(http://lists.llvm.org/pipermail/llvm-dev/2015-December/093022.html), but the
feedback from that RFC thread was that this is not something that belongs in
clang.
* Use a small dynamic loader to fix up these variables, by adding the
difference between the load and execution address of the relevant section.
This would require linker co-operation to generate a table of addresses that
need fixing up.
Differential Revision: https://reviews.llvm.org/D23196
Added:
cfe/trunk/test/Driver/ropi-rwpi.c
Modified:
cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/lib/Driver/Tools.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=278016&r1=278015&r2=278016&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Mon Aug 8 10:28:40
2016
@@ -240,6 +240,11 @@ def err_test_module_file_extension_forma
def warn_drv_invoking_fallback : Warning<"falling back to %0">,
InGroup;
+def err_drv_ropi_rwpi_incompatible_with_pic : Error<
+ "embedded and GOT-based position independence are incompatible">;
+def err_drv_ropi_incompatible_with_cxx : Error<
+ "ROPI is not compatible with c++">;
+
def warn_target_unsupported_nan2008 : Warning<
"ignoring '-mnan=2008' option because the '%0' architecture does not support
it">,
InGroup;
Modified: cfe/trunk/include/clang/Driver/Options.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=278016&r1=278015&r2=278016&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Mon Aug 8 10:28:40 2016
@@ -1087,6 +1087,10 @@ def fpic : Flag<["-"], "fpic">, Group, Group;
def fpie : Flag<["-"], "fpie">, Group;
def fno_pie : Flag<["-"], "fno-pie">, Group;
+def fropi : Flag<["-"], "fropi">, Group;
+def fno_ropi : Flag<["-"], "fno-ropi">, Group;
+def frwpi : Flag<["-"], "frwpi">, Group;
+def fno_rwpi : Flag<["-"], "fno-rwpi">, Group;
def fplugin_EQ : Joined<["-"], "fplugin=">, Group,
Flags<[DriverOption]>, MetaVarName<"">,
HelpText<"Load the named plugin (dynamic shared object)">;
def fpreserve_as_comments : Flag<["-"], "fpreserve-as-comments">,
Group;
Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=278016&r1=278015&r2=278016&view=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Mon Aug 8 10:28:40 2016
@@ -525,6 +525,12 @@ void EmitAssemblyHelper::CreateTargetMac
RM = llvm::Reloc::Static;
} else if (CodeGenOpts.RelocationModel == "pic") {
RM = llvm::Reloc::PIC_;
+ } else if (CodeGenOpts.RelocationModel == "ropi") {
+RM = llvm::Reloc::ROPI;
+ } else if (CodeGenOpts.RelocationModel == "rwpi") {
+RM = llvm::Reloc::RWPI;
+ } else if (CodeGenOpts.Reloc
Re: [PATCH] D23196: [ARM] Command-line options for embedded position-independent code
This revision was automatically updated to reflect the committed changes.
Closed by commit rL278016: [ARM] Command-line options for embedded
position-independent code (authored by olista01).
Changed prior to commit:
https://reviews.llvm.org/D23196?vs=66921&id=67176#toc
Repository:
rL LLVM
https://reviews.llvm.org/D23196
Files:
cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/lib/Driver/Tools.cpp
cfe/trunk/test/Driver/ropi-rwpi.c
Index: cfe/trunk/lib/CodeGen/BackendUtil.cpp
===
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp
@@ -525,6 +525,12 @@
RM = llvm::Reloc::Static;
} else if (CodeGenOpts.RelocationModel == "pic") {
RM = llvm::Reloc::PIC_;
+ } else if (CodeGenOpts.RelocationModel == "ropi") {
+RM = llvm::Reloc::ROPI;
+ } else if (CodeGenOpts.RelocationModel == "rwpi") {
+RM = llvm::Reloc::RWPI;
+ } else if (CodeGenOpts.RelocationModel == "ropi-rwpi") {
+RM = llvm::Reloc::ROPI_RWPI;
} else {
assert(CodeGenOpts.RelocationModel == "dynamic-no-pic" &&
"Invalid PIC model!");
Index: cfe/trunk/lib/Driver/Tools.cpp
===
--- cfe/trunk/lib/Driver/Tools.cpp
+++ cfe/trunk/lib/Driver/Tools.cpp
@@ -3777,10 +3777,52 @@
return std::make_tuple(llvm::Reloc::DynamicNoPIC, PIC ? 2U : 0U, false);
}
+ bool EmbeddedPISupported;
+ switch (ToolChain.getArch()) {
+case llvm::Triple::arm:
+case llvm::Triple::armeb:
+case llvm::Triple::thumb:
+case llvm::Triple::thumbeb:
+ EmbeddedPISupported = true;
+ break;
+default:
+ EmbeddedPISupported = false;
+ break;
+ }
+
+ bool ROPI = false, RWPI = false;
+ Arg* LastROPIArg = Args.getLastArg(options::OPT_fropi, options::OPT_fno_ropi);
+ if (LastROPIArg && LastROPIArg->getOption().matches(options::OPT_fropi)) {
+if (!EmbeddedPISupported)
+ ToolChain.getDriver().Diag(diag::err_drv_unsupported_opt_for_target)
+ << LastROPIArg->getSpelling() << ToolChain.getTriple().str();
+ROPI = true;
+ }
+ Arg *LastRWPIArg = Args.getLastArg(options::OPT_frwpi, options::OPT_fno_rwpi);
+ if (LastRWPIArg && LastRWPIArg->getOption().matches(options::OPT_frwpi)) {
+if (!EmbeddedPISupported)
+ ToolChain.getDriver().Diag(diag::err_drv_unsupported_opt_for_target)
+ << LastRWPIArg->getSpelling() << ToolChain.getTriple().str();
+RWPI = true;
+ }
+
+ // ROPI and RWPI are not comaptible with PIC or PIE.
+ if ((ROPI || RWPI) && (PIC || PIE)) {
+ToolChain.getDriver().Diag(diag::err_drv_ropi_rwpi_incompatible_with_pic);
+ }
+
if (PIC)
return std::make_tuple(llvm::Reloc::PIC_, IsPICLevelTwo ? 2U : 1U, PIE);
- return std::make_tuple(llvm::Reloc::Static, 0U, false);
+ llvm::Reloc::Model RelocM = llvm::Reloc::Static;
+ if (ROPI && RWPI)
+RelocM = llvm::Reloc::ROPI_RWPI;
+ else if (ROPI)
+RelocM = llvm::Reloc::ROPI;
+ else if (RWPI)
+RelocM = llvm::Reloc::RWPI;
+
+ return std::make_tuple(RelocM, 0U, false);
}
static const char *RelocationModelName(llvm::Reloc::Model Model) {
@@ -3791,6 +3833,12 @@
return "pic";
case llvm::Reloc::DynamicNoPIC:
return "dynamic-no-pic";
+ case llvm::Reloc::ROPI:
+return "ropi";
+ case llvm::Reloc::RWPI:
+return "rwpi";
+ case llvm::Reloc::ROPI_RWPI:
+return "ropi-rwpi";
}
llvm_unreachable("Unknown Reloc::Model kind");
}
@@ -4075,6 +4123,13 @@
ParsePICArgs(getToolChain(), Triple, Args);
const char *RMName = RelocationModelName(RelocationModel);
+
+ if ((RelocationModel == llvm::Reloc::ROPI ||
+ RelocationModel == llvm::Reloc::ROPI_RWPI) &&
+ types::isCXX(Input.getType()) &&
+ !Args.hasArg(options::OPT_fallow_unsupported))
+D.Diag(diag::err_drv_ropi_incompatible_with_cxx);
+
if (RMName) {
CmdArgs.push_back("-mrelocation-model");
CmdArgs.push_back(RMName);
Index: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
@@ -240,6 +240,11 @@
def warn_drv_invoking_fallback : Warning<"falling back to %0">,
InGroup;
+def err_drv_ropi_rwpi_incompatible_with_pic : Error<
+ "embedded and GOT-based position independence are incompatible">;
+def err_drv_ropi_incompatible_with_cxx : Error<
+ "ROPI is not compatible with c++">;
+
def warn_target_unsupported_nan2008 : Warning<
"ignoring '-mnan=2008' option because the '%0' architecture does not support it">,
InGroup;
Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@
Re: [PATCH] D23130: Add a check for definitions in the global namespace.
-Wmissing-prototype only warns for functions, I want to catch classes too. Also functions in the global namespace with a prototype are still badness in some coding styles. The limitation on definitions was to cut down on false positives, the current version of the patch doesn't have that limitation but I'm pondering on putting it back as there are too many false positives. On Mon, Aug 8, 2016 at 5:34 PM, David Blaikie wrote: > This seems to have a lot of overlap with -Wmissing-prototype, really - what > do you think of the overlap/distinction between the two? > > On Wed, Aug 3, 2016 at 1:25 PM Eugene Zelenko via cfe-commits > wrote: >> >> Eugene.Zelenko added a subscriber: Eugene.Zelenko. >> Eugene.Zelenko added a comment. >> >> Please mention this check in docs/ReleaseNotes.rst (in alphabetical >> order). >> >> >> https://reviews.llvm.org/D23130 >> >> >> >> ___ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22045: [X86] Support of no_caller_saved_registers attribute (Clang part)
hjl.tools added a comment. In https://reviews.llvm.org/D22045#506996, @joerg wrote: > For what it is worth, this certainly seems to be misnamed. By nature, if it > doesn't preserve at least the stack pointer, there is no way to recover on > return, right? This is the best name we came up with and has been implemented in GCC. https://reviews.llvm.org/D22045 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
alexfh added a comment. In https://reviews.llvm.org/D21134#508524, @jroelofs wrote: > I think the replacement is wrong for something like: > > int *arr; int offs1, offs2; > offs1[arr + offs2] = 42; > > > which I think would give: > > int *arr; int offs1, offs2; > arr + offs2[offs1] = 42; > > > If the precedence of the "indexing" argument is less than that of the > subscript operator, then you need to insert parens: > > int *arr; int offs1, offs2; > (arr + offs2)[offs1] = 42; > Indeed. It might be relevant to future (current ones seem to be fine) uses of `tooling::fixit::createReplacement` as well, so should probably be taken care of in its implementation. Maybe an additional hint from the calling code is needed (`bool InsertParensIfNeeded = false`). https://reviews.llvm.org/D21134 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22045: [X86] Support of no_caller_saved_registers attribute (Clang part)
aaron.ballman added a comment. In https://reviews.llvm.org/D22045#508644, @hjl.tools wrote: > In https://reviews.llvm.org/D22045#506996, @joerg wrote: > > > For what it is worth, this certainly seems to be misnamed. By nature, if it > > doesn't preserve at least the stack pointer, there is no way to recover on > > return, right? > > > This is the best name we came up with and has been implemented in GCC. What version of GCC supports this attribute? I tried 6.1.0 and it is unknown there. https://reviews.llvm.org/D22045 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22045: [X86] Support of no_caller_saved_registers attribute (Clang part)
erichkeane added a comment. In https://reviews.llvm.org/D22045#508648, @aaron.ballman wrote: > In https://reviews.llvm.org/D22045#508644, @hjl.tools wrote: > > > In https://reviews.llvm.org/D22045#506996, @joerg wrote: > > > > > For what it is worth, this certainly seems to be misnamed. By nature, if > > > it doesn't preserve at least the stack pointer, there is no way to > > > recover on return, right? > > > > > > This is the best name we came up with and has been implemented in GCC. > > > What version of GCC supports this attribute? I tried 6.1.0 and it is unknown > there. I see reference to it in this bug against 6.0: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68661 https://reviews.llvm.org/D22045 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23257: Fix clang-tidy crash when a single fix is applied on multiple files.
hokein added inline comments. Comment at: test/clang-tidy/Inputs/modernize-pass-by-value/header-with-fix.h:1 @@ +1,2 @@ +#include + Usually test should not use STL headers as it relies on the system headers. You don't have to use std::string to reproduce the crash, IMO. https://reviews.llvm.org/D23257 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23266: [include-fixer] Support processing multiple files in one run.
hokein added a comment. > Can you add a lit test for this? We should've added that earlier :| I forgot to upload the test first time. But I have already uploaded it, see `multiple_fixes.cpp` Comment at: include-fixer/IncludeFixerContext.h:78 @@ -72,1 +77,3 @@ + /// \brief The absolute path to the file being processed. + std::string FilePath; It depends on `InFile` parameter in `clang::ASTFrontendAction::CreateASTConsumer` method. Isn't it always an absolute file path? https://reviews.llvm.org/D23266 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22862: [analyzer] Fix for PR15623: eliminate unwanted ProgramState checker data propagation.
NoQ added a comment.
Hmm. The test in `unwanted-programstate-data-propagation.c` passes on my
machine even without the patch, and the return value on the respective path is
correctly represented as `(conj_$6{void *}) != 0U`, which comes from the
`evalCast()` call in `VisitLogicalExpr()` and is the default behavior of
`evalCast()` for Loc to pointer casts. There seems to be something amiss.
https://reviews.llvm.org/D22862
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22045: [X86] Support of no_caller_saved_registers attribute (Clang part)
hjl.tools added a comment. In https://reviews.llvm.org/D22045#508648, @aaron.ballman wrote: > In https://reviews.llvm.org/D22045#508644, @hjl.tools wrote: > > > In https://reviews.llvm.org/D22045#506996, @joerg wrote: > > > > > For what it is worth, this certainly seems to be misnamed. By nature, if > > > it doesn't preserve at least the stack pointer, there is no way to > > > recover on return, right? > > > > > > This is the best name we came up with and has been implemented in GCC. > > > What version of GCC supports this attribute? I tried 6.1.0 and it is unknown > there. It is implemented in GCC 7. https://reviews.llvm.org/D22045 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
alexfh added inline comments. Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:48 @@ +47,3 @@ + + auto D = + diag(ArraySubscriptE->getLocStart(), aaron.ballman wrote: > Should not use `auto` here because the type is not spelled out in the > initialization. While in general I agree with this recommendation, `auto Diag = diag(...);` has almost become an idiom in this code. I'd not worry about obscurity of `auto` in this context, especially, if the variable is renamed to `Diag`, which will make its meaning and possible ways of using it clearer. https://reviews.llvm.org/D21134 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22045: [X86] Support of no_caller_saved_registers attribute (Clang part)
On Mon, Aug 8, 2016 at 11:50 AM, H.J Lu wrote: > hjl.tools added a comment. > > In https://reviews.llvm.org/D22045#508648, @aaron.ballman wrote: > >> In https://reviews.llvm.org/D22045#508644, @hjl.tools wrote: >> >> > In https://reviews.llvm.org/D22045#506996, @joerg wrote: >> > >> > > For what it is worth, this certainly seems to be misnamed. By nature, if >> > > it doesn't preserve at least the stack pointer, there is no way to >> > > recover on return, right? >> > >> > >> > This is the best name we came up with and has been implemented in GCC. >> >> >> What version of GCC supports this attribute? I tried 6.1.0 and it is unknown >> there. > > > It is implemented in GCC 7. Okay, that is good to know! Then the attribute should be using the GCC spelling rather than the GNU spelling. This also obviates the need for an explicit CXX11 spelling under the clang:: namespace, if our intention is for this attribute to match GCC's behavior. ~Aaron > > > https://reviews.llvm.org/D22045 > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23243: [clang-tidy] enhance modernize-use-bool-literals to check ternary operator
alexfh requested changes to this revision. This revision now requires changes to proceed. Comment at: clang-tidy/modernize/UseBoolLiteralsCheck.cpp:39 @@ +38,3 @@ + eachOf( + // anyOf() is not evaluating second argument if first is evaluated + // as Remove `anyOf` and `anything`. And the comment. https://reviews.llvm.org/D23243 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
aaron.ballman added inline comments. Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:48 @@ +47,3 @@ + + auto D = + diag(ArraySubscriptE->getLocStart(), alexfh wrote: > aaron.ballman wrote: > > Should not use `auto` here because the type is not spelled out in the > > initialization. > While in general I agree with this recommendation, `auto Diag = diag(...);` > has almost become an idiom in this code. I'd not worry about obscurity of > `auto` in this context, especially, if the variable is renamed to `Diag`, > which will make its meaning and possible ways of using it clearer. I'm okay with that approach. https://reviews.llvm.org/D21134 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23130: Add a check for definitions in the global namespace.
On Mon, Aug 8, 2016 at 8:37 AM Benjamin Kramer wrote: > -Wmissing-prototype only warns for functions, I want to catch classes > too. Ah, fair enough. Yeah, a clang-tidy check for things in the global namespace that are in a main file rather than a header could be another pivot there. > Also functions in the global namespace with a prototype are still > badness in some coding styles. *nod* makes sense - especially in C++, where, as you say, you might want to bless the extern "C" function declarations only. > The limitation on definitions was to > cut down on false positives, the current version of the patch doesn't > have that limitation but I'm pondering on putting it back as there are > too many false positives. > > On Mon, Aug 8, 2016 at 5:34 PM, David Blaikie wrote: > > This seems to have a lot of overlap with -Wmissing-prototype, really - > what > > do you think of the overlap/distinction between the two? > > > > On Wed, Aug 3, 2016 at 1:25 PM Eugene Zelenko via cfe-commits > > wrote: > >> > >> Eugene.Zelenko added a subscriber: Eugene.Zelenko. > >> Eugene.Zelenko added a comment. > >> > >> Please mention this check in docs/ReleaseNotes.rst (in alphabetical > >> order). > >> > >> > >> https://reviews.llvm.org/D23130 > >> > >> > >> > >> ___ > >> cfe-commits mailing list > >> cfe-commits@lists.llvm.org > >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23266: [include-fixer] Support processing multiple files in one run.
hokein updated this revision to Diff 67178.
hokein marked 2 inline comments as done.
hokein added a comment.
Address review comments.
https://reviews.llvm.org/D23266
Files:
include-fixer/IncludeFixer.cpp
include-fixer/IncludeFixer.h
include-fixer/IncludeFixerContext.cpp
include-fixer/IncludeFixerContext.h
include-fixer/tool/ClangIncludeFixer.cpp
include-fixer/tool/clang-include-fixer.py
test/include-fixer/commandline_options.cpp
test/include-fixer/multiple_fixes.cpp
unittests/include-fixer/IncludeFixerTest.cpp
Index: unittests/include-fixer/IncludeFixerTest.cpp
===
--- unittests/include-fixer/IncludeFixerTest.cpp
+++ unittests/include-fixer/IncludeFixerTest.cpp
@@ -88,15 +88,15 @@
SymbolIndexMgr->addSymbolIndex(
llvm::make_unique(Symbols));
- IncludeFixerContext FixerContext;
- IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContext, "llvm");
-
+ std::vector FixerContexts;
+ IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContexts, "llvm");
std::string FakeFileName = "input.cc";
runOnCode(&Factory, Code, FakeFileName, ExtraArgs);
- if (FixerContext.getHeaderInfos().empty())
+ assert(FixerContexts.size() == 1);
+ if (FixerContexts.front().getHeaderInfos().empty())
return Code;
auto Replaces = clang::include_fixer::createIncludeFixerReplacements(
- Code, FakeFileName, FixerContext);
+ Code, FixerContexts.front());
EXPECT_TRUE(static_cast(Replaces))
<< llvm::toString(Replaces.takeError()) << "\n";
if (!Replaces)
Index: test/include-fixer/multiple_fixes.cpp
===
--- /dev/null
+++ test/include-fixer/multiple_fixes.cpp
@@ -0,0 +1,13 @@
+// REQUIRES: shell
+// RUN: sed -e 's#//.*$##' %s > %t.cpp
+// RUN: mkdir -p %T/include-fixer/multiple-fixes
+// RUN: echo 'foo f;' > %T/include-fixer/multiple-fixes/foo.cpp
+// RUN: echo 'bar b;' > %T/include-fixer/multiple-fixes/bar.cpp
+// RUN: clang-include-fixer -db=fixed -input='foo= "foo.h";bar= "bar.h"' %T/include-fixer/multiple-fixes/*.cpp --
+// RUN: FileCheck -input-file=%T/include-fixer/multiple-fixes/bar.cpp %s -check-prefix=CHECK-BAR
+// RUN: FileCheck -input-file=%T/include-fixer/multiple-fixes/foo.cpp %s -check-prefix=CHECK-FOO
+//
+// CHECK-FOO: #include "foo.h"
+// CHECK-FOO: foo f;
+// CHECK-BAR: #include "bar.h"
+// CHECK-BAR: bar b;
Index: test/include-fixer/commandline_options.cpp
===
--- test/include-fixer/commandline_options.cpp
+++ test/include-fixer/commandline_options.cpp
@@ -1,9 +1,9 @@
// REQUIRES: shell
// RUN: echo "foo f;" > %t.cpp
// RUN: clang-include-fixer -db=fixed -input='foo= "foo.h","bar.h"' -output-headers %t.cpp -- | FileCheck %s
-// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"}]}' %t.cpp | FileCheck %s -check-prefix=CHECK-CODE
-// RUN: cat %t.cpp | not clang-include-fixer -stdin -insert-header='{QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"},{Header: "\"foo2.h\"", QualifiedName: "foo"}]}' %t.cpp
-// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "a:foo"},{Header: "\"foo.h\"", QualifiedName: "b:foo"}]}' %t.cpp
+// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{FilePath: %t.cpp, QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"}]}' %t.cpp | FileCheck %s -check-prefix=CHECK-CODE
+// RUN: cat %t.cpp | not clang-include-fixer -stdin -insert-header='{FilePath: %t.cpp, QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"},{Header: "\"foo2.h\"", QualifiedName: "foo"}]}' %t.cpp
+// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{FilePath: %t.cpp, QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "a:foo"},{Header: "\"foo.h\"", QualifiedName: "b:foo"}]}' %t.cpp
//
// CHECK: "HeaderInfos": [
// CHECK-NEXT: {"Header": "\"foo.h\"",
Index: include-fixer/tool/clang-include-fixer.py
===
--- include-fixer/tool/clang-include-fixer.py
+++ include-fixer/tool/clang-include-fixer.py
@@ -149,21 +149,16 @@
return
try:
-# If there is only one suggested header, insert it directly.
-if len(unique_headers) == 1 or maximum_suggested_headers == 1:
- InsertHeaderToVimBuffer({"QuerySymbolInfos": query_symbol_infos,
- "HeaderInfos": header_infos},
Re: [PATCH] D23266: [include-fixer] Support processing multiple files in one run.
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. lgtm Comment at: include-fixer/IncludeFixerContext.h:78 @@ -72,1 +77,3 @@ + /// \brief The absolute path to the file being processed. + std::string FilePath; I don't think there are any guarantees about that. https://reviews.llvm.org/D23266 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23193: [clang-rename] fix bug with initializer lists
alexfh added a comment.
In https://reviews.llvm.org/D23193#508563, @omtcyfz wrote:
> Do these symbolic pointers in test seem reasonable?
Yes, kind of. I'd change these to C-style comments, place them on the same line
with the interesting identifier (probably, right after the identifier), and
document a generic way of finding the offset using `grep -Ubo 'Foo.*'` or
whatever grep flags are needed.
For example:
$ cat /tmp/a.h
#include
struct Foo /* test 1 */ {
Foo /* test 2 */ (const std::string &s);
std::string s;
};
// Test 1.
// RUN: . -offset=26 ...
// Test2.
// RUN: . -offset=47 ...
// To find offsets after modifying the file, use:
// grep -Ubo 'Foo.*'
$ grep -Ubo 'Foo.*' /tmp/a.h
26:Foo /* test 1 */ {
47:Foo /* test 2 */ (const std::string &s);
255:Foo.*'
https://reviews.llvm.org/D23193
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22856: [analyzer] Change -analyze-function to accept qualified names.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL278018: [analyzer] Change -analyze-function to accept
qualified names. (authored by dergachev).
Changed prior to commit:
https://reviews.llvm.org/D22856?vs=66360&id=67180#toc
Repository:
rL LLVM
https://reviews.llvm.org/D22856
Files:
cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
cfe/trunk/test/Analysis/analyzeOneFunction.m
cfe/trunk/test/Analysis/analyze_display_progress.cpp
cfe/trunk/test/Analysis/analyzer-display-progress.cpp
cfe/trunk/test/Analysis/analyzer-display-progress.m
Index: cfe/trunk/test/Analysis/analyzer-display-progress.cpp
===
--- cfe/trunk/test/Analysis/analyzer-display-progress.cpp
+++ cfe/trunk/test/Analysis/analyzer-display-progress.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -analyze -analyzer-display-progress %s 2>&1 | FileCheck %s
+
+void f() {};
+void g() {};
+void h() {}
+
+struct SomeStruct {
+ void f() {}
+};
+
+struct SomeOtherStruct {
+ void f() {}
+};
+
+namespace ns {
+ struct SomeStruct {
+void f(int) {}
+void f(float, ::SomeStruct) {}
+void f(float, SomeStruct) {}
+ };
+}
+
+// CHECK: analyzer-display-progress.cpp f()
+// CHECK: analyzer-display-progress.cpp g()
+// CHECK: analyzer-display-progress.cpp h()
+// CHECK: analyzer-display-progress.cpp SomeStruct::f()
+// CHECK: analyzer-display-progress.cpp SomeOtherStruct::f()
+// CHECK: analyzer-display-progress.cpp ns::SomeStruct::f(int)
+// CHECK: analyzer-display-progress.cpp ns::SomeStruct::f(float, ::SomeStruct)
+// CHECK: analyzer-display-progress.cpp ns::SomeStruct::f(float, struct ns::SomeStruct)
Index: cfe/trunk/test/Analysis/analyzeOneFunction.m
===
--- cfe/trunk/test/Analysis/analyzeOneFunction.m
+++ cfe/trunk/test/Analysis/analyzeOneFunction.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -analyze -analyze-function="myMethodWithY:withX:" -analyzer-checker=core,osx.cocoa.RetainCount -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -analyze -analyze-function="-[Test1 myMethodWithY:withX:]" -analyzer-checker=core,osx.cocoa.RetainCount -analyzer-store=region -verify %s
typedef signed char BOOL;
typedef unsigned int NSUInteger;
Index: cfe/trunk/test/Analysis/analyzer-display-progress.m
===
--- cfe/trunk/test/Analysis/analyzer-display-progress.m
+++ cfe/trunk/test/Analysis/analyzer-display-progress.m
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -fblocks -analyze -analyzer-display-progress %s 2>&1 | FileCheck %s
+
+#include "Inputs/system-header-simulator-objc.h"
+
+static void f() {}
+
+@interface I: NSObject
+-(void)instanceMethod:(int)arg1 with:(int)arg2;
++(void)classMethod;
+@end
+
+@implementation I
+-(void)instanceMethod:(int)arg1 with:(int)arg2 {}
++(void)classMethod {}
+@end
+
+void g(I *i, int x, int y) {
+ [I classMethod];
+ [i instanceMethod: x with: y];
+
+ void (^block)(void);
+ block = ^{};
+ block();
+}
+
+// CHECK: analyzer-display-progress.m f
+// CHECK: analyzer-display-progress.m -[I instanceMethod:with:]
+// CHECK: analyzer-display-progress.m +[I classMethod]
+// CHECK: analyzer-display-progress.m g
+// CHECK: analyzer-display-progress.m block (line: 22, col: 11)
Index: cfe/trunk/test/Analysis/analyze_display_progress.cpp
===
--- cfe/trunk/test/Analysis/analyze_display_progress.cpp
+++ cfe/trunk/test/Analysis/analyze_display_progress.cpp
@@ -1,26 +0,0 @@
-// RUN: %clang_cc1 -analyze -analyzer-display-progress %s 2>&1 | FileCheck %s
-
-void f() {};
-void g() {};
-void h() {}
-
-struct SomeStruct {
- void f() {}
-};
-
-struct SomeOtherStruct {
- void f() {}
-};
-
-namespace ns {
- struct SomeStruct {
-void f() {}
- };
-}
-
-// CHECK: analyze_display_progress.cpp f
-// CHECK: analyze_display_progress.cpp g
-// CHECK: analyze_display_progress.cpp h
-// CHECK: analyze_display_progress.cpp SomeStruct::f
-// CHECK: analyze_display_progress.cpp SomeOtherStruct::f
-// CHECK: analyze_display_progress.cpp ns::SomeStruct::f
Index: cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -265,19 +265,8 @@
else
assert(Mode == (AM_Syntax | AM_Path) && "Unexpected mode!");
- llvm::errs() << ": " << Loc.getFilename();
- if (isa(D) || isa(D)) {
-const NamedDecl *ND = cast(D);
-llvm::errs() << ' ' << ND->getQualifiedNameAsString() << '\n';
- }
- else if (isa(D)) {
-llvm::errs() << ' ' << "block(line:" << Loc.getLine() << ",col:"
- << Loc.getColumn() << '\n';
- }
- else if (const ObjCMethodDecl *MD = dyn_cast(D)) {
-Selector S = MD->get
Re: [PATCH] D23257: Fix clang-tidy crash when a single fix is applied on multiple files.
alexfh added inline comments.
Comment at: test/clang-tidy/Inputs/modernize-pass-by-value/header-with-fix.h:1
@@ +1,2 @@
+#include
+
hokein wrote:
> Usually test should not use STL headers as it relies on the system headers.
>
> You don't have to use std::string to reproduce the crash, IMO.
Yep, just a class with a user-defined move constructor should be fine.
```
// .h file:
struct S {
S(S&&);
S(const S&);
};
struct Foo {
Foo(const S &s);
S s;
};
// .cpp file:
#include "a.h"
Foo::Foo(const S &s) : s(s) {}
```
https://reviews.llvm.org/D23257
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D23272: [analyzer][Rewrite] Fix boxes and shadows in HTML rewrites in Firefox.
NoQ created this revision.
NoQ added reviewers: dcoughlin, zaks.anna.
NoQ added subscribers: xazax.hun, a.sidorin, cfe-commits.
Use the official CSS3 properties `border-radius` and `box-shadow` (not only
`-webkit-`specific properties).
Fixes analyzer's diagnostic pieces in HTML diagnostics mode in Firefox and
other non-webkit browsers.
Before: {F2254935}
After: {F2254934}
Not sure, do we need more reviewers? - it seems that HTMLRewrite is mostly used
in the analyzer. Also, are there html/css gurus around? Cause the best thing i
can do is trust the "a lot of green boxes" feeling at
http://caniuse.com/#feat=border-radius and
http://caniuse.com/#feat=css-boxshadow :)
https://reviews.llvm.org/D23272
Files:
lib/Rewrite/HTMLRewrite.cpp
Index: lib/Rewrite/HTMLRewrite.cpp
===
--- lib/Rewrite/HTMLRewrite.cpp
+++ lib/Rewrite/HTMLRewrite.cpp
@@ -300,6 +300,7 @@
" .expansion { display: none; }\n"
" .macro:hover .expansion { display: block; border: 2px solid #FF; "
"padding: 2px; background-color:#FFF0F0; font-weight: normal; "
+ " border-radius:5px; box-shadow:1px 1px 7px #000; "
" -webkit-border-radius:5px; -webkit-box-shadow:1px 1px 7px #000; "
"position: absolute; top: -1em; left:10em; z-index: 1 } \n"
" .macro { color: darkmagenta; background-color:LemonChiffon;"
@@ -310,7 +311,9 @@
" .num { color:#44 }\n"
" .line { padding-left: 1ex; border-left: 3px solid #ccc }\n"
" .line { white-space: pre }\n"
+ " .msg { box-shadow:1px 1px 7px #000 }\n"
" .msg { -webkit-box-shadow:1px 1px 7px #000 }\n"
+ " .msg { border-radius:5px }\n"
" .msg { -webkit-border-radius:5px }\n"
" .msg { font-family:Helvetica, sans-serif; font-size:8pt }\n"
" .msg { float:left }\n"
@@ -325,6 +328,7 @@
" .mrange { border-bottom:1px solid #6F9DBE }\n"
" .PathIndex { font-weight: bold; padding:0px 5px; "
"margin-right:5px; }\n"
+ " .PathIndex { border-radius:8px }\n"
" .PathIndex { -webkit-border-radius:8px }\n"
" .PathIndexEvent { background-color:#bfba87 }\n"
" .PathIndexControl { background-color:#8c8c8c }\n"
Index: lib/Rewrite/HTMLRewrite.cpp
===
--- lib/Rewrite/HTMLRewrite.cpp
+++ lib/Rewrite/HTMLRewrite.cpp
@@ -300,6 +300,7 @@
" .expansion { display: none; }\n"
" .macro:hover .expansion { display: block; border: 2px solid #FF; "
"padding: 2px; background-color:#FFF0F0; font-weight: normal; "
+ " border-radius:5px; box-shadow:1px 1px 7px #000; "
" -webkit-border-radius:5px; -webkit-box-shadow:1px 1px 7px #000; "
"position: absolute; top: -1em; left:10em; z-index: 1 } \n"
" .macro { color: darkmagenta; background-color:LemonChiffon;"
@@ -310,7 +311,9 @@
" .num { color:#44 }\n"
" .line { padding-left: 1ex; border-left: 3px solid #ccc }\n"
" .line { white-space: pre }\n"
+ " .msg { box-shadow:1px 1px 7px #000 }\n"
" .msg { -webkit-box-shadow:1px 1px 7px #000 }\n"
+ " .msg { border-radius:5px }\n"
" .msg { -webkit-border-radius:5px }\n"
" .msg { font-family:Helvetica, sans-serif; font-size:8pt }\n"
" .msg { float:left }\n"
@@ -325,6 +328,7 @@
" .mrange { border-bottom:1px solid #6F9DBE }\n"
" .PathIndex { font-weight: bold; padding:0px 5px; "
"margin-right:5px; }\n"
+ " .PathIndex { border-radius:8px }\n"
" .PathIndex { -webkit-border-radius:8px }\n"
" .PathIndexEvent { background-color:#bfba87 }\n"
" .PathIndexControl { background-color:#8c8c8c }\n"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23272: [analyzer][Rewrite] Fix boxes and shadows in HTML rewrites in Firefox.
a.sidorin added inline comments. Comment at: lib/Rewrite/HTMLRewrite.cpp:304 @@ -303,2 +303,3 @@ + " border-radius:5px; box-shadow:1px 1px 7px #000; " " -webkit-border-radius:5px; -webkit-box-shadow:1px 1px 7px #000; " "position: absolute; top: -1em; left:10em; z-index: 1 } \n" Should we remove '-webkit'-prefixed options as well? AFAIR, non-prefixed options should be supported by Webkit. Could you check with Chrome/Safari? https://reviews.llvm.org/D23272 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r277743 - [OpenCL] Added underscores to the names of 'to_addr' OpenCL built-ins.
Okay, merged in r278019.
Cheers,
Hans
On Fri, Aug 5, 2016 at 9:36 AM, Anastasia Stulova
wrote:
> Hans,
>
> If still possible could we merge this into 3.9. It contains just a minor
> renaming but it makes all those new OpenCL Builtins usable.
>
> Thanks,
> Anastasia
>
> -Original Message-
> From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of
> Alexey Bader via cfe-commits
> Sent: 04 August 2016 19:06
> To: cfe-commits@lists.llvm.org
> Subject: r277743 - [OpenCL] Added underscores to the names of 'to_addr'
> OpenCL built-ins.
>
> Author: bader
> Date: Thu Aug 4 13:06:27 2016
> New Revision: 277743
>
> URL: http://llvm.org/viewvc/llvm-project?rev=277743&view=rev
> Log:
> [OpenCL] Added underscores to the names of 'to_addr' OpenCL built-ins.
>
> Summary:
> In order to re-define OpenCL built-in functions 'to_{private,local,global}'
> in OpenCL run-time library LLVM names must be different from the clang
> built-in function names.
>
> Reviewers: yaxunl, Anastasia
>
> Subscribers: cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D23120
>
> Modified:
> cfe/trunk/lib/CodeGen/CGBuiltin.cpp
> cfe/trunk/test/CodeGenOpenCL/to_addr_builtin.cl
>
> Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=277743&r1=277742&r2=277743&view=diff
> ==
> --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Thu Aug 4 13:06:27 2016
> @@ -2209,8 +2209,9 @@ RValue CodeGenFunction::EmitBuiltinExpr(
>NewArg = Builder.CreateAddrSpaceCast(Arg0, NewArgT);
> else
>NewArg = Builder.CreateBitOrPointerCast(Arg0, NewArgT);
> -auto NewCall = Builder.CreateCall(CGM.CreateRuntimeFunction(FTy,
> - E->getDirectCallee()->getName()), {NewArg});
> +auto NewName = std::string("__") + E->getDirectCallee()->getName().str();
> +auto NewCall =
> +Builder.CreateCall(CGM.CreateRuntimeFunction(FTy, NewName),
> + {NewArg});
> return RValue::get(Builder.CreateBitOrPointerCast(NewCall,
>ConvertType(E->getType(;
>}
>
> Modified: cfe/trunk/test/CodeGenOpenCL/to_addr_builtin.cl
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/to_addr_builtin.cl?rev=277743&r1=277742&r2=277743&view=diff
> ==
> --- cfe/trunk/test/CodeGenOpenCL/to_addr_builtin.cl (original)
> +++ cfe/trunk/test/CodeGenOpenCL/to_addr_builtin.cl Thu Aug 4 13:06:27
> +++ 2016
> @@ -14,74 +14,74 @@ void test(void) {
>generic int *gen;
>
>//CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(1)* %{{.*}} to i8
> addrspace(4)*
> - //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8 addrspace(4)*
> %[[ARG]])
> + //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @__to_global(i8
> + addrspace(4)* %[[ARG]])
>//CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
>glob = to_global(glob);
>
>//CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(3)* %{{.*}} to i8
> addrspace(4)*
> - //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8 addrspace(4)*
> %[[ARG]])
> + //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @__to_global(i8
> + addrspace(4)* %[[ARG]])
>//CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
>glob = to_global(loc);
>
>//CHECK: %[[ARG:.*]] = addrspacecast i32* %{{.*}} to i8 addrspace(4)*
> - //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8 addrspace(4)*
> %[[ARG]])
> + //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @__to_global(i8
> + addrspace(4)* %[[ARG]])
>//CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
>glob = to_global(priv);
>
>//CHECK: %[[ARG:.*]] = bitcast i32 addrspace(4)* %{{.*}} to i8
> addrspace(4)*
> - //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8 addrspace(4)*
> %[[ARG]])
> + //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @__to_global(i8
> + addrspace(4)* %[[ARG]])
>//CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
>glob = to_global(gen);
>
>//CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(1)* %{{.*}} to i8
> addrspace(4)*
> - //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @to_local(i8 addrspace(4)*
> %[[ARG]])
> + //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @__to_local(i8
> + addrspace(4)* %[[ARG]])
>//CHECK: %{{.*}} = bitcast i8 addrspace(3)* %[[RET]] to i32 addrspace(3)*
>loc = to_local(glob);
>
>//CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(3)* %{{.*}} to i8
> addrspace(4)*
> - //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @to_local(i8 addrspace(4)*
> %[[ARG]])
> + //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @__to_local(i8
> + addrspace(4)* %[[ARG]])
>//CHECK: %{{.*}} = bitcast i8 addrspace(3)* %[[RET]] to i32 addrspace(3)*
>loc = to_local(loc);
>
>//CH
Re: Diagnostics improvements for 3.9
I've merged the lot in r278020. Thanks, Hans On Fri, Aug 5, 2016 at 7:43 PM, Hans Wennborg wrote: > These all sgtm for 3.9. Go ahead and merge with > utils/release/merge.sh, or I'll do it when I get in on Monday. > > Cheers, > Hans > > On Fri, Aug 5, 2016 at 6:52 PM, Richard Trieu wrote: >> Typos and unused test variables fixed in r277900 >> >> On Fri, Aug 5, 2016 at 6:46 PM, Richard Smith wrote: >>> >>> On Fri, Aug 5, 2016 at 6:33 PM, Richard Smith >>> wrote: These all look OK for the branch. Hans, the second one fixes a crasher, so I'd like to take at least that one. The others fix warning false positives (which are not regressions) and look fairly safe. >>> >>> >>> Correction: looks like the final one is in fact a fix for a diagnostic >>> regression. >>> On Fri, Aug 5, 2016 at 4:39 PM, Richard Trieu wrote: > > Hans, Richard, > > These are some last minute diagnostic improvements for 3.9. > > http://llvm.org/viewvc/llvm-project?rev=277796&view=rev > r277796 > Don't warn when negative values, like -1, are used to initialize > unsigned bit fields. > > http://llvm.org/viewvc/llvm-project?rev=277797&view=rev > r277797 > Fix crash with type alias in Template Type Diffing (You have a typo Internalal in this.) > > http://llvm.org/viewvc/llvm-project?rev=277866&view=rev > r277866 > Fix false positive in -Wunsequenced and templates > > http://llvm.org/viewvc/llvm-project?rev=277889&view=rev > r277889 > Fixes for two false positives in -Wreturn-stack-address dealing with > parameter variables and reference to pointer types. (Test has some x3 variables that are unused.) >>> >>> >> ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r278018 - [analyzer] Change -analyze-function to accept qualified names.
Author: dergachev
Date: Mon Aug 8 11:01:02 2016
New Revision: 278018
URL: http://llvm.org/viewvc/llvm-project?rev=278018&view=rev
Log:
[analyzer] Change -analyze-function to accept qualified names.
Both -analyze-function and -analyzer-display-progress now share the same
convention for naming functions, which allows discriminating between
methods with the same name in different classes, C++ overloads, and also
presents Objective-C instance and class methods in the convenient notation.
This also allows looking up the name for the particular function you're trying
to restrict analysis to in the -analyzer-display-progress output,
in case it was not instantly obvious.
Differential Revision: https://reviews.llvm.org/D22856
Added:
cfe/trunk/test/Analysis/analyzer-display-progress.cpp
cfe/trunk/test/Analysis/analyzer-display-progress.m
Removed:
cfe/trunk/test/Analysis/analyze_display_progress.cpp
Modified:
cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
cfe/trunk/test/Analysis/analyzeOneFunction.m
Modified: cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp?rev=278018&r1=278017&r2=278018&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp Mon Aug 8
11:01:02 2016
@@ -265,19 +265,8 @@ public:
else
assert(Mode == (AM_Syntax | AM_Path) && "Unexpected mode!");
- llvm::errs() << ": " << Loc.getFilename();
- if (isa(D) || isa(D)) {
-const NamedDecl *ND = cast(D);
-llvm::errs() << ' ' << ND->getQualifiedNameAsString() << '\n';
- }
- else if (isa(D)) {
-llvm::errs() << ' ' << "block(line:" << Loc.getLine() << ",col:"
- << Loc.getColumn() << '\n';
- }
- else if (const ObjCMethodDecl *MD = dyn_cast(D)) {
-Selector S = MD->getSelector();
-llvm::errs() << ' ' << S.getAsString();
- }
+ llvm::errs() << ": " << Loc.getFilename() << ' '
+ << getFunctionName(D) << '\n';
}
}
@@ -377,6 +366,7 @@ public:
private:
void storeTopLevelDecls(DeclGroupRef DG);
+ std::string getFunctionName(const Decl *D);
/// \brief Check if we should skip (not analyze) the given function.
AnalysisMode getModeForDecl(Decl *D, AnalysisMode Mode);
@@ -568,16 +558,64 @@ void AnalysisConsumer::HandleTranslation
}
-static std::string getFunctionName(const Decl *D) {
- if (const ObjCMethodDecl *ID = dyn_cast(D)) {
-return ID->getSelector().getAsString();
- }
- if (const FunctionDecl *ND = dyn_cast(D)) {
-IdentifierInfo *II = ND->getIdentifier();
-if (II)
- return II->getName();
+std::string AnalysisConsumer::getFunctionName(const Decl *D) {
+ std::string Str;
+ llvm::raw_string_ostream OS(Str);
+
+ if (const FunctionDecl *FD = dyn_cast(D)) {
+OS << FD->getQualifiedNameAsString();
+
+// In C++, there are overloads.
+if (Ctx->getLangOpts().CPlusPlus) {
+ OS << '(';
+ for (const auto &P : FD->parameters()) {
+if (P != *FD->param_begin())
+ OS << ", ";
+OS << P->getType().getAsString();
+ }
+ OS << ')';
+}
+
+ } else if (isa(D)) {
+PresumedLoc Loc = Ctx->getSourceManager().getPresumedLoc(D->getLocation());
+
+if (Loc.isValid()) {
+ OS << "block (line: " << Loc.getLine() << ", col: " << Loc.getColumn()
+ << ')';
+}
+
+ } else if (const ObjCMethodDecl *OMD = dyn_cast(D)) {
+
+// FIXME: copy-pasted from CGDebugInfo.cpp.
+OS << (OMD->isInstanceMethod() ? '-' : '+') << '[';
+const DeclContext *DC = OMD->getDeclContext();
+if (const auto *OID = dyn_cast(DC)) {
+ OS << OID->getName();
+} else if (const auto *OID = dyn_cast(DC)) {
+ OS << OID->getName();
+} else if (const auto *OC = dyn_cast(DC)) {
+ if (OC->IsClassExtension()) {
+OS << OC->getClassInterface()->getName();
+ } else {
+OS << OC->getIdentifier()->getNameStart() << '('
+ << OC->getIdentifier()->getNameStart() << ')';
+ }
+} else if (const auto *OCD = dyn_cast(DC)) {
+ OS << ((const NamedDecl *)OCD)->getIdentifier()->getNameStart() << '('
+ << OCD->getIdentifier()->getNameStart() << ')';
+} else if (isa(DC)) {
+ // We can extract the type of the class from the self pointer.
+ if (ImplicitParamDecl *SelfDecl = OMD->getSelfDecl()) {
+QualType ClassTy =
+cast(SelfDecl->getType())->getPointeeType();
+ClassTy.print(OS, PrintingPolicy(LangOptions()));
+ }
+}
+OS << ' ' << OMD->getSelector().getAsString() << ']';
+
}
- return "";
+
+ return OS.str();
}
AnalysisConsumer::AnalysisMode
Modified: cfe/trunk/test/Analysis/analyzeOneFunction.m
URL:
http://llvm.org/viewvc/llvm
RE: r277743 - [OpenCL] Added underscores to the names of 'to_addr' OpenCL built-ins.
Thanks!
-Original Message-
From: hwennb...@google.com [mailto:hwennb...@google.com] On Behalf Of Hans
Wennborg
Sent: 08 August 2016 17:40
To: Anastasia Stulova
Cc: Alexey Bader; cfe-commits@lists.llvm.org; nd
Subject: Re: r277743 - [OpenCL] Added underscores to the names of 'to_addr'
OpenCL built-ins.
Okay, merged in r278019.
Cheers,
Hans
On Fri, Aug 5, 2016 at 9:36 AM, Anastasia Stulova
wrote:
> Hans,
>
> If still possible could we merge this into 3.9. It contains just a minor
> renaming but it makes all those new OpenCL Builtins usable.
>
> Thanks,
> Anastasia
>
> -Original Message-
> From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On
> Behalf Of Alexey Bader via cfe-commits
> Sent: 04 August 2016 19:06
> To: cfe-commits@lists.llvm.org
> Subject: r277743 - [OpenCL] Added underscores to the names of 'to_addr'
> OpenCL built-ins.
>
> Author: bader
> Date: Thu Aug 4 13:06:27 2016
> New Revision: 277743
>
> URL: http://llvm.org/viewvc/llvm-project?rev=277743&view=rev
> Log:
> [OpenCL] Added underscores to the names of 'to_addr' OpenCL built-ins.
>
> Summary:
> In order to re-define OpenCL built-in functions 'to_{private,local,global}'
> in OpenCL run-time library LLVM names must be different from the clang
> built-in function names.
>
> Reviewers: yaxunl, Anastasia
>
> Subscribers: cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D23120
>
> Modified:
> cfe/trunk/lib/CodeGen/CGBuiltin.cpp
> cfe/trunk/test/CodeGenOpenCL/to_addr_builtin.cl
>
> Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cp
> p?rev=277743&r1=277742&r2=277743&view=diff
> ==
>
> --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Thu Aug 4 13:06:27 2016
> @@ -2209,8 +2209,9 @@ RValue CodeGenFunction::EmitBuiltinExpr(
>NewArg = Builder.CreateAddrSpaceCast(Arg0, NewArgT);
> else
>NewArg = Builder.CreateBitOrPointerCast(Arg0, NewArgT);
> -auto NewCall = Builder.CreateCall(CGM.CreateRuntimeFunction(FTy,
> - E->getDirectCallee()->getName()), {NewArg});
> +auto NewName = std::string("__") + E->getDirectCallee()->getName().str();
> +auto NewCall =
> +Builder.CreateCall(CGM.CreateRuntimeFunction(FTy, NewName),
> + {NewArg});
> return RValue::get(Builder.CreateBitOrPointerCast(NewCall,
>ConvertType(E->getType(;
>}
>
> Modified: cfe/trunk/test/CodeGenOpenCL/to_addr_builtin.cl
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/to_ad
> dr_builtin.cl?rev=277743&r1=277742&r2=277743&view=diff
> ==
>
> --- cfe/trunk/test/CodeGenOpenCL/to_addr_builtin.cl (original)
> +++ cfe/trunk/test/CodeGenOpenCL/to_addr_builtin.cl Thu Aug 4
> +++ 13:06:27
> +++ 2016
> @@ -14,74 +14,74 @@ void test(void) {
>generic int *gen;
>
>//CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(1)* %{{.*}} to
> i8 addrspace(4)*
> - //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8
> addrspace(4)* %[[ARG]])
> + //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @__to_global(i8
> + addrspace(4)* %[[ARG]])
>//CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
>glob = to_global(glob);
>
>//CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(3)* %{{.*}} to
> i8 addrspace(4)*
> - //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8
> addrspace(4)* %[[ARG]])
> + //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @__to_global(i8
> + addrspace(4)* %[[ARG]])
>//CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
>glob = to_global(loc);
>
>//CHECK: %[[ARG:.*]] = addrspacecast i32* %{{.*}} to i8
> addrspace(4)*
> - //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8
> addrspace(4)* %[[ARG]])
> + //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @__to_global(i8
> + addrspace(4)* %[[ARG]])
>//CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
>glob = to_global(priv);
>
>//CHECK: %[[ARG:.*]] = bitcast i32 addrspace(4)* %{{.*}} to i8
> addrspace(4)*
> - //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8
> addrspace(4)* %[[ARG]])
> + //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @__to_global(i8
> + addrspace(4)* %[[ARG]])
>//CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
>glob = to_global(gen);
>
>//CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(1)* %{{.*}} to
> i8 addrspace(4)*
> - //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @to_local(i8
> addrspace(4)* %[[ARG]])
> + //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @__to_local(i8
> + addrspace(4)* %[[ARG]])
>//CHECK: %{{.*}} = bitcast i8 addrspace(3)* %[[RET]] to i32 addrspace(3)*
>loc = to_local(glob);
>
>//CHECK: %[[ARG:.*]] = addrspacecast i32 addrs
Re: [PATCH] D23243: [clang-tidy] enhance modernize-use-bool-literals to check ternary operator
omtcyfz marked an inline comment as done. omtcyfz added a comment. https://reviews.llvm.org/D23243 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D23274: Add an option to clang-format to remove duplicate headers.
ioeric created this revision.
ioeric added a reviewer: djasper.
ioeric added a subscriber: cfe-commits.
Herald added a subscriber: klimek.
https://reviews.llvm.org/D23274
Files:
include/clang/Format/Format.h
lib/Format/Format.cpp
test/Format/remove-duplicate-includes.cpp
tools/clang-format/ClangFormat.cpp
unittests/Format/SortIncludesTest.cpp
Index: unittests/Format/SortIncludesTest.cpp
===
--- unittests/Format/SortIncludesTest.cpp
+++ unittests/Format/SortIncludesTest.cpp
@@ -26,8 +26,9 @@
std::string sort(StringRef Code, StringRef FileName = "input.cpp") {
auto Ranges = GetCodeRange(Code);
-auto Sorted =
-applyAllReplacements(Code, sortIncludes(Style, Code, Ranges, FileName));
+auto Replaces = sortIncludes(Style, Code, Ranges, FileName);
+Ranges = tooling::calculateRangesAfterReplacements(Replaces, Ranges);
+auto Sorted = applyAllReplacements(Code, Replaces);
EXPECT_TRUE(static_cast(Sorted));
auto Result = applyAllReplacements(
*Sorted, reformat(Style, *Sorted, Ranges, FileName));
@@ -286,6 +287,87 @@
EXPECT_EQ(10u, newCursor(Code, 43));
}
+TEST_F(SortIncludesTest, DeduplicateIncludes) {
+ Style.DeduplicateIncludes = true;
+ EXPECT_EQ("#include \n"
+"#include \n"
+"#include \n",
+sort("#include \n"
+ "#include \n"
+ "#include \n"
+ "#include \n"
+ "#include \n"
+ "#include \n"));
+}
+
+TEST_F(SortIncludesTest, SortAndDeduplicateIncludes) {
+ Style.DeduplicateIncludes = true;
+ EXPECT_EQ("#include \n"
+"#include \n"
+"#include \n",
+sort("#include \n"
+ "#include \n"
+ "#include \n"
+ "#include \n"
+ "#include \n"
+ "#include \n"));
+}
+
+TEST_F(SortIncludesTest, CalculatesCorrectCursorPositionAfterDeduplicate) {
+ Style.DeduplicateIncludes = true;
+ std::string Code = "#include \n" // Start of line: 0
+ "#include \n" // Start of line: 13
+ "#include \n" // Start of line: 26
+ "#include \n" // Start of line: 39
+ "#include \n" // Start of line: 52
+ "#include \n"; // Start of line: 65
+ std::string Expected = "#include \n" // Start of line: 0
+ "#include \n" // Start of line: 13
+ "#include \n"; // Start of line: 26
+ EXPECT_EQ(Expected, sort(Code));
+ // Cursor on 'i' in "#include ".
+ EXPECT_EQ(1u, newCursor(Code, 14));
+ // Cursor on 'b' in "#include ".
+ EXPECT_EQ(23u, newCursor(Code, 10));
+ EXPECT_EQ(23u, newCursor(Code, 36));
+ EXPECT_EQ(23u, newCursor(Code, 49));
+ EXPECT_EQ(23u, newCursor(Code, 36));
+ EXPECT_EQ(23u, newCursor(Code, 75));
+ // Cursor on '#' in "#include ".
+ EXPECT_EQ(26u, newCursor(Code, 52));
+}
+
+TEST_F(SortIncludesTest, DeduplicateLocallyInEachBlock) {
+ Style.DeduplicateIncludes = true;
+ EXPECT_EQ("#include \n"
+"#include \n"
+"\n"
+"#include \n"
+"#include \n",
+sort("#include \n"
+ "#include \n"
+ "\n"
+ "#include \n"
+ "#include \n"
+ "#include \n"));
+}
+
+TEST_F(SortIncludesTest, ValidAffactedRangesAfterDeduplicatingIncludes) {
+ Style.DeduplicateIncludes = true;
+ std::string Code = "#include \n"
+ "#include \n"
+ "#include \n"
+ "#include \n"
+ "\n"
+ " int x ;";
+ std::vector Ranges = {tooling::Range(0, 52)};
+ auto Replaces = sortIncludes(Style, Code, Ranges, "input.cpp");
+ Ranges = tooling::calculateRangesAfterReplacements(Replaces, Ranges);
+ EXPECT_EQ(1u, Ranges.size());
+ EXPECT_EQ(0u, Ranges[0].getOffset());
+ EXPECT_EQ(26u, Ranges[0].getLength());
+}
+
} // end namespace
} // end namespace format
} // end namespace clang
Index: tools/clang-format/ClangFormat.cpp
===
--- tools/clang-format/ClangFormat.cpp
+++ tools/clang-format/ClangFormat.cpp
@@ -260,9 +260,8 @@
llvm::errs() << llvm::toString(ChangedCode.takeError()) << "\n";
return true;
}
- for (const auto &R : Replaces)
-Ranges.push_back({R.getOffset(), R.getLength()});
-
+ // Get new affected ranges after sorting `#includes`.
+ Ranges = tooling::calculateRangesAfterReplacements(Replaces, Ranges);
bool IncompleteFormat = false;
Replacements FormatChanges = reformat(FormatStyle, *ChangedCode, Ranges,
AssumedFileName, &IncompleteFormat);
Index: test/Format/remove-duplicate-includes.cpp
===
--- /dev/null
+++
Re: [PATCH] D23243: [clang-tidy] enhance modernize-use-bool-literals to check ternary operator
omtcyfz updated this revision to Diff 67189.
omtcyfz added a comment.
Remove redundant `anyOf` and `anything`.
https://reviews.llvm.org/D23243
Files:
clang-tidy/modernize/UseBoolLiteralsCheck.cpp
docs/clang-tidy/checks/modernize-use-bool-literals.rst
test/clang-tidy/modernize-use-bool-literals.cpp
Index: test/clang-tidy/modernize-use-bool-literals.cpp
===
--- test/clang-tidy/modernize-use-bool-literals.cpp
+++ test/clang-tidy/modernize-use-bool-literals.cpp
@@ -116,3 +116,33 @@
// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: {{.*}}
// CHECK-FIXES: {{^ *}}IntToTrue = true;{{$}}
}
+
+static int Value = 1;
+
+bool Function1() {
+ bool Result = Value == 1 ? 1 : 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: {{.*}}
+ // CHECK-MESSAGES: :[[@LINE-2]]:34: {{.*}}
+ // CHECK-FIXES: {{^ *}}bool Result = Value == 1 ? true : false;{{$}}
+ return Result;
+}
+
+bool Function2() {
+ return Value == 1 ? 1 : 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: {{.*}}
+ // CHECK-MESSAGES: :[[@LINE-2]]:27: {{.*}}
+ // CHECK-FIXES: {{^ *}}return Value == 1 ? true : false;{{$}}
+}
+
+void foo() {
+ bool Result;
+ Result = Value == 1 ? true : 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:32: {{.*}}
+ // CHECK-FIXES: {{^ *}}Result = Value == 1 ? true : false;{{$}}
+ Result = Value == 1 ? false : bool(0);
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: {{.*}}
+ // CHECK-FIXES: {{^ *}}Result = Value == 1 ? false : false;{{$}}
+ Result = Value == 1 ? (bool)0 : false;
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: {{.*}}
+ // CHECK-FIXES: {{^ *}}Result = Value == 1 ? false : false;{{$}}
+}
Index: docs/clang-tidy/checks/modernize-use-bool-literals.rst
===
--- docs/clang-tidy/checks/modernize-use-bool-literals.rst
+++ docs/clang-tidy/checks/modernize-use-bool-literals.rst
@@ -10,9 +10,11 @@
bool p = 1;
bool f = static_cast(1);
std::ios_base::sync_with_stdio(0);
+ bool x = p ? 1 : 0;
// transforms to
bool p = true;
bool f = true;
std::ios_base::sync_with_stdio(false);
+ bool x = p ? true : false;
Index: clang-tidy/modernize/UseBoolLiteralsCheck.cpp
===
--- clang-tidy/modernize/UseBoolLiteralsCheck.cpp
+++ clang-tidy/modernize/UseBoolLiteralsCheck.cpp
@@ -29,6 +29,17 @@
unless(isInTemplateInstantiation()),
anyOf(hasParent(explicitCastExpr().bind("cast")), anything())),
this);
+
+ Finder->addMatcher(
+ conditionalOperator(
+ hasParent(implicitCastExpr(
+ hasImplicitDestinationType(qualType(booleanType())),
+ unless(isInTemplateInstantiation(,
+ eachOf(hasTrueExpression(
+ ignoringParenImpCasts(integerLiteral().bind("literal"))),
+ hasFalseExpression(
+
ignoringParenImpCasts(integerLiteral().bind("literal"),
+ this);
}
void UseBoolLiteralsCheck::check(const MatchFinder::MatchResult &Result) {
Index: test/clang-tidy/modernize-use-bool-literals.cpp
===
--- test/clang-tidy/modernize-use-bool-literals.cpp
+++ test/clang-tidy/modernize-use-bool-literals.cpp
@@ -116,3 +116,33 @@
// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: {{.*}}
// CHECK-FIXES: {{^ *}}IntToTrue = true;{{$}}
}
+
+static int Value = 1;
+
+bool Function1() {
+ bool Result = Value == 1 ? 1 : 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: {{.*}}
+ // CHECK-MESSAGES: :[[@LINE-2]]:34: {{.*}}
+ // CHECK-FIXES: {{^ *}}bool Result = Value == 1 ? true : false;{{$}}
+ return Result;
+}
+
+bool Function2() {
+ return Value == 1 ? 1 : 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: {{.*}}
+ // CHECK-MESSAGES: :[[@LINE-2]]:27: {{.*}}
+ // CHECK-FIXES: {{^ *}}return Value == 1 ? true : false;{{$}}
+}
+
+void foo() {
+ bool Result;
+ Result = Value == 1 ? true : 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:32: {{.*}}
+ // CHECK-FIXES: {{^ *}}Result = Value == 1 ? true : false;{{$}}
+ Result = Value == 1 ? false : bool(0);
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: {{.*}}
+ // CHECK-FIXES: {{^ *}}Result = Value == 1 ? false : false;{{$}}
+ Result = Value == 1 ? (bool)0 : false;
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: {{.*}}
+ // CHECK-FIXES: {{^ *}}Result = Value == 1 ? false : false;{{$}}
+}
Index: docs/clang-tidy/checks/modernize-use-bool-literals.rst
===
--- docs/clang-tidy/checks/modernize-use-bool-literals.rst
+++ docs/clang-tidy/checks/modernize-use-bool-literals.rst
@@ -10,9 +10,11 @@
bool p = 1;
bool f = static_cast(1);
std::ios_base::sync_with_stdio(0);
+ bool x = p ? 1 : 0;
// transforms to
bool p = true;
bool f = true;
std::ios_base::sync_with_stdio(false);
+ bool x = p ? true : false;
Index: clang-tidy/modernize/UseBoolLiteralsCheck.cpp
==
Re: [PATCH] D23274: Add an option to clang-format to remove duplicate headers.
ioeric updated this revision to Diff 67191.
ioeric added a comment.
- Check code that is not in affected range is not reformatted in lit test.
https://reviews.llvm.org/D23274
Files:
include/clang/Format/Format.h
lib/Format/Format.cpp
test/Format/remove-duplicate-includes.cpp
tools/clang-format/ClangFormat.cpp
unittests/Format/SortIncludesTest.cpp
Index: unittests/Format/SortIncludesTest.cpp
===
--- unittests/Format/SortIncludesTest.cpp
+++ unittests/Format/SortIncludesTest.cpp
@@ -26,8 +26,9 @@
std::string sort(StringRef Code, StringRef FileName = "input.cpp") {
auto Ranges = GetCodeRange(Code);
-auto Sorted =
-applyAllReplacements(Code, sortIncludes(Style, Code, Ranges, FileName));
+auto Replaces = sortIncludes(Style, Code, Ranges, FileName);
+Ranges = tooling::calculateRangesAfterReplacements(Replaces, Ranges);
+auto Sorted = applyAllReplacements(Code, Replaces);
EXPECT_TRUE(static_cast(Sorted));
auto Result = applyAllReplacements(
*Sorted, reformat(Style, *Sorted, Ranges, FileName));
@@ -286,6 +287,87 @@
EXPECT_EQ(10u, newCursor(Code, 43));
}
+TEST_F(SortIncludesTest, DeduplicateIncludes) {
+ Style.DeduplicateIncludes = true;
+ EXPECT_EQ("#include \n"
+"#include \n"
+"#include \n",
+sort("#include \n"
+ "#include \n"
+ "#include \n"
+ "#include \n"
+ "#include \n"
+ "#include \n"));
+}
+
+TEST_F(SortIncludesTest, SortAndDeduplicateIncludes) {
+ Style.DeduplicateIncludes = true;
+ EXPECT_EQ("#include \n"
+"#include \n"
+"#include \n",
+sort("#include \n"
+ "#include \n"
+ "#include \n"
+ "#include \n"
+ "#include \n"
+ "#include \n"));
+}
+
+TEST_F(SortIncludesTest, CalculatesCorrectCursorPositionAfterDeduplicate) {
+ Style.DeduplicateIncludes = true;
+ std::string Code = "#include \n" // Start of line: 0
+ "#include \n" // Start of line: 13
+ "#include \n" // Start of line: 26
+ "#include \n" // Start of line: 39
+ "#include \n" // Start of line: 52
+ "#include \n"; // Start of line: 65
+ std::string Expected = "#include \n" // Start of line: 0
+ "#include \n" // Start of line: 13
+ "#include \n"; // Start of line: 26
+ EXPECT_EQ(Expected, sort(Code));
+ // Cursor on 'i' in "#include ".
+ EXPECT_EQ(1u, newCursor(Code, 14));
+ // Cursor on 'b' in "#include ".
+ EXPECT_EQ(23u, newCursor(Code, 10));
+ EXPECT_EQ(23u, newCursor(Code, 36));
+ EXPECT_EQ(23u, newCursor(Code, 49));
+ EXPECT_EQ(23u, newCursor(Code, 36));
+ EXPECT_EQ(23u, newCursor(Code, 75));
+ // Cursor on '#' in "#include ".
+ EXPECT_EQ(26u, newCursor(Code, 52));
+}
+
+TEST_F(SortIncludesTest, DeduplicateLocallyInEachBlock) {
+ Style.DeduplicateIncludes = true;
+ EXPECT_EQ("#include \n"
+"#include \n"
+"\n"
+"#include \n"
+"#include \n",
+sort("#include \n"
+ "#include \n"
+ "\n"
+ "#include \n"
+ "#include \n"
+ "#include \n"));
+}
+
+TEST_F(SortIncludesTest, ValidAffactedRangesAfterDeduplicatingIncludes) {
+ Style.DeduplicateIncludes = true;
+ std::string Code = "#include \n"
+ "#include \n"
+ "#include \n"
+ "#include \n"
+ "\n"
+ " int x ;";
+ std::vector Ranges = {tooling::Range(0, 52)};
+ auto Replaces = sortIncludes(Style, Code, Ranges, "input.cpp");
+ Ranges = tooling::calculateRangesAfterReplacements(Replaces, Ranges);
+ EXPECT_EQ(1u, Ranges.size());
+ EXPECT_EQ(0u, Ranges[0].getOffset());
+ EXPECT_EQ(26u, Ranges[0].getLength());
+}
+
} // end namespace
} // end namespace format
} // end namespace clang
Index: tools/clang-format/ClangFormat.cpp
===
--- tools/clang-format/ClangFormat.cpp
+++ tools/clang-format/ClangFormat.cpp
@@ -260,9 +260,8 @@
llvm::errs() << llvm::toString(ChangedCode.takeError()) << "\n";
return true;
}
- for (const auto &R : Replaces)
-Ranges.push_back({R.getOffset(), R.getLength()});
-
+ // Get new affected ranges after sorting `#includes`.
+ Ranges = tooling::calculateRangesAfterReplacements(Replaces, Ranges);
bool IncompleteFormat = false;
Replacements FormatChanges = reformat(FormatStyle, *ChangedCode, Ranges,
AssumedFileName, &IncompleteFormat);
Index: test/Format/remove-duplicate-includes.cpp
===
--- /dev/nu
Re: [PATCH] D23243: [clang-tidy] enhance modernize-use-bool-literals to check ternary operator
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
LG with a nit.
Comment at: test/clang-tidy/modernize-use-bool-literals.cpp:124
@@ +123,3 @@
+ bool Result = Value == 1 ? 1 : 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: {{.*}}
+ // CHECK-MESSAGES: :[[@LINE-2]]:34: {{.*}}
The check lines should be a bit stricter. Please change to `:[[@LINE-1]]:30:
warning: converting integer literal to bool`. Other new patterns in this file
as well.
https://reviews.llvm.org/D23243
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23257: Fix clang-tidy crash when a single fix is applied on multiple files.
ioeric updated this revision to Diff 67193.
ioeric marked 2 inline comments as done.
ioeric added a comment.
- Update test to not use
https://reviews.llvm.org/D23257
Files:
clang-tidy/ClangTidy.cpp
clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tidy/ClangTidyDiagnosticConsumer.h
test/clang-tidy/Inputs/modernize-pass-by-value/header-with-fix.h
test/clang-tidy/modernize-pass-by-value-multi-fixes.cpp
unittests/clang-tidy/ClangTidyTest.h
Index: unittests/clang-tidy/ClangTidyTest.h
===
--- unittests/clang-tidy/ClangTidyTest.h
+++ unittests/clang-tidy/ClangTidyTest.h
@@ -119,14 +119,15 @@
DiagConsumer.finish();
tooling::Replacements Fixes;
for (const ClangTidyError &Error : Context.getErrors())
-for (const auto &Fix : Error.Fix) {
- auto Err = Fixes.add(Fix);
- // FIXME: better error handling. Keep the behavior for now.
- if (Err) {
-llvm::errs() << llvm::toString(std::move(Err)) << "\n";
-return "";
+for (const auto &FileAndFixes : Error.Fix)
+ for (const auto &Fix : FileAndFixes.second) {
+auto Err = Fixes.add(Fix);
+// FIXME: better error handling. Keep the behavior for now.
+if (Err) {
+ llvm::errs() << llvm::toString(std::move(Err)) << "\n";
+ return "";
+}
}
-}
if (Errors)
*Errors = Context.getErrors();
auto Result = tooling::applyAllReplacements(Code, Fixes);
Index: test/clang-tidy/modernize-pass-by-value-multi-fixes.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-pass-by-value-multi-fixes.cpp
@@ -0,0 +1,12 @@
+// RUN: cat %S/Inputs/modernize-pass-by-value/header-with-fix.h > %T/pass-by-value-header-with-fix.h
+// RUN: sed -e 's#//.*$##' %s > %t.cpp
+// RUN: clang-tidy %t.cpp -checks='-*,modernize-pass-by-value' -header-filter='.*' -fix -- -std=c++11 -I %T | FileCheck %s -check-prefix=CHECK-MESSAGES -implicit-check-not="{{warning|error}}:"
+// RUN: FileCheck -input-file=%t.cpp %s -check-prefix=CHECK-FIXES
+// RUN: FileCheck -input-file=%T/pass-by-value-header-with-fix.h %s -check-prefix=CHECK-HEADER-FIXES
+
+#include "pass-by-value-header-with-fix.h"
+// CHECK-HEADER-FIXES: Foo(S s);
+Foo::Foo(const S &s) : s(s) {}
+// CHECK-MESSAGES: :9:10: warning: pass by value and use std::move [modernize-pass-by-value]
+// CHECK-FIXES: #include
+// CHECK-FIXES: Foo::Foo(S s) : s(std::move(s)) {}
Index: test/clang-tidy/Inputs/modernize-pass-by-value/header-with-fix.h
===
--- /dev/null
+++ test/clang-tidy/Inputs/modernize-pass-by-value/header-with-fix.h
@@ -0,0 +1,8 @@
+struct S {
+ S(S&&);
+ S(const S&);
+};
+struct Foo {
+ Foo(const S &s);
+ S s;
+};
Index: clang-tidy/ClangTidyDiagnosticConsumer.h
===
--- clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -62,7 +62,8 @@
std::string CheckName;
ClangTidyMessage Message;
- tooling::Replacements Fix;
+ // Fixes grouped by file path.
+ llvm::StringMap Fix;
SmallVector Notes;
// A build directory of the diagnostic source file.
Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -77,8 +77,8 @@
assert(Range.getBegin().isFileID() && Range.getEnd().isFileID() &&
"Only file locations supported in fix-it hints.");
- auto Err =
- Error.Fix.add(tooling::Replacement(SM, Range, FixIt.CodeToInsert));
+ tooling::Replacement Replacement(SM, Range, FixIt.CodeToInsert);
+ auto Err = Error.Fix[Replacement.getFilePath()].add(Replacement);
// FIXME: better error handling.
if (Err) {
llvm::errs() << "Fix conflicts with existing fix! "
@@ -495,27 +495,28 @@
std::vector Sizes;
for (const auto &Error : Errors) {
int Size = 0;
-for (const auto &Replace : Error.Fix)
- Size += Replace.getLength();
+for (const auto &FileAndReplaces : Error.Fix)
+ for (const auto &Replace : FileAndReplaces.second)
+Size += Replace.getLength();
Sizes.push_back(Size);
}
// Build events from error intervals.
std::map> FileEvents;
- for (unsigned I = 0; I < Errors.size(); ++I) {
-for (const auto &Replace : Errors[I].Fix) {
- unsigned Begin = Replace.getOffset();
- unsigned End = Begin + Replace.getLength();
- const std::string &FilePath = Replace.getFilePath();
- // FIXME: Handle empty intervals, such as those from insertions.
- if (Begin == End)
-continue;
- FileEvents[FilePath].push_back(
- Event(Begin, End, Event::ET_Begin, I, Sizes[I]));
- FileEvents[FilePath].push_back(
- Event(Begin, End, Event::ET_End, I, Size
Re: [PATCH] D23243: [clang-tidy] enhance modernize-use-bool-literals to check ternary operator
omtcyfz marked an inline comment as done. omtcyfz added a comment. https://reviews.llvm.org/D23243 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r278022 - [clang-tidy] enhance modernize-use-bool-literals to check ternary operator
Author: omtcyfz
Date: Mon Aug 8 12:11:56 2016
New Revision: 278022
URL: http://llvm.org/viewvc/llvm-project?rev=278022&view=rev
Log:
[clang-tidy] enhance modernize-use-bool-literals to check ternary operator
modernize-use-bool-literals doesn't checks operands in ternary operator.
For example:
``` c++
static int Value = 1;
bool foo() {
bool Result = Value == 1 ? 1 : 0;
return Result;
}
bool boo() {
return Value == 1 ? 1 : 0;
}
```
This issue was reported in bug 28854. The patch fixes it.
Reviewers: alexfh, aaron.ballman, Prazek
Subscribers: Prazek, Eugene.Zelenko
Differential Revision: https://reviews.llvm.org/D23243
Modified:
clang-tools-extra/trunk/clang-tidy/modernize/UseBoolLiteralsCheck.cpp
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-bool-literals.rst
clang-tools-extra/trunk/test/clang-tidy/modernize-use-bool-literals.cpp
Modified: clang-tools-extra/trunk/clang-tidy/modernize/UseBoolLiteralsCheck.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/UseBoolLiteralsCheck.cpp?rev=278022&r1=278021&r2=278022&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/UseBoolLiteralsCheck.cpp
(original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/UseBoolLiteralsCheck.cpp Mon
Aug 8 12:11:56 2016
@@ -29,6 +29,17 @@ void UseBoolLiteralsCheck::registerMatch
unless(isInTemplateInstantiation()),
anyOf(hasParent(explicitCastExpr().bind("cast")), anything())),
this);
+
+ Finder->addMatcher(
+ conditionalOperator(
+ hasParent(implicitCastExpr(
+ hasImplicitDestinationType(qualType(booleanType())),
+ unless(isInTemplateInstantiation(,
+ eachOf(hasTrueExpression(
+ ignoringParenImpCasts(integerLiteral().bind("literal"))),
+ hasFalseExpression(
+
ignoringParenImpCasts(integerLiteral().bind("literal"),
+ this);
}
void UseBoolLiteralsCheck::check(const MatchFinder::MatchResult &Result) {
Modified:
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-bool-literals.rst
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-bool-literals.rst?rev=278022&r1=278021&r2=278022&view=diff
==
---
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-bool-literals.rst
(original)
+++
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-bool-literals.rst
Mon Aug 8 12:11:56 2016
@@ -10,9 +10,11 @@ Finds integer literals which are cast to
bool p = 1;
bool f = static_cast(1);
std::ios_base::sync_with_stdio(0);
+ bool x = p ? 1 : 0;
// transforms to
bool p = true;
bool f = true;
std::ios_base::sync_with_stdio(false);
+ bool x = p ? true : false;
Modified:
clang-tools-extra/trunk/test/clang-tidy/modernize-use-bool-literals.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-use-bool-literals.cpp?rev=278022&r1=278021&r2=278022&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/modernize-use-bool-literals.cpp
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-use-bool-literals.cpp Mon
Aug 8 12:11:56 2016
@@ -5,30 +5,30 @@ bool IntToTrue = 1;
// CHECK-FIXES: {{^}}bool IntToTrue = true;{{$}}
bool IntToFalse(0);
-// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: {{.*}}
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: converting integer literal to bool
// CHECK-FIXES: {{^}}bool IntToFalse(false);{{$}}
bool LongLongToTrue{0x1LL};
-// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: {{.*}}
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: converting integer literal to bool
// CHECK-FIXES: {{^}}bool LongLongToTrue{true};{{$}}
bool ExplicitCStyleIntToFalse = (bool)0;
-// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: {{.*}}
+// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: converting integer literal to bool
// CHECK-FIXES: {{^}}bool ExplicitCStyleIntToFalse = false;{{$}}
bool ExplicitFunctionalIntToFalse = bool(0);
-// CHECK-MESSAGES: :[[@LINE-1]]:37: warning: {{.*}}
+// CHECK-MESSAGES: :[[@LINE-1]]:37: warning: converting integer literal to bool
// CHECK-FIXES: {{^}}bool ExplicitFunctionalIntToFalse = false;{{$}}
bool ExplicitStaticIntToFalse = static_cast(0);
-// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: {{.*}}
+// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: converting integer literal to bool
// CHECK-FIXES: {{^}}bool ExplicitStaticIntToFalse = false;{{$}}
#define TRUE_MACRO 1
// CHECK-FIXES: {{^}}#define TRUE_MACRO 1{{$}}
bool MacroIntToTrue = TRUE_MACRO;
-// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: converting integer literal to
bool, use bool literal instead [modernize-use-bool-literals]
+// CHECK-MESSAGES:
Re: [PATCH] D23243: [clang-tidy] enhance modernize-use-bool-literals to check ternary operator
This revision was automatically updated to reflect the committed changes.
Closed by commit rL278022: [clang-tidy] enhance modernize-use-bool-literals to
check ternary operator (authored by omtcyfz).
Changed prior to commit:
https://reviews.llvm.org/D23243?vs=67195&id=67197#toc
Repository:
rL LLVM
https://reviews.llvm.org/D23243
Files:
clang-tools-extra/trunk/clang-tidy/modernize/UseBoolLiteralsCheck.cpp
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-bool-literals.rst
clang-tools-extra/trunk/test/clang-tidy/modernize-use-bool-literals.cpp
Index: clang-tools-extra/trunk/clang-tidy/modernize/UseBoolLiteralsCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/modernize/UseBoolLiteralsCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/modernize/UseBoolLiteralsCheck.cpp
@@ -29,6 +29,17 @@
unless(isInTemplateInstantiation()),
anyOf(hasParent(explicitCastExpr().bind("cast")), anything())),
this);
+
+ Finder->addMatcher(
+ conditionalOperator(
+ hasParent(implicitCastExpr(
+ hasImplicitDestinationType(qualType(booleanType())),
+ unless(isInTemplateInstantiation(,
+ eachOf(hasTrueExpression(
+ ignoringParenImpCasts(integerLiteral().bind("literal"))),
+ hasFalseExpression(
+ ignoringParenImpCasts(integerLiteral().bind("literal"),
+ this);
}
void UseBoolLiteralsCheck::check(const MatchFinder::MatchResult &Result) {
Index: clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-bool-literals.rst
===
--- clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-bool-literals.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-bool-literals.rst
@@ -10,9 +10,11 @@
bool p = 1;
bool f = static_cast(1);
std::ios_base::sync_with_stdio(0);
+ bool x = p ? 1 : 0;
// transforms to
bool p = true;
bool f = true;
std::ios_base::sync_with_stdio(false);
+ bool x = p ? true : false;
Index: clang-tools-extra/trunk/test/clang-tidy/modernize-use-bool-literals.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/modernize-use-bool-literals.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-use-bool-literals.cpp
@@ -5,39 +5,39 @@
// CHECK-FIXES: {{^}}bool IntToTrue = true;{{$}}
bool IntToFalse(0);
-// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: {{.*}}
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: converting integer literal to bool
// CHECK-FIXES: {{^}}bool IntToFalse(false);{{$}}
bool LongLongToTrue{0x1LL};
-// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: {{.*}}
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: converting integer literal to bool
// CHECK-FIXES: {{^}}bool LongLongToTrue{true};{{$}}
bool ExplicitCStyleIntToFalse = (bool)0;
-// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: {{.*}}
+// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: converting integer literal to bool
// CHECK-FIXES: {{^}}bool ExplicitCStyleIntToFalse = false;{{$}}
bool ExplicitFunctionalIntToFalse = bool(0);
-// CHECK-MESSAGES: :[[@LINE-1]]:37: warning: {{.*}}
+// CHECK-MESSAGES: :[[@LINE-1]]:37: warning: converting integer literal to bool
// CHECK-FIXES: {{^}}bool ExplicitFunctionalIntToFalse = false;{{$}}
bool ExplicitStaticIntToFalse = static_cast(0);
-// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: {{.*}}
+// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: converting integer literal to bool
// CHECK-FIXES: {{^}}bool ExplicitStaticIntToFalse = false;{{$}}
#define TRUE_MACRO 1
// CHECK-FIXES: {{^}}#define TRUE_MACRO 1{{$}}
bool MacroIntToTrue = TRUE_MACRO;
-// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: converting integer literal to bool, use bool literal instead [modernize-use-bool-literals]
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: converting integer literal to bool
// CHECK-FIXES: {{^}}bool MacroIntToTrue = TRUE_MACRO;{{$}}
#define FALSE_MACRO bool(0)
// CHECK-FIXES: {{^}}#define FALSE_MACRO bool(0){{$}}
bool TrueBool = true; // OK
bool FalseBool = bool(FALSE_MACRO);
-// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: {{.*}}
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: converting integer literal to bool
// CHECK-FIXES: {{^}}bool FalseBool = bool(FALSE_MACRO);{{$}}
void boolFunction(bool bar) {
@@ -52,28 +52,28 @@
// CHECK-FIXES: {{^}}#define MACRO_DEPENDENT_CAST(x) static_cast(x){{$}}
bool MacroDependentBool = MACRO_DEPENDENT_CAST(0);
-// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: {{.*}}
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: converting integer literal to bool
// CHECK-FIXES: {{^}}bool MacroDependentBool = MACRO_DEPENDENT_CAST(0);{{$}}
bool ManyMacrosDependent = MACRO_DEPENDENT_CAST(FALSE_MACRO);
-// CHECK-MESSAGES: :[[@LINE-1]]:49: warning: {{.*}}
+// CHECK-MESSAGES: :[[@LINE-1]]:49: warning: converting integ
[libunwind] r278023 - Merging r277868:
Author: hans Date: Mon Aug 8 12:18:56 2016 New Revision: 278023 URL: http://llvm.org/viewvc/llvm-project?rev=278023&view=rev Log: Merging r277868: r277868 | compnerd | 2016-08-05 14:35:28 -0700 (Fri, 05 Aug 2016) | 4 lines unwind: disable executable stacks Similar to compiler-rt, ensure that we disable executable stacks for the custom assembly. Modified: libunwind/branches/release_39/ (props changed) libunwind/branches/release_39/src/UnwindRegistersRestore.S libunwind/branches/release_39/src/UnwindRegistersSave.S libunwind/branches/release_39/src/assembly.h Propchange: libunwind/branches/release_39/ -- --- svn:mergeinfo (original) +++ svn:mergeinfo Mon Aug 8 12:18:56 2016 @@ -1 +1 @@ -/libunwind/trunk:276128 +/libunwind/trunk:276128,277868 Modified: libunwind/branches/release_39/src/UnwindRegistersRestore.S URL: http://llvm.org/viewvc/llvm-project/libunwind/branches/release_39/src/UnwindRegistersRestore.S?rev=278023&r1=278022&r2=278023&view=diff == --- libunwind/branches/release_39/src/UnwindRegistersRestore.S (original) +++ libunwind/branches/release_39/src/UnwindRegistersRestore.S Mon Aug 8 12:18:56 2016 @@ -479,3 +479,6 @@ DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9li l.nop #endif + +NO_EXEC_STACK_DIRECTIVE + Modified: libunwind/branches/release_39/src/UnwindRegistersSave.S URL: http://llvm.org/viewvc/llvm-project/libunwind/branches/release_39/src/UnwindRegistersSave.S?rev=278023&r1=278022&r2=278023&view=diff == --- libunwind/branches/release_39/src/UnwindRegistersSave.S (original) +++ libunwind/branches/release_39/src/UnwindRegistersSave.S Mon Aug 8 12:18:56 2016 @@ -464,3 +464,6 @@ DEFINE_LIBUNWIND_FUNCTION(unw_getcontext l.sw 120(r3), r30 l.sw 124(r3), r31 #endif + +NO_EXEC_STACK_DIRECTIVE + Modified: libunwind/branches/release_39/src/assembly.h URL: http://llvm.org/viewvc/llvm-project/libunwind/branches/release_39/src/assembly.h?rev=278023&r1=278022&r2=278023&view=diff == --- libunwind/branches/release_39/src/assembly.h (original) +++ libunwind/branches/release_39/src/assembly.h Mon Aug 8 12:18:56 2016 @@ -35,19 +35,34 @@ #define SYMBOL_NAME(name) GLUE(__USER_LABEL_PREFIX__, name) #if defined(__APPLE__) + #define SYMBOL_IS_FUNC(name) +#define NO_EXEC_STACK_DIRECTIVE + #elif defined(__ELF__) + #if defined(__arm__) #define SYMBOL_IS_FUNC(name) .type name,%function #else #define SYMBOL_IS_FUNC(name) .type name,@function #endif + +#if defined(__GNU__) || defined(__ANDROID__) || defined(__FreeBSD__) +#define NO_EXEC_STACK_DIRECTIVE .section .note.GNU-stack,"",%progbits +#else +#define NO_EXEC_STACK_DIRECTIVE +#endif + #else + #define SYMBOL_IS_FUNC(name) \ .def name SEPARATOR \ .scl 2 SEPARATOR \ .type 32 SEPARATOR \ .endef + +#define NO_EXEC_STACK_DIRECTIVE + #endif #define DEFINE_LIBUNWIND_FUNCTION(name) \ ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [libunwind] r277868 - unwind: disable executable stacks
Okay, r278023. Thanks, Hans On Fri, Aug 5, 2016 at 2:46 PM, Saleem Abdulrasool wrote: > On Fri, Aug 5, 2016 at 2:35 PM, Saleem Abdulrasool via cfe-commits > wrote: >> >> Author: compnerd >> Date: Fri Aug 5 16:35:28 2016 >> New Revision: 277868 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=277868&view=rev >> Log: >> unwind: disable executable stacks >> >> Similar to compiler-rt, ensure that we disable executable stacks for the >> custom >> assembly. > > > Hi Hans, > > I think that we should get this merged into 3.9. > >> >> >> Modified: >> libunwind/trunk/src/UnwindRegistersRestore.S >> libunwind/trunk/src/UnwindRegistersSave.S >> libunwind/trunk/src/assembly.h >> >> Modified: libunwind/trunk/src/UnwindRegistersRestore.S >> URL: >> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/UnwindRegistersRestore.S?rev=277868&r1=277867&r2=277868&view=diff >> >> == >> --- libunwind/trunk/src/UnwindRegistersRestore.S (original) >> +++ libunwind/trunk/src/UnwindRegistersRestore.S Fri Aug 5 16:35:28 2016 >> @@ -488,3 +488,6 @@ DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9li >> l.nop >> >> #endif >> + >> +NO_EXEC_STACK_DIRECTIVE >> + >> >> Modified: libunwind/trunk/src/UnwindRegistersSave.S >> URL: >> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/UnwindRegistersSave.S?rev=277868&r1=277867&r2=277868&view=diff >> >> == >> --- libunwind/trunk/src/UnwindRegistersSave.S (original) >> +++ libunwind/trunk/src/UnwindRegistersSave.S Fri Aug 5 16:35:28 2016 >> @@ -468,3 +468,6 @@ DEFINE_LIBUNWIND_FUNCTION(unw_getcontext >>l.sw 120(r3), r30 >>l.sw 124(r3), r31 >> #endif >> + >> +NO_EXEC_STACK_DIRECTIVE >> + >> >> Modified: libunwind/trunk/src/assembly.h >> URL: >> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/assembly.h?rev=277868&r1=277867&r2=277868&view=diff >> >> == >> --- libunwind/trunk/src/assembly.h (original) >> +++ libunwind/trunk/src/assembly.h Fri Aug 5 16:35:28 2016 >> @@ -35,19 +35,34 @@ >> #define SYMBOL_NAME(name) GLUE(__USER_LABEL_PREFIX__, name) >> >> #if defined(__APPLE__) >> + >> #define SYMBOL_IS_FUNC(name) >> +#define NO_EXEC_STACK_DIRECTIVE >> + >> #elif defined(__ELF__) >> + >> #if defined(__arm__) >> #define SYMBOL_IS_FUNC(name) .type name,%function >> #else >> #define SYMBOL_IS_FUNC(name) .type name,@function >> #endif >> + >> +#if defined(__GNU__) || defined(__ANDROID__) || defined(__FreeBSD__) >> +#define NO_EXEC_STACK_DIRECTIVE .section .note.GNU-stack,"",%progbits >> +#else >> +#define NO_EXEC_STACK_DIRECTIVE >> +#endif >> + >> #else >> + >> #define SYMBOL_IS_FUNC(name) >> \ >>.def name SEPARATOR >> \ >> .scl 2 SEPARATOR >> \ >> .type 32 SEPARATOR >> \ >>.endef >> + >> +#define NO_EXEC_STACK_DIRECTIVE >> + >> #endif >> >> #define DEFINE_LIBUNWIND_FUNCTION(name) \ >> >> >> ___ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > > > > -- > Saleem Abdulrasool > compnerd (at) compnerd (dot) org ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23193: [clang-rename] fix bug with initializer lists
omtcyfz updated this revision to Diff 67199.
omtcyfz added a comment.
Added more info to the test.
https://reviews.llvm.org/D23193
Files:
clang-rename/USRFinder.cpp
clang-rename/USRLocFinder.cpp
test/clang-rename/Field.cpp
Index: test/clang-rename/Field.cpp
===
--- test/clang-rename/Field.cpp
+++ test/clang-rename/Field.cpp
@@ -1,14 +1,15 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=148 -new-name=Bar %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
class Baz {
- int Foo; // CHECK: Bar;
+ int Foo; /* Test 1 */ // CHECK: int Bar;
public:
Baz();
};
-Baz::Baz() : Foo(0) {} // CHECK: Baz::Baz() : Bar(0) {}
+Baz::Baz() : Foo(0) /* Test 2 */ {} // CHECK: Baz::Baz() : Bar(0)
+
+// Test 1.
+// RUN: clang-rename -offset=18 -new-name=Bar %s -- | sed 's,//.*,,' |
FileCheck %s
+// Test 2.
+// RUN: clang-rename -offset=89 -new-name=Bar %s -- | sed 's,//.*,,' |
FileCheck %s
-// Use grep -FUbo 'Foo' to get the correct offset of foo when changing
-// this file.
+// To find offsets after modifying the file, use:
+// grep -Ubo 'Foo.*'
Index: clang-rename/USRLocFinder.cpp
===
--- clang-rename/USRLocFinder.cpp
+++ clang-rename/USRLocFinder.cpp
@@ -48,18 +48,9 @@
// Ignore implicit initializers.
continue;
}
- if (const clang::FieldDecl *FieldDecl = Initializer->getAnyMember()) {
+ if (const clang::FieldDecl *FieldDecl = Initializer->getMember()) {
if (USRSet.find(getUSRForDecl(FieldDecl)) != USRSet.end()) {
- // The initializer refers to a field that is to be renamed.
- SourceLocation Location = Initializer->getSourceLocation();
- StringRef TokenName = Lexer::getSourceText(
- CharSourceRange::getTokenRange(Location),
- Context.getSourceManager(), Context.getLangOpts());
- if (TokenName == PrevName) {
-// The token of the source location we find actually has the old
-// name.
-LocationsFound.push_back(Initializer->getSourceLocation());
- }
+ LocationsFound.push_back(Initializer->getSourceLocation());
}
}
}
Index: clang-rename/USRFinder.cpp
===
--- clang-rename/USRFinder.cpp
+++ clang-rename/USRFinder.cpp
@@ -90,6 +90,24 @@
TypeEndLoc);
}
+ bool VisitCXXConstructorDecl(clang::CXXConstructorDecl *ConstructorDecl) {
+for (auto &Initializer : ConstructorDecl->inits()) {
+ if (Initializer->getSourceOrder() == -1) {
+// Ignore implicit initializers.
+continue;
+ }
+ if (const clang::FieldDecl *FieldDecl = Initializer->getMember()) {
+const auto InitBeginLoc = Initializer->getSourceLocation();
+const auto InitEndLoc = Lexer::getLocForEndOfToken(
+InitBeginLoc, 0, Context.getSourceManager(),
Context.getLangOpts());
+if (!setResult(FieldDecl, InitBeginLoc, InitEndLoc)) {
+ return false;
+}
+ }
+}
+return true;
+ }
+
// Other:
const NamedDecl *getNamedDecl() { return Result; }
Index: test/clang-rename/Field.cpp
===
--- test/clang-rename/Field.cpp
+++ test/clang-rename/Field.cpp
@@ -1,14 +1,15 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=148 -new-name=Bar %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
class Baz {
- int Foo; // CHECK: Bar;
+ int Foo; /* Test 1 */ // CHECK: int Bar;
public:
Baz();
};
-Baz::Baz() : Foo(0) {} // CHECK: Baz::Baz() : Bar(0) {}
+Baz::Baz() : Foo(0) /* Test 2 */ {} // CHECK: Baz::Baz() : Bar(0)
+
+// Test 1.
+// RUN: clang-rename -offset=18 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// Test 2.
+// RUN: clang-rename -offset=89 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
-// Use grep -FUbo 'Foo' to get the correct offset of foo when changing
-// this file.
+// To find offsets after modifying the file, use:
+// grep -Ubo 'Foo.*'
Index: clang-rename/USRLocFinder.cpp
===
--- clang-rename/USRLocFinder.cpp
+++ clang-rename/USRLocFinder.cpp
@@ -48,18 +48,9 @@
// Ignore implicit initializers.
continue;
}
- if (const clang::FieldDecl *FieldDecl = Initializer->getAnyMember()) {
+ if (const clang::FieldDecl *FieldDecl = Initializer->getMember()) {
if (USRSet.find(getUSRForDecl(FieldDecl)) != USRSet.end()) {
- // The initializer refers to a field that is to be renamed.
- SourceLocation Location = Initializer->getSourceLocation();
- StringRef TokenName = Lexer::getSourceText(
- CharSourceRange::getTokenRange(Location),
- Context.getSourceManager(), Context.getL
Re: [PATCH] D23243: [clang-tidy] enhance modernize-use-bool-literals to check ternary operator
omtcyfz updated this revision to Diff 67195.
omtcyfz added a comment.
More strict test messages.
https://reviews.llvm.org/D23243
Files:
clang-tidy/modernize/UseBoolLiteralsCheck.cpp
docs/clang-tidy/checks/modernize-use-bool-literals.rst
test/clang-tidy/modernize-use-bool-literals.cpp
Index: test/clang-tidy/modernize-use-bool-literals.cpp
===
--- test/clang-tidy/modernize-use-bool-literals.cpp
+++ test/clang-tidy/modernize-use-bool-literals.cpp
@@ -5,39 +5,39 @@
// CHECK-FIXES: {{^}}bool IntToTrue = true;{{$}}
bool IntToFalse(0);
-// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: {{.*}}
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: converting integer literal to bool
// CHECK-FIXES: {{^}}bool IntToFalse(false);{{$}}
bool LongLongToTrue{0x1LL};
-// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: {{.*}}
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: converting integer literal to bool
// CHECK-FIXES: {{^}}bool LongLongToTrue{true};{{$}}
bool ExplicitCStyleIntToFalse = (bool)0;
-// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: {{.*}}
+// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: converting integer literal to bool
// CHECK-FIXES: {{^}}bool ExplicitCStyleIntToFalse = false;{{$}}
bool ExplicitFunctionalIntToFalse = bool(0);
-// CHECK-MESSAGES: :[[@LINE-1]]:37: warning: {{.*}}
+// CHECK-MESSAGES: :[[@LINE-1]]:37: warning: converting integer literal to bool
// CHECK-FIXES: {{^}}bool ExplicitFunctionalIntToFalse = false;{{$}}
bool ExplicitStaticIntToFalse = static_cast(0);
-// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: {{.*}}
+// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: converting integer literal to bool
// CHECK-FIXES: {{^}}bool ExplicitStaticIntToFalse = false;{{$}}
#define TRUE_MACRO 1
// CHECK-FIXES: {{^}}#define TRUE_MACRO 1{{$}}
bool MacroIntToTrue = TRUE_MACRO;
-// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: converting integer literal to bool, use bool literal instead [modernize-use-bool-literals]
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: converting integer literal to bool
// CHECK-FIXES: {{^}}bool MacroIntToTrue = TRUE_MACRO;{{$}}
#define FALSE_MACRO bool(0)
// CHECK-FIXES: {{^}}#define FALSE_MACRO bool(0){{$}}
bool TrueBool = true; // OK
bool FalseBool = bool(FALSE_MACRO);
-// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: {{.*}}
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: converting integer literal to bool
// CHECK-FIXES: {{^}}bool FalseBool = bool(FALSE_MACRO);{{$}}
void boolFunction(bool bar) {
@@ -52,28 +52,28 @@
// CHECK-FIXES: {{^}}#define MACRO_DEPENDENT_CAST(x) static_cast(x){{$}}
bool MacroDependentBool = MACRO_DEPENDENT_CAST(0);
-// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: {{.*}}
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: converting integer literal to bool
// CHECK-FIXES: {{^}}bool MacroDependentBool = MACRO_DEPENDENT_CAST(0);{{$}}
bool ManyMacrosDependent = MACRO_DEPENDENT_CAST(FALSE_MACRO);
-// CHECK-MESSAGES: :[[@LINE-1]]:49: warning: {{.*}}
+// CHECK-MESSAGES: :[[@LINE-1]]:49: warning: converting integer literal to bool
// CHECK-FIXES: {{^}}bool ManyMacrosDependent = MACRO_DEPENDENT_CAST(FALSE_MACRO);{{$}}
class FooClass {
public:
FooClass() : JustBool(0) {}
- // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: {{.*}}
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: converting integer literal to bool
// CHECK-FIXES: {{^ *}}FooClass() : JustBool(false) {}{{$}}
FooClass(int) : JustBool{0} {}
- // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: {{.*}}
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: converting integer literal to bool
// CHECK-FIXES: {{^ *}}FooClass(int) : JustBool{false} {}{{$}}
private:
bool JustBool;
bool BoolWithBraces{0};
- // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: {{.*}}
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: converting integer literal to bool
// CHECK-FIXES: {{^ *}}bool BoolWithBraces{false};{{$}}
bool BoolFromInt = 0;
- // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: {{.*}}
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: converting integer literal to bool
// CHECK-FIXES: {{^ *}}bool BoolFromInt = false;{{$}}
bool SimpleBool = true; // OK
};
@@ -93,13 +93,13 @@
template
void anotherTemplateFunction(type) {
bool JustBool = 0;
- // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: {{.*}}
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: converting integer literal to bool
// CHECK-FIXES: {{^ *}}bool JustBool = false;{{$}}
}
int main() {
boolFunction(1);
- // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: {{.*}}
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: converting integer literal to bool
// CHECK-FIXES: {{^ *}}boolFunction(true);{{$}}
boolFunction(false);
@@ -113,6 +113,36 @@
anotherTemplateFunction(1);
IntToTrue = 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: {{.*}}
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: converting integer literal to bool
// CHECK-FIXES: {{^ *}}IntToTrue = true;{{$}}
}
+
Re: [PATCH] D22729: MPIBufferDerefCheck for Clang-Tidy
Alexander_Droste added a comment. Great; thanks again for the review! https://reviews.llvm.org/D22729 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23238: [CUDA] Rename CheckCUDATarget to IsAllowedCUDACall. NFC
tra accepted this revision. tra added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D23238 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] r278029 - CMakeLists.txt cleanups: synchronize version and CMake minimum required version with rest of LLVM, consistent spacing.
Author: eugenezelenko
Date: Mon Aug 8 12:56:28 2016
New Revision: 278029
URL: http://llvm.org/viewvc/llvm-project?rev=278029&view=rev
Log:
CMakeLists.txt cleanups: synchronize version and CMake minimum required version
with rest of LLVM, consistent spacing.
Differential revision: https://reviews.llvm.org/D23094
Modified:
libunwind/trunk/CMakeLists.txt
Modified: libunwind/trunk/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/libunwind/trunk/CMakeLists.txt?rev=278029&r1=278028&r2=278029&view=diff
==
--- libunwind/trunk/CMakeLists.txt (original)
+++ libunwind/trunk/CMakeLists.txt Mon Aug 8 12:56:28 2016
@@ -2,7 +2,7 @@
# Setup Project
#===
-cmake_minimum_required(VERSION 2.8.8)
+cmake_minimum_required(VERSION 3.4.3)
if (POLICY CMP0042)
cmake_policy(SET CMP0042 NEW) # Set MACOSX_RPATH=YES by default
@@ -28,11 +28,11 @@ if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURR
if (NOT HAD_ERROR)
string(REGEX REPLACE "[ \t]*[\r\n]+[ \t]*" ";"
CONFIG_OUTPUT ${CONFIG_OUTPUT})
-else ()
+else()
string(REPLACE ";" " " CONFIG_COMMAND_STR "${CONFIG_COMMAND}")
message(STATUS "${CONFIG_COMMAND_STR}")
message(FATAL_ERROR "llvm-config failed with status ${HAD_ERROR}")
-endif ()
+endif()
list(GET CONFIG_OUTPUT 0 INCLUDE_DIR)
list(GET CONFIG_OUTPUT 1 LLVM_OBJ_ROOT)
@@ -43,32 +43,32 @@ if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURR
set(LLVM_MAIN_SRC_DIR ${MAIN_SRC_DIR} CACHE PATH "Path to LLVM source
tree")
set(LLVM_CMAKE_PATH
"${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm")
set(LLVM_LIT_PATH "${LLVM_PATH}/utils/lit/lit.py")
- else ()
+ else()
message(FATAL_ERROR "llvm-config not found and LLVM_MAIN_SRC_DIR not
defined. "
"Reconfigure with -DLLVM_CONFIG=path/to/llvm-config "
"or -DLLVM_PATH=path/to/llvm-source-root.")
- endif ()
+ endif()
if (EXISTS ${LLVM_CMAKE_PATH})
list(APPEND CMAKE_MODULE_PATH "${LLVM_CMAKE_PATH}")
include("${LLVM_CMAKE_PATH}/AddLLVM.cmake")
include("${LLVM_CMAKE_PATH}/HandleLLVMOptions.cmake")
- else ()
+ else()
message(FATAL_ERROR "Not found: ${LLVM_CMAKE_PATH}")
- endif ()
+ endif()
set(PACKAGE_NAME libunwind)
- set(PACKAGE_VERSION 3.8.0svn)
+ set(PACKAGE_VERSION 4.0.0svn)
set(PACKAGE_STRING "${PACKAGE_NAME} ${PACKAGE_VERSION}")
set(PACKAGE_BUGREPORT "llvm-b...@lists.llvm.org")
if (EXISTS ${LLVM_MAIN_SRC_DIR}/utils/lit/lit.py)
set(LLVM_LIT ${LLVM_MAIN_SRC_DIR}/utils/lit/lit.py)
- else ()
+ else()
# Seek installed Lit.
find_program(LLVM_LIT "lit.py" ${LLVM_MAIN_SRC_DIR}/utils/lit
DOC "Path to lit.py")
- endif ()
+ endif()
if (LLVM_LIT)
# Define the default arguments to use with 'lit', and an option for the
user
@@ -76,16 +76,16 @@ if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURR
set(LIT_ARGS_DEFAULT "-sv")
if (MSVC OR XCODE)
set(LIT_ARGS_DEFAULT "${LIT_ARGS_DEFAULT} --no-progress-bar")
-endif ()
+endif()
set(LLVM_LIT_ARGS "${LIT_ARGS_DEFAULT}" CACHE STRING "Default options for
lit")
# On Win32 hosts, provide an option to specify the path to the GnuWin32
tools.
if (WIN32 AND NOT CYGWIN)
set(LLVM_LIT_TOOLS_DIR "" CACHE PATH "Path to GnuWin32 tools")
-endif ()
- else ()
+endif()
+ else()
set(LLVM_INCLUDE_TESTS OFF)
- endif ()
+ endif()
set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY
${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX})
set(CMAKE_LIBRARY_OUTPUT_DIRECTORY
${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX})
@@ -111,7 +111,6 @@ set(LIBUNWIND_TARGET_TRIPLE "" CACHE STR
set(LIBUNWIND_GCC_TOOLCHAIN "" CACHE PATH "GCC toolchain for cross compiling.")
set(LIBUNWIND_SYSROOT "" CACHE PATH "Sysroot for cross compiling.")
-
# Check that we can build with 32 bits if requested.
if (CMAKE_SIZEOF_VOID_P EQUAL 8 AND NOT WIN32)
if (LIBUNWIND_BUILD_32_BITS AND NOT LLVM_BUILD_32_BITS) # Don't duplicate
the output from LLVM
@@ -220,23 +219,23 @@ if (LIBUNWIND_ENABLE_ASSERTIONS)
# MSVC doesn't like _DEBUG on release builds. See PR 4379.
if (NOT MSVC)
list(APPEND LIBUNWIND_COMPILE_FLAGS -D_DEBUG)
- endif ()
+ endif()
# On Release builds cmake automatically defines NDEBUG, so we
# explicitly undefine it:
if (uppercase_CMAKE_BUILD_TYPE STREQUAL "RELEASE")
list(APPEND LIBUNWIND_COMPILE_FLAGS -UNDEBUG)
- endif ()
+ endif()
else()
if (NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "RELEASE")
list(APPEND LIBUNWIND_COMPILE_FLAGS -DNDEBUG)
- endif ()
-endif ()
+ endif()
+endif()
# Cross-unwinding
if (NOT LIBUNWIND_ENABLE_CROSS_UNWINDING)
list(APPEND LIBUNWIND_COMPILE_FLAGS -D_LIBUNWIND_IS_NATIVE_ONLY)
-endif ()
+endif()
# ARM WMMX register support
if (LIBUNWIND_ENABLE_ARM_WMMX)
@@ -245,12 +244,12 @@ if (LIBUNWIND_E
Re: [PATCH] D23239: [CUDA] Add __device__ overloads for placement new and delete.
tra added a comment. I think we need to add `noexcept` for these in c++11. https://reviews.llvm.org/D23239 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxxabi] r278030 - CMakeLists.txt cleanups: synchronize version with rest of LLVM, consistent spacing.
Author: eugenezelenko
Date: Mon Aug 8 12:59:02 2016
New Revision: 278030
URL: http://llvm.org/viewvc/llvm-project?rev=278030&view=rev
Log:
CMakeLists.txt cleanups: synchronize version with rest of LLVM, consistent
spacing.
Differential revision: https://reviews.llvm.org/D23092
Modified:
libcxxabi/trunk/CMakeLists.txt
Modified: libcxxabi/trunk/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/CMakeLists.txt?rev=278030&r1=278029&r2=278030&view=diff
==
--- libcxxabi/trunk/CMakeLists.txt (original)
+++ libcxxabi/trunk/CMakeLists.txt Mon Aug 8 12:59:02 2016
@@ -64,7 +64,7 @@ if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURR
endif()
set(PACKAGE_NAME libcxxabi)
- set(PACKAGE_VERSION 3.7.0svn)
+ set(PACKAGE_VERSION 4.0.0svn)
set(PACKAGE_STRING "${PACKAGE_NAME} ${PACKAGE_VERSION}")
set(PACKAGE_BUGREPORT "llvm-b...@lists.llvm.org")
@@ -189,8 +189,7 @@ set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${LIB
if (NOT LIBCXXABI_LIBCXX_LIBRARY_PATH)
set(LIBCXXABI_LIBCXX_LIBRARY_PATH "${LIBCXXABI_LIBRARY_DIR}" CACHE PATH
"The path to libc++ library.")
-endif ()
-
+endif()
# Check that we can build with 32 bits if requested.
if (CMAKE_SIZEOF_VOID_P EQUAL 8 AND NOT WIN32)
@@ -246,7 +245,7 @@ endif()
if (LIBCXXABI_USE_COMPILER_RT)
list(APPEND LIBCXXABI_LINK_FLAGS "-rtlib=compiler-rt")
-endif ()
+endif()
append_if(LIBCXXABI_COMPILE_FLAGS LIBCXXABI_HAS_WERROR_FLAG
-Werror=return-type)
@@ -383,7 +382,7 @@ if (LIBCXXABI_USE_LLVM_UNWINDER OR LLVM_
include_directories("${LIBCXXABI_LIBUNWIND_INCLUDES_INTERNAL}")
include_directories("${LIBCXXABI_LIBUNWIND_SOURCES}")
-endif ()
+endif()
# Add source code. This also contains all of the logic for deciding linker
flags
# soname, etc...
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23240: [CUDA] Print a "previous-decl" note when calling an illegal member fn.
tra accepted this revision. tra added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D23240 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
