Re: [PATCH] D20687: Update hasDynamicExceptionSpec to use functionType instead of functionDecl.

2016-06-04 Thread don hinton via cfe-commits
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

2016-06-04 Thread Sean Silva via cfe-commits
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=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=271543=271544=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.

2016-06-04 Thread NAKAMURA Takumi via cfe-commits
Author: chapuni
Date: Sat Jun  4 19:12:59 2016
New Revision: 271818

URL: http://llvm.org/viewvc/llvm-project?rev=271818=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=271817=271818=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

2016-06-04 Thread Joerg Sonnenberger via cfe-commits
Author: joerg
Date: Sat Jun  4 15:03:26 2016
New Revision: 271801

URL: http://llvm.org/viewvc/llvm-project?rev=271801=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=271800=271801=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=271800=271801=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{{.*}}" "-cc1" "-triple" 

r271795 - [AVX512] Remove 512-bit andnot tests from the avx512vl test file.

2016-06-04 Thread Craig Topper via cfe-commits
Author: ctopper
Date: Sat Jun  4 11:37:38 2016
New Revision: 271795

URL: http://llvm.org/viewvc/llvm-project?rev=271795=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=271794=271795=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.

2016-06-04 Thread Marshall Clow via cfe-commits
Author: marshall
Date: Sat Jun  4 11:16:59 2016
New Revision: 271794

URL: http://llvm.org/viewvc/llvm-project?rev=271794=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=271793=271794=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

2016-06-04 Thread James Reynolds via cfe-commits
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.

2016-06-04 Thread Igor Kudrin via cfe-commits
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

2016-06-04 Thread Piotr Padlewski via cfe-commits
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

2016-06-04 Thread Piotr Padlewski via cfe-commits
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.

2016-06-04 Thread Murray Cumming via cfe-commits
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