Re: [PATCH] D20687: Update hasDynamicExceptionSpec to use functionType instead of functionDecl.
hintonda added a comment. Ping... http://reviews.llvm.org/D20687 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r271544 - [docs] Add a limitations section to SourceBasedCodeCoverage.rst
On Thu, Jun 2, 2016 at 10:19 AM, Vedant Kumar via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Author: vedantk
> Date: Thu Jun 2 12:19:45 2016
> New Revision: 271544
>
> URL: http://llvm.org/viewvc/llvm-project?rev=271544&view=rev
> Log:
> [docs] Add a limitations section to SourceBasedCodeCoverage.rst
>
> Modified:
> cfe/trunk/docs/SourceBasedCodeCoverage.rst
>
> Modified: cfe/trunk/docs/SourceBasedCodeCoverage.rst
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/SourceBasedCodeCoverage.rst?rev=271544&r1=271543&r2=271544&view=diff
>
> ==
> --- cfe/trunk/docs/SourceBasedCodeCoverage.rst (original)
> +++ cfe/trunk/docs/SourceBasedCodeCoverage.rst Thu Jun 2 12:19:45 2016
> @@ -165,9 +165,9 @@ A few final notes:
>indexed profiles. To combine profiling data from multiple runs of a
> program,
>try e.g:
>
> -.. code-block:: console
> + .. code-block:: console
>
> -% llvm-profdata merge -sparse foo1.profraw foo2.profdata -o
> foo3.profdata
> + % llvm-profdata merge -sparse foo1.profraw foo2.profdata -o
> foo3.profdata
>
> Format compatibility guarantees
> ===
> @@ -184,3 +184,20 @@ Format compatibility guarantees
> * There is a third format in play: the format of the coverage mappings
> emitted
>into instrumented binaries. Tools must retain **backwards**
> compatibility
>with these formats. These formats are not forwards-compatible.
> +
> +Drawbacks and limitations
> +=
> +
> +* Code coverage does not handle stack unwinding in the presence of
> uncaught
> + exceptions precisely.
I think it's more accurate to say "thrown exceptions" (or just
"exceptions") instead of "uncaught exceptions". The latter could be
interpreted as meaning that if the exception is caught later (in a caller
of f), then the result might still be precise.
Also setjmp/longjmp can affect things too, no?
-- Sean Silva
> Consider the following function:
> +
> + .. code-block:: cpp
> +
> + int f() {
> +may_throw();
> +return 0;
> + }
> +
> + If the function ``may_throw()`` propagates an exception into ``f``, the
> code
> + coverage tool may mark the ``return`` statement as executed even though
> it is
> + not.
>
>
> ___
> 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
r271818 - check-clang: LTO, aka libLTO.so, was redundant here, since llvm-lto depends on it.
Author: chapuni
Date: Sat Jun 4 19:12:59 2016
New Revision: 271818
URL: http://llvm.org/viewvc/llvm-project?rev=271818&view=rev
Log:
check-clang: LTO, aka libLTO.so, was redundant here, since llvm-lto depends on
it.
Modified:
cfe/trunk/test/CMakeLists.txt
Modified: cfe/trunk/test/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CMakeLists.txt?rev=271818&r1=271817&r2=271818&view=diff
==
--- cfe/trunk/test/CMakeLists.txt (original)
+++ cfe/trunk/test/CMakeLists.txt Sat Jun 4 19:12:59 2016
@@ -78,10 +78,6 @@ if( NOT CLANG_BUILT_STANDALONE )
if(TARGET llvm-lto)
list(APPEND CLANG_TEST_DEPS llvm-lto)
endif()
-
- if(TARGET LTO)
-list(APPEND CLANG_TEST_DEPS LTO)
- endif()
endif()
add_custom_target(clang-test-depends DEPENDS ${CLANG_TEST_DEPS})
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r271801 - Add PIE magic for NetBSD. Add tests for the correct flags for
Author: joerg
Date: Sat Jun 4 15:03:26 2016
New Revision: 271801
URL: http://llvm.org/viewvc/llvm-project?rev=271801&view=rev
Log:
Add PIE magic for NetBSD. Add tests for the correct flags for
non-shared, PIE and shared output mode.
Modified:
cfe/trunk/lib/Driver/Tools.cpp
cfe/trunk/test/Driver/netbsd.c
Modified: cfe/trunk/lib/Driver/Tools.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=271801&r1=271800&r2=271801&view=diff
==
--- cfe/trunk/lib/Driver/Tools.cpp (original)
+++ cfe/trunk/lib/Driver/Tools.cpp Sat Jun 4 15:03:26 2016
@@ -8608,6 +8608,7 @@ void netbsd::Linker::ConstructJob(Compil
if (Args.hasArg(options::OPT_shared)) {
CmdArgs.push_back("-Bshareable");
} else {
+ Args.AddAllArgs(CmdArgs, options::OPT_pie);
CmdArgs.push_back("-dynamic-linker");
CmdArgs.push_back("/libexec/ld.elf_so");
}
@@ -8709,15 +8710,15 @@ void netbsd::Linker::ConstructJob(Compil
if (!Args.hasArg(options::OPT_shared)) {
CmdArgs.push_back(
Args.MakeArgString(getToolChain().GetFilePath("crt0.o")));
+}
+CmdArgs.push_back(
+Args.MakeArgString(getToolChain().GetFilePath("crti.o")));
+if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_pie)) {
CmdArgs.push_back(
- Args.MakeArgString(getToolChain().GetFilePath("crti.o")));
- CmdArgs.push_back(
- Args.MakeArgString(getToolChain().GetFilePath("crtbegin.o")));
+ Args.MakeArgString(getToolChain().GetFilePath("crtbeginS.o")));
} else {
CmdArgs.push_back(
- Args.MakeArgString(getToolChain().GetFilePath("crti.o")));
- CmdArgs.push_back(
- Args.MakeArgString(getToolChain().GetFilePath("crtbeginS.o")));
+ Args.MakeArgString(getToolChain().GetFilePath("crtbegin.o")));
}
}
@@ -8783,12 +8784,12 @@ void netbsd::Linker::ConstructJob(Compil
}
if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
-if (!Args.hasArg(options::OPT_shared))
+if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_pie))
CmdArgs.push_back(
- Args.MakeArgString(getToolChain().GetFilePath("crtend.o")));
+ Args.MakeArgString(getToolChain().GetFilePath("crtendS.o")));
else
CmdArgs.push_back(
- Args.MakeArgString(getToolChain().GetFilePath("crtendS.o")));
+ Args.MakeArgString(getToolChain().GetFilePath("crtend.o")));
CmdArgs.push_back(Args.MakeArgString(getToolChain().GetFilePath("crtn.o")));
}
Modified: cfe/trunk/test/Driver/netbsd.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/netbsd.c?rev=271801&r1=271800&r2=271801&view=diff
==
--- cfe/trunk/test/Driver/netbsd.c (original)
+++ cfe/trunk/test/Driver/netbsd.c Sat Jun 4 15:03:26 2016
@@ -1,5 +1,15 @@
// RUN: %clang -no-canonical-prefixes -target x86_64--netbsd \
// RUN: --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=STATIC %s
+// RUN: %clang -no-canonical-prefixes -target x86_64--netbsd \
+// RUN: -pie --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=PIE %s
+// RUN: %clang -no-canonical-prefixes -target x86_64--netbsd \
+// RUN: -shared --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SHARED %s
+
+// RUN: %clang -no-canonical-prefixes -target x86_64--netbsd \
+// RUN: --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
// RUN: | FileCheck -check-prefix=X86_64 %s
// RUN: %clang -no-canonical-prefixes -target x86_64--netbsd7.0.0 \
// RUN: --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
@@ -105,6 +115,32 @@
// RUN: --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
// RUN: | FileCheck -check-prefix=S-POWERPC64 %s
+// STATIC: ld{{.*}}"
+// STATIC-NOT: "-pie"
+// STATIC-NOT: "-Bshareable"
+// STATIC: "-dynamic-linker" "/libexec/ld.elf_so"
+// STATIC-NOT: "-pie"
+// STATIC-NOT: "-Bshareable"
+// STATIC: "{{.*}}/usr/lib{{/|}}crt0.o"
+// STATIC: "{{.*}}/usr/lib{{/|}}crti.o"
"{{.*}}/usr/lib{{/|}}crtbegin.o"
+// STATIC: "{{.*}}/usr/lib{{/|}}crtend.o" "{{.*}}/usr/lib{{/|}}crtn.o"
+
+// SHARED: ld{{.*}}"
+// SHARED-NOT: "-pie"
+// SHARED-NOT: "-dynamic-linker"
+// SHARED-NOT: "{{.*}}/usr/lib{{/|}}crt0.o"
+// SHARED: "{{.*}}/usr/lib{{/|}}crti.o"
"{{.*}}/usr/lib{{/|}}crtbeginS.o"
+// SHARED: "{{.*}}/usr/lib{{/|}}crtendS.o" "{{.*}}/usr/lib{{/|}}crtn.o"
+
+// PIE: ld{{.*}}"
+// PIE-NOT: "-Bshareable"
+// PIE "-pie" "-dynamic-linker" "/libexec/ld.elf_so"
+// PIE-NOT: "-Bshareable"
+// PIE: "{{.*}}/usr/lib{{/|}}crt0.o" "{{.*}}/usr/lib{{/|}}crti.o"
+// PIE: "{{.*}}/usr/lib{{/|}}crtbeginS.o"
+// PIE: "{{.*}}/usr/lib{{/|}}crtendS.o"
+// PIE: "{{.*}}/usr/lib{{/|}}crtn.o"
+
// X86_64: clang{{.*}}"
r271795 - [AVX512] Remove 512-bit andnot tests from the avx512vl test file.
Author: ctopper
Date: Sat Jun 4 11:37:38 2016
New Revision: 271795
URL: http://llvm.org/viewvc/llvm-project?rev=271795&view=rev
Log:
[AVX512] Remove 512-bit andnot tests from the avx512vl test file.
Modified:
cfe/trunk/test/CodeGen/avx512vl-builtins.c
Modified: cfe/trunk/test/CodeGen/avx512vl-builtins.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/avx512vl-builtins.c?rev=271795&r1=271794&r2=271795&view=diff
==
--- cfe/trunk/test/CodeGen/avx512vl-builtins.c (original)
+++ cfe/trunk/test/CodeGen/avx512vl-builtins.c Sat Jun 4 11:37:38 2016
@@ -557,42 +557,6 @@ __mmask8 test_mm256_mask_cmp_epu64_mask(
return (__mmask8)_mm256_mask_cmp_epu64_mask(__u, __a, __b, 7);
}
-__m512i test_mm512_maskz_andnot_epi32 (__mmask16 __k,__m512i __A, __m512i __B)
{
- //CHECK-LABEL: @test_mm512_maskz_andnot_epi32
- //CHECK: @llvm.x86.avx512.mask.pandn.d.512
- return _mm512_maskz_andnot_epi32(__k,__A,__B);
-}
-
-__m512i test_mm512_mask_andnot_epi32 (__mmask16 __k,__m512i __A, __m512i __B,
__m512i __src) {
- //CHECK-LABEL: @test_mm512_mask_andnot_epi32
- //CHECK: @llvm.x86.avx512.mask.pandn.d.512
- return _mm512_mask_andnot_epi32(__src,__k,__A,__B);
-}
-
-__m512i test_mm512_andnot_epi32(__m512i __A, __m512i __B) {
- //CHECK-LABEL: @test_mm512_andnot_epi32
- //CHECK: @llvm.x86.avx512.mask.pandn.d.512
- return _mm512_andnot_epi32(__A,__B);
-}
-
-__m512i test_mm512_maskz_andnot_epi64 (__mmask8 __k,__m512i __A, __m512i __B) {
- //CHECK-LABEL: @test_mm512_maskz_andnot_epi64
- //CHECK: @llvm.x86.avx512.mask.pandn.q.512
- return _mm512_maskz_andnot_epi64(__k,__A,__B);
-}
-
-__m512i test_mm512_mask_andnot_epi64 (__mmask8 __k,__m512i __A, __m512i __B,
__m512i __src) {
- //CHECK-LABEL: @test_mm512_mask_andnot_epi64
- //CHECK: @llvm.x86.avx512.mask.pandn.q.512
- return _mm512_mask_andnot_epi64(__src,__k,__A,__B);
-}
-
-__m512i test_mm512_andnot_epi64(__m512i __A, __m512i __B) {
- //CHECK-LABEL: @test_mm512_andnot_epi64
- //CHECK: @llvm.x86.avx512.mask.pandn.q.512
- return _mm512_andnot_epi64(__A,__B);
-}
-
__m256i test_mm256_mask_add_epi32 (__m256i __W, __mmask8 __U, __m256i __A,
__m256i __B) {
//CHECK-LABEL: @test_mm256_mask_add_epi32
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] r271794 - Don't call memmove when there's nothing to move. Fixes PR#27978.
Author: marshall
Date: Sat Jun 4 11:16:59 2016
New Revision: 271794
URL: http://llvm.org/viewvc/llvm-project?rev=271794&view=rev
Log:
Don't call memmove when there's nothing to move. Fixes PR#27978.
Modified:
libcxx/trunk/include/fstream
Modified: libcxx/trunk/include/fstream
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/fstream?rev=271794&r1=271793&r2=271794&view=diff
==
--- libcxx/trunk/include/fstream (original)
+++ libcxx/trunk/include/fstream Sat Jun 4 11:16:59 2016
@@ -606,7 +606,9 @@ basic_filebuf<_CharT, _Traits>::underflo
}
else
{
-memmove(__extbuf_, __extbufnext_, __extbufend_ - __extbufnext_);
+_LIBCPP_ASSERT ( !(__extbufnext_ == NULL && (__extbufend_ !=
__extbufnext_)), "underflow moving from NULL" );
+if (__extbufend_ != __extbufnext_)
+memmove(__extbuf_, __extbufnext_, __extbufend_ -
__extbufnext_);
__extbufnext_ = __extbuf_ + (__extbufend_ - __extbufnext_);
__extbufend_ = __extbuf_ + (__extbuf_ == __extbuf_min_ ?
sizeof(__extbuf_min_) : __ebs_);
size_t __nmemb = _VSTD::min(static_cast(__ibs_ -
__unget_sz),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20856: [clang-tidy] readability-identifier-naming - Support for Type Aliases
JamesReynolds added a comment. Great, and happy to help. I'm working on another couple that I should hopefully complete sometime next week. If you could commit this for me that would be great, thank you! http://reviews.llvm.org/D20856 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D20997: [Coverage] Fix an assertion failure if the definition of an unused function spans multiple files.
ikudrin created this revision.
ikudrin added reviewers: vsk, bogner, davidxl.
ikudrin added a subscriber: cfe-commits.
ikudrin set the repository for this revision to rL LLVM.
We had an assertion failure if, for example, the definition of an unused inline
function starts
in one macro and ends in another. This patch fixes the issue by finding the
common ancestor
of the start and end locations of that function's body and changing the
locations accordingly.
Repository:
rL LLVM
http://reviews.llvm.org/D20997
Files:
lib/CodeGen/CoverageMappingGen.cpp
test/CoverageMapping/Inputs/ends_a_scope_only
test/CoverageMapping/Inputs/starts_a_scope_only
test/CoverageMapping/unused_function.cpp
Index: test/CoverageMapping/unused_function.cpp
===
--- /dev/null
+++ test/CoverageMapping/unused_function.cpp
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only %s | tee %t.out | FileCheck %s
+
+#define START_SCOPE {
+#define END_SCOPE }
+
+// CHECK: _Z2f0v:
+// CHECK-NEXT: File 0, [[@LINE+1]]:18 -> [[@LINE+1]]:20 = 0
+inline void f0() {}
+
+// CHECK: _Z2f1v:
+// CHECK-NEXT: File 0, [[@LINE+1]]:18 -> [[@LINE+1]]:31 = 0
+inline void f1() START_SCOPE }
+
+// CHECK: _Z2f2v:
+// CHECK-NEXT: File 0, [[@LINE+1]]:18 -> [[@LINE+1]]:29 = 0
+inline void f2() { END_SCOPE
+
+// CHECK: _Z2f3v:
+// CHECK-NEXT: File 0, [[@LINE+1]]:18 -> [[@LINE+1]]:39 = 0
+inline void f3() START_SCOPE END_SCOPE
+
+// CHECK: _Z2f4v:
+// CHECK-NEXT: File 0, [[@LINE+2]]:10 -> [[@LINE+3]]:2 = 0
+inline void f4()
+#include "Inputs/starts_a_scope_only"
+}
+
+// CHECK: _Z2f5v:
+// CHECK-NEXT: File 0, [[@LINE+1]]:18 -> [[@LINE+2]]:36 = 0
+inline void f5() {
+#include "Inputs/ends_a_scope_only"
+
+// CHECK: _Z2f6v:
+// CHECK-NEXT: File 0, [[@LINE+2]]:10 -> [[@LINE+3]]:36 = 0
+inline void f6()
+#include "Inputs/starts_a_scope_only"
+#include "Inputs/ends_a_scope_only"
Index: test/CoverageMapping/Inputs/starts_a_scope_only
===
--- /dev/null
+++ test/CoverageMapping/Inputs/starts_a_scope_only
@@ -0,0 +1 @@
+{
Index: test/CoverageMapping/Inputs/ends_a_scope_only
===
--- /dev/null
+++ test/CoverageMapping/Inputs/ends_a_scope_only
@@ -0,0 +1 @@
+}
Index: lib/CodeGen/CoverageMappingGen.cpp
===
--- lib/CodeGen/CoverageMappingGen.cpp
+++ lib/CodeGen/CoverageMappingGen.cpp
@@ -130,6 +130,16 @@
return strcmp(SM.getBufferName(SM.getSpellingLoc(Loc)), "") == 0;
}
+ /// \brief Check whether \c Loc is included or expanded from \c Parent.
+ bool isNestedIn(SourceLocation Loc, FileID Parent) {
+do {
+ Loc = getIncludeOrExpansionLoc(Loc);
+ if (Loc.isInvalid())
+return false;
+} while (!SM.isInFileID(Loc, Parent));
+return true;
+ }
+
/// \brief Get the start of \c S ignoring macro arguments and builtin macros.
SourceLocation getStart(const Stmt *S) {
SourceLocation Loc = S->getLocStart();
@@ -310,7 +320,23 @@
if (!D->hasBody())
return;
auto Body = D->getBody();
-SourceRegions.emplace_back(Counter(), getStart(Body), getEnd(Body));
+SourceLocation Start = getStart(Body);
+SourceLocation End = getEnd(Body);
+if (!SM.isWrittenInSameFile(Start, End)) {
+ // Walk up to find the common ancestor.
+ // Correct the locations accordingly.
+ FileID ParentFile = SM.getFileID(Start);
+ while (ParentFile != SM.getFileID(End) && !isNestedIn(End, ParentFile)) {
+Start = getIncludeOrExpansionLoc(Start);
+assert(Start.isValid());
+ParentFile = SM.getFileID(Start);
+ }
+ while (ParentFile != SM.getFileID(End)) {
+End = getPreciseTokenLocEnd(getIncludeOrExpansionLoc(End));
+assert(End.isValid());
+ }
+}
+SourceRegions.emplace_back(Counter(), Start, End);
}
/// \brief Write the mapping data to the output stream
@@ -471,16 +497,6 @@
MostRecentLocation = getIncludeOrExpansionLoc(MostRecentLocation);
}
- /// \brief Check whether \c Loc is included or expanded from \c Parent.
- bool isNestedIn(SourceLocation Loc, FileID Parent) {
-do {
- Loc = getIncludeOrExpansionLoc(Loc);
- if (Loc.isInvalid())
-return false;
-} while (!SM.isInFileID(Loc, Parent));
-return true;
- }
-
/// \brief Adjust regions and state when \c NewLoc exits a file.
///
/// If moving from our most recently tracked location to \c NewLoc exits any
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20964: [clang-tidy] Add modernize-use-emplace
Prazek added a comment. In http://reviews.llvm.org/D20964#448551, @Eugene.Zelenko wrote: > In http://reviews.llvm.org/D20964#448525, @Prazek wrote: > > > In http://reviews.llvm.org/D20964#448455, @Eugene.Zelenko wrote: > > > > > I think will be good idea to try this check with LLVM STL too. > > > > > > You mean llvm::SmallVector stuff? > > > No, I meant to build example with -stdlib=libc++, -lc++, -lc++abi. Just to > make sure, that hasName() is proper matcher. I runned it on llvm codebase with libstdc++ and it worked perfectly. I don't think there should be any change with libc++. I will run it only on small with libc++, because it will take too much time to compile whole llvm again. http://reviews.llvm.org/D20964 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18821: Add bugprone-bool-to-integer-conversion
Prazek added inline comments.
Comment at: docs/clang-tidy/checks/bugprone-bool-to-integer-conversion.rst:37
@@ +36,3 @@
+
+It turns out that the common bug is to have function returning only bools but
having int as return type.
+If check finds case like this then it function return type to bool.
alexfh wrote:
> It may well not be a bug. It's definitely not a bug for `extern "C"`
> functions and functions in C code. We definitely don't need to change the
> function return type, since it's a rather involved change (and clang-tidy
> checks currently are simply not able to make such change correctly in case of
> a function with external linkage). So please remove this automated fix.
This check runs only on C++ functions. Maybe checking that the function
isExternC() would be enough?
Comment at: test/clang-tidy/bugprone-bool-to-integer-conversion.cpp:192
@@ +191,3 @@
+ // CHECK-FIXES: bool yat2() {
+
+ auto l = []() {
Why it is dangerous? What I see after runnign on llvm code base, that it is one
of the most frequent case.
The only bug is the extern "C" thing.
http://reviews.llvm.org/D18821
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20857: [clang-tidy] Add modernize-explicit-operator-bool check.
murrayc added a comment.
In http://reviews.llvm.org/D20857#449080, @alexfh wrote:
> Looks like a useful check to have. I'm not sure though, that it has anything
> to do with "modernize". I'd suggest adding a new "bugprone" module (should be
> added by http://reviews.llvm.org/D18821, hopefully soon) and moving the check
> there.
Fair enough. My logic is that this is a problem that can only be fixed properly
in C++11 and that the best/correct/common way to do this has changed from C++98
to C++11. It's not just a nice use of new syntax (such as auto), it's also
fixes bugs, but it's still use of new syntax.
Comment at: test/clang-tidy/modernize-explicit-operator-bool-void-pointer.cpp:6
@@ +5,3 @@
+ operator const void *() const {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: implicit operator const void*
declaration should probably be explicit operator bool
[modernize-explicit-operator-bool]
+return reinterpret_cast(something != 0);
alexfh wrote:
> From the first glance, this doesn't look like an easy mistake to make. Have
> you actually seen this pattern in real code?
Yes, in glibmm and gtkmm, which I maintain. This commit is from me though the
idea wasn't:
https://git.gnome.org/browse/gtkmm/commit/gtk/src/treerowreference.hg?id=c182608593e2d4799f523580a0532fbc68d296b2
We later used a typedef to make that clearer:
https://git.gnome.org/browse/gtkmm/commit/gtk/src/treerowreference.hg?id=7dff74cca47827d6e34bc8f239674bf044ddedaa
There's lots of mention of this in StackOverflow, though not always so clearly.
For instance:
https://stackoverflow.com/questions/9134888/is-using-void-instead-of-bool-an-advisable-practice
http://reviews.llvm.org/D20857
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
