Re: [PATCH] D13126: New static analyzer checker for loss of sign/precision

2016-08-10 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki updated this revision to Diff 67470.
danielmarjamaki added a comment.

Make sure patch can be applied in latest trunk.

Ping.


https://reviews.llvm.org/D13126

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
  test/Analysis/conversion.c

Index: test/Analysis/conversion.c
===
--- test/Analysis/conversion.c
+++ test/Analysis/conversion.c
@@ -0,0 +1,125 @@
+// RUN: %clang_cc1 -Wno-conversion -analyze -analyzer-checker=core,alpha.core.Conversion -verify %s
+
+unsigned char U8;
+signed char S8;
+
+void assign(unsigned U, signed S) {
+  if (S < -10)
+U8 = S; // expected-warning {{Loss of sign in implicit conversion}}
+  if (U > 300)
+S8 = U; // expected-warning {{Loss of precision in implicit conversion}}
+  if (S > 10)
+U8 = S;
+  if (U < 200)
+S8 = U;
+}
+
+void init1() {
+  long long A = 1LL << 60;
+  short X = A; // expected-warning {{Loss of precision in implicit conversion}}
+}
+
+void relational(unsigned U, signed S) {
+  if (S > 10) {
+if (U < S) {
+}
+  }
+  if (S < -10) {
+if (U < S) { // expected-warning {{Loss of sign in implicit conversion}}
+}
+  }
+}
+
+void multiplication(unsigned U, signed S) {
+  if (S > 5)
+S = U * S;
+  if (S < -10)
+S = U * S; // expected-warning {{Loss of sign}}
+}
+
+void division(unsigned U, signed S) {
+  if (S > 5)
+S = U / S;
+  if (S < -10)
+S = U / S; // expected-warning {{Loss of sign}}
+}
+
+void dontwarn1(unsigned U, signed S) {
+  U8 = S; // It might be known that S is always 0x00-0xff.
+  S8 = U; // It might be known that U is always 0x00-0xff.
+
+  U8 = -1;  // Explicit conversion.
+  S8 = ~0U; // Explicit conversion.
+  if (U > 300)
+U8 &= U; // No loss of precision since there is &=.
+}
+
+void dontwarn2(unsigned int U) {
+  if (U <= 4294967295) {
+  }
+  if (U <= (2147483647 * 2U + 1U)) {
+  }
+}
+
+void dontwarn3(int X) {
+  S8 = X ? 'a' : 'b';
+}
+
+// don't warn for macros
+#define DOSTUFF ({ unsigned X = 1000; U8 = X; })
+void dontwarn4() {
+  DOSTUFF;
+}
+
+// don't warn for calculations
+// seen some fp. For instance:  c2 = (c2 >= 'A' && c2 <= 'Z') ? c2 - 'A' + 'a' : c2;
+// there is a todo in the checker to handle calculations
+void dontwarn5() {
+  signed S = -32;
+  U8 = S + 10;
+}
+
+
+// false positives..
+
+int isascii(int c);
+void falsePositive1() {
+  char kb2[5];
+  int X = 1000;
+  if (isascii(X)) {
+// FIXME: should not warn here:
+kb2[0] = X; // expected-warning {{Loss of precision}}
+  }
+}
+
+
+typedef struct FILE {} FILE; int getc(FILE *stream);
+# define EOF (-1)
+char reply_string[8192];
+FILE *cin;
+extern int dostuff (void);
+int falsePositive2() {
+  int c, n;
+  int dig;
+  char *cp = reply_string;
+  int pflag = 0;
+  int code;
+
+  for (;;) {
+dig = n = code = 0;
+while ((c = getc(cin)) != '\n') {
+  if (dig < 4 && dostuff())
+code = code * 10 + (c - '0');
+  if (!pflag && code == 227)
+pflag = 1;
+  if (n == 0)
+n = c;
+  if (c == EOF)
+return(4);
+  if (cp < &reply_string[sizeof(reply_string) - 1])
+// FIXME: should not warn here:
+*cp++ = c; // expected-warning {{Loss of precision}}
+}
+  }
+}
+
Index: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
+++ lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
@@ -0,0 +1,183 @@
+//=== ConversionChecker.cpp -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// Check that there is not loss of sign/precision in assignments, comparisons
+// and multiplications.
+//
+// ConversionChecker uses path sensitive analysis to determine possible values
+// of expressions. A warning is reported when:
+// * a negative value is implicitly converted to an unsigned value in an
+//   assignment, comparison or multiplication.
+// * assignment / initialization when source value is greater than the max
+//   value of target
+//
+// Many compilers and tools have similar checks that are based on semantic
+// analysis. Those checks are sound but has poor precision. ConversionChecker
+// is an alternative to those checks.
+//
+//===--===//
+#include "ClangSACheckers.h"
+#include "clang/AST/ParentMap.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"

[clang-tools-extra] r278201 - clang-rename YAML reader: address post-commit comments

2016-08-10 Thread Miklos Vajna via cfe-commits
Author: vmiklos
Date: Wed Aug 10 02:13:29 2016
New Revision: 278201

URL: http://llvm.org/viewvc/llvm-project?rev=278201&view=rev
Log:
clang-rename YAML reader: address post-commit comments

Modified:
clang-tools-extra/trunk/clang-rename/tool/ClangRename.cpp
clang-tools-extra/trunk/docs/clang-rename.rst

Modified: clang-tools-extra/trunk/clang-rename/tool/ClangRename.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-rename/tool/ClangRename.cpp?rev=278201&r1=278200&r2=278201&view=diff
==
--- clang-tools-extra/trunk/clang-rename/tool/ClangRename.cpp (original)
+++ clang-tools-extra/trunk/clang-rename/tool/ClangRename.cpp Wed Aug 10 
02:13:29 2016
@@ -59,10 +59,8 @@ static int helpMain(int argc, const char
 /// \brief An oldname -> newname rename.
 struct RenameAllInfo {
   std::string OldName;
-  unsigned Offset;
+  unsigned Offset = 0;
   std::string NewName;
-
-  RenameAllInfo() : Offset(0) {}
 };
 
 LLVM_YAML_IS_SEQUENCE_VECTOR(RenameAllInfo)
@@ -70,7 +68,7 @@ LLVM_YAML_IS_SEQUENCE_VECTOR(RenameAllIn
 namespace llvm {
 namespace yaml {
 
-/// \brief Specialized MappingTraits to describe how a RenameAllInfo is /
+/// \brief Specialized MappingTraits to describe how a RenameAllInfo is
 /// (de)serialized.
 template <> struct MappingTraits {
   static void mapping(IO &IO, RenameAllInfo &Info) {
@@ -149,11 +147,12 @@ int subcommandMain(bool isRenameAll, int
 
   if (!Input.empty()) {
 // Populate OldNames and NewNames from a YAML file.
-auto Buffer = llvm::MemoryBuffer::getFile(Input);
+ErrorOr> Buffer =
+llvm::MemoryBuffer::getFile(Input);
 if (!Buffer) {
   errs() << "clang-rename: failed to read " << Input << ": "
  << Buffer.getError().message() << "\n";
-  exit(1);
+  return 1;
 }
 
 std::vector Infos;

Modified: clang-tools-extra/trunk/docs/clang-rename.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-rename.rst?rev=278201&r1=278200&r2=278201&view=diff
==
--- clang-tools-extra/trunk/docs/clang-rename.rst (original)
+++ clang-tools-extra/trunk/docs/clang-rename.rst Wed Aug 10 02:13:29 2016
@@ -42,7 +42,7 @@ To get an offset of a symbol in a file r
   $ grep -FUbo 'foo' file.cpp
 
 
-The tool currently supports renaming actions inside a single Translation Unit
+The tool currently supports renaming actions inside a single translation unit
 only. It is planned to extend the tool's functionality to support multi-TU
 renaming actions in the future.
 


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D23198: clang-rename rename-all: support reading old/newname pairs from a YAML file

2016-08-10 Thread Miklos Vajna via cfe-commits
vmiklos added a comment.

In https://reviews.llvm.org/D23198#510269, @alexfh wrote:

> A few late comments.


I've addressed these in r278201.


Repository:
  rL LLVM

https://reviews.llvm.org/D23198



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r278202 - [x86] Fix a really nasty bug introduced in r276417 where alignment

2016-08-10 Thread Chandler Carruth via cfe-commits
Author: chandlerc
Date: Wed Aug 10 02:32:47 2016
New Revision: 278202

URL: http://llvm.org/viewvc/llvm-project?rev=278202&view=rev
Log:
[x86] Fix a really nasty bug introduced in r276417 where alignment
constraints were added to _mm256_broadcast_{pd,ps} intel intrinsics.

The spec for these intrinics is ... pretty much silent on alignment.
This is especially frustrating considering the amount of discussion of
alignment in the load and store instrinsics. So I was forced to rely on
the specification for the VBROADCASTF128 instruction.

That instruction's spec is *also* completely silent on alignment.
Fortunately, when it comes to the instruction's spec, silence is enough.
There is no #GP fault option for an underaligned address so this
instruction, and by inference the intrinsic, can read any alignment.

As it happens, the old code worked exactly this way and in fact we have
plenty of code that hands pointers with less than 16-byte alignment to
these intrinsics. This code broke pretty spectacularly with this commit.

Fortunately, the fix is super simple! Change a 16 to a 1, and ta da!

Anyways, a lot of debugging for a really boring fix. =]

Modified:
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/test/CodeGen/avx-builtins.c

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=278202&r1=278201&r2=278202&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Wed Aug 10 02:32:47 2016
@@ -7020,7 +7020,7 @@ Value *CodeGenFunction::EmitX86BuiltinEx
   case X86::BI__builtin_ia32_vbroadcastf128_pd256:
   case X86::BI__builtin_ia32_vbroadcastf128_ps256: {
 llvm::Type *DstTy = ConvertType(E->getType());
-return EmitX86SubVectorBroadcast(*this, Ops, DstTy, 128, 16);
+return EmitX86SubVectorBroadcast(*this, Ops, DstTy, 128, 1);
   }
 
   case X86::BI__builtin_ia32_storehps:

Modified: cfe/trunk/test/CodeGen/avx-builtins.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/avx-builtins.c?rev=278202&r1=278201&r2=278202&view=diff
==
--- cfe/trunk/test/CodeGen/avx-builtins.c (original)
+++ cfe/trunk/test/CodeGen/avx-builtins.c Wed Aug 10 02:32:47 2016
@@ -84,14 +84,14 @@ __m256 test_mm256_blendv_ps(__m256 V1, _
 
 __m256d test_mm256_broadcast_pd(__m128d* A) {
   // CHECK-LABEL: test_mm256_broadcast_pd
-  // CHECK: load <2 x double>, <2 x double>* %{{.*}}, align 16
+  // CHECK: load <2 x double>, <2 x double>* %{{.*}}, align 1
   // CHECK: shufflevector <2 x double> %{{.*}}, <2 x double> %{{.*}}, <4 x 
i32> 
   return _mm256_broadcast_pd(A);
 }
 
 __m256 test_mm256_broadcast_ps(__m128* A) {
   // CHECK-LABEL: test_mm256_broadcast_ps
-  // CHECK: load <4 x float>, <4 x float>* %{{.*}}, align 16
+  // CHECK: load <4 x float>, <4 x float>* %{{.*}}, align 1
   // CHECK: shufflevector <4 x float> %{{.*}}, <4 x float> %{{.*}}, <8 x i32> 

   return _mm256_broadcast_ps(A);
 }


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: r276417 - [X86][AVX] Added support for lowering to VBROADCASTF128/VBROADCASTI128 with generic IR

2016-08-10 Thread Chandler Carruth via cfe-commits
On Fri, Jul 22, 2016 at 7:06 AM Simon Pilgrim via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rksimon
> Date: Fri Jul 22 08:58:56 2016
> New Revision: 276417
>
> URL: http://llvm.org/viewvc/llvm-project?rev=276417&view=rev
> Log:
> [X86][AVX] Added support for lowering to VBROADCASTF128/VBROADCASTI128
> with generic IR
>
> As discussed on D22460, I've updated the vbroadcastf128 pd256/ps256
> builtins to map directly to generic IR - load+splat a 128-bit vector to
> both lanes of a 256-bit vector.
>
> Fix for PR28657.
>
> Modified:
> cfe/trunk/lib/CodeGen/CGBuiltin.cpp
> cfe/trunk/test/CodeGen/avx-builtins.c
>
> Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=276417&r1=276416&r2=276417&view=diff
>
> ==
> --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Fri Jul 22 08:58:56 2016
> @@ -6619,6 +6619,26 @@ static Value *EmitX86MaskedLoad(CodeGenF
>return CGF.Builder.CreateMaskedLoad(Ops[0], Align, MaskVec, Ops[1]);
>  }
>
> +static Value *EmitX86SubVectorBroadcast(CodeGenFunction &CGF,
> +SmallVectorImpl &Ops,
> +llvm::Type *DstTy,
> +unsigned SrcSizeInBits,
> +unsigned Align) {
> +  // Load the subvector.
> +  Ops[0] = CGF.Builder.CreateAlignedLoad(Ops[0], Align);
> +
> +  // Create broadcast mask.
> +  unsigned NumDstElts = DstTy->getVectorNumElements();
> +  unsigned NumSrcElts = SrcSizeInBits / DstTy->getScalarSizeInBits();
> +
> +  SmallVector Mask;
> +  for (unsigned i = 0; i != NumDstElts; i += NumSrcElts)
> +for (unsigned j = 0; j != NumSrcElts; ++j)
> +  Mask.push_back(j);
> +
> +  return CGF.Builder.CreateShuffleVector(Ops[0], Ops[0], Mask,
> "subvecbcst");
> +}
> +
>  static Value *EmitX86Select(CodeGenFunction &CGF,
>  Value *Mask, Value *Op0, Value *Op1) {
>
> @@ -6995,6 +7015,13 @@ Value *CodeGenFunction::EmitX86BuiltinEx
>
>  getContext().getTypeAlignInChars(E->getArg(1)->getType()).getQuantity();
>  return EmitX86MaskedLoad(*this, Ops, Align);
>}
> +
> +  case X86::BI__builtin_ia32_vbroadcastf128_pd256:
> +  case X86::BI__builtin_ia32_vbroadcastf128_ps256: {
> +llvm::Type *DstTy = ConvertType(E->getType());
> +return EmitX86SubVectorBroadcast(*this, Ops, DstTy, 128, 16);
>

Somewhat to my surprise, after a bunch of debugging, we found a bug in this
line.

See my fix in r278202. I wanted to mention it here in case others bisect
back to this and wonder. And because frankly, I would never have thought of
this. The broadcast instructions, even when taking a 128-bit input, don't
have an alignment requirement here. Paint me surprised.

Anyways, just FYI and in case you want to double check my fix.
___
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

2016-08-10 Thread Kirill Bobyrev via cfe-commits
omtcyfz updated this revision to Diff 67475.
omtcyfz marked 7 inline comments as done.
omtcyfz added a comment.

Address comments.


https://reviews.llvm.org/D23265

Files:
  clang-tidy/readability/ElseAfterReturnCheck.cpp
  docs/clang-tidy/checks/readability-else-after-return.rst
  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
@@ -3,16 +3,16 @@
 void f(int a) {
   if (a > 0)
 return;
-  else // comment
-// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: don't use else after return
-// CHECK-FIXES: {{^}}  // comment
+  else // comment-0
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'else' after 'return'
+  // CHECK-FIXES: {{^}}  // comment-0
 return;
 
   if (a > 0) {
 return;
-  } else { // comment
-// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: don't use else after return
-// CHECK-FIXES:  } // comment
+  } else { // comment-1
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return'
+  // CHECK-FIXES: {{^}}  } // comment-1
 return;
   }
 
@@ -28,7 +28,34 @@
 f(0);
   else if (a > 10)
 return;
-  else
+  else // comment-2
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'else' after 'return'
+  // CHECK-FIXES: {{^}}  // comment-2
 f(0);
 }
 
+void foo() {
+  for (unsigned x = 0; x < 42; ++x) {
+if (x) {
+  continue;
+} else { // comment-3
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'continue'
+// CHECK-FIXES: {{^}}} // comment-3
+  x++;
+}
+if (x) {
+  break;
+} else { // comment-4
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'break'
+// CHECK-FIXES: {{^}}} // comment-4
+  x++;
+}
+if (x) {
+  throw 42;
+} else { // comment-5
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'throw'
+// CHECK-FIXES: {{^}}} // comment-5
+  x++;
+}
+  }
+}
Index: docs/clang-tidy/checks/readability-else-after-return.rst
===
--- docs/clang-tidy/checks/readability-else-after-return.rst
+++ docs/clang-tidy/checks/readability-else-after-return.rst
@@ -3,7 +3,61 @@
 readability-else-after-return
 =
 
+`LLVM Coding Standards `_ advises to
+reduce indentation where possible and where it makes understanding code easier.
+Early exit is one of the suggested enforcement of that. Please do not use `else`
+or `else if` after something that interrupts control flow - like `return`,
+`break`, `continue`, `throw`, etc.
 
-Flags the usages of ``else`` after ``return``.
+Following piece of code illustrates how check would work. This piece of code:
 
-http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return
+``` c++
+void foo(int Value) {
+  int Local = 0;
+  for (int i = 0; i < 42; i++) {
+if (Value == 1) {
+  return;
+} else {
+  Local++;
+}
+
+if (Value == 2)
+  continue;
+else
+  Local++;
+
+if (Value == 3) {
+  throw 42;
+} else {
+  Local++;
+}
+  }
+}
+```
+
+Would be transformed into:
+
+``` c++
+void foo(int Value) {
+  int Local = 0;
+  for (int i = 0; i < 42; i++) {
+if (Value == 1) {
+  return;
+}
+Local++;
+
+if (Value == 2)
+  continue;
+Local++;
+
+if (Value == 3) {
+  throw 42;
+}
+Local++;
+  }
+}
+```
+
+
+This checks helps to enforce this `Coding Standars recommendation
+`_.
Index: clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -19,20 +19,31 @@
 namespace readability {
 
 void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) {
-  // FIXME: Support continue, break and throw.
+  auto ControlFlowInterruptorMatcher =
+  stmt(anyOf(returnStmt().bind("return"), continueStmt().bind("continue"),
+ breakStmt().bind("break"), cxxThrowExpr().bind("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(ControlFlowInterruptorMatcher,
+   compoundStmt(has(ControlFlowInterruptorMatcher),
+ hasElse(stmt().bind("else")))
+  .bind("if"))),
   this);
 }
 
 void ElseAfterReturnCheck::check(const MatchFi

Re: [PATCH] D23265: [clang-tidy] enhance readability-else-after-return

2016-08-10 Thread Kirill Bobyrev via cfe-commits
omtcyfz updated this revision to Diff 67476.
omtcyfz added a comment.

Make helper matcher `const`.


https://reviews.llvm.org/D23265

Files:
  clang-tidy/readability/ElseAfterReturnCheck.cpp
  docs/clang-tidy/checks/readability-else-after-return.rst
  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
@@ -3,16 +3,16 @@
 void f(int a) {
   if (a > 0)
 return;
-  else // comment
-// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: don't use else after return
-// CHECK-FIXES: {{^}}  // comment
+  else // comment-0
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'else' after 'return'
+  // CHECK-FIXES: {{^}}  // comment-0
 return;
 
   if (a > 0) {
 return;
-  } else { // comment
-// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: don't use else after return
-// CHECK-FIXES:  } // comment
+  } else { // comment-1
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return'
+  // CHECK-FIXES: {{^}}  } // comment-1
 return;
   }
 
@@ -28,7 +28,34 @@
 f(0);
   else if (a > 10)
 return;
-  else
+  else // comment-2
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'else' after 'return'
+  // CHECK-FIXES: {{^}}  // comment-2
 f(0);
 }
 
+void foo() {
+  for (unsigned x = 0; x < 42; ++x) {
+if (x) {
+  continue;
+} else { // comment-3
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'continue'
+// CHECK-FIXES: {{^}}} // comment-3
+  x++;
+}
+if (x) {
+  break;
+} else { // comment-4
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'break'
+// CHECK-FIXES: {{^}}} // comment-4
+  x++;
+}
+if (x) {
+  throw 42;
+} else { // comment-5
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'throw'
+// CHECK-FIXES: {{^}}} // comment-5
+  x++;
+}
+  }
+}
Index: docs/clang-tidy/checks/readability-else-after-return.rst
===
--- docs/clang-tidy/checks/readability-else-after-return.rst
+++ docs/clang-tidy/checks/readability-else-after-return.rst
@@ -3,7 +3,61 @@
 readability-else-after-return
 =
 
+`LLVM Coding Standards `_ advises to
+reduce indentation where possible and where it makes understanding code easier.
+Early exit is one of the suggested enforcement of that. Please do not use `else`
+or `else if` after something that interrupts control flow - like `return`,
+`break`, `continue`, `throw`, etc.
 
-Flags the usages of ``else`` after ``return``.
+Following piece of code illustrates how check would work. This piece of code:
 
-http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return
+``` c++
+void foo(int Value) {
+  int Local = 0;
+  for (int i = 0; i < 42; i++) {
+if (Value == 1) {
+  return;
+} else {
+  Local++;
+}
+
+if (Value == 2)
+  continue;
+else
+  Local++;
+
+if (Value == 3) {
+  throw 42;
+} else {
+  Local++;
+}
+  }
+}
+```
+
+Would be transformed into:
+
+``` c++
+void foo(int Value) {
+  int Local = 0;
+  for (int i = 0; i < 42; i++) {
+if (Value == 1) {
+  return;
+}
+Local++;
+
+if (Value == 2)
+  continue;
+Local++;
+
+if (Value == 3) {
+  throw 42;
+}
+Local++;
+  }
+}
+```
+
+
+This checks helps to enforce this `Coding Standars recommendation
+`_.
Index: clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -19,20 +19,31 @@
 namespace readability {
 
 void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) {
-  // FIXME: Support continue, break and throw.
+  const auto ControlFlowInterruptorMatcher =
+  stmt(anyOf(returnStmt().bind("return"), continueStmt().bind("continue"),
+ breakStmt().bind("break"), cxxThrowExpr().bind("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(ControlFlowInterruptorMatcher,
+   compoundStmt(has(ControlFlowInterruptorMatcher),
+ hasElse(stmt().bind("else")))
+  .bind("if"))),
   this);
 }
 
 void ElseAfterReturnCheck::check(const MatchFinder::MatchResult &Result

Re: [PATCH] D22729: MPIBufferDerefCheck for Clang-Tidy

2016-08-10 Thread Haojian Wu via cfe-commits
hokein added a comment.

Do you have commit access now?


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] D23274: Add an option to clang-format to remove duplicate headers.

2016-08-10 Thread Daniel Jasper via cfe-commits
djasper added inline comments.


Comment at: lib/Format/Format.cpp:1242
@@ -1240,1 +1241,3 @@
   });
+  // The index of the include on which the cursor is currently put.
+  unsigned CurrentCursorIndex = UINT_MAX;

This needs a comment on what it is actually doing (including what the intended 
behavior is). And that comment should also go into the patch description.

Also, it might be a good idea to pull this out into a function.


https://reviews.llvm.org/D23274



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D23274: Add an style option "DeduplicateIncludes" to clang-format to remove duplicate headers when sorting #includes.

2016-08-10 Thread Daniel Jasper via cfe-commits
djasper added inline comments.


Comment at: include/clang/Format/Format.h:551
@@ +550,3 @@
+  /// sorting ``#includes``.
+  bool DeduplicateIncludes;
+

Actually, how about we just do this unconditionally? I think people will always 
want it.


https://reviews.llvm.org/D23274



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D23274: Add an style option "DeduplicateIncludes" to clang-format to remove duplicate headers when sorting #includes.

2016-08-10 Thread Eric Liu via cfe-commits
ioeric updated this revision to Diff 67483.
ioeric marked an inline comment as done.
ioeric added a comment.

- Addressed review comments.


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] D23274: Add an style option "DeduplicateIncludes" to clang-format to remove duplicate headers when sorting #includes.

2016-08-10 Thread Eric Liu via cfe-commits
ioeric added inline comments.


Comment at: include/clang/Format/Format.h:551
@@ +550,3 @@
+  /// sorting ``#includes``.
+  bool DeduplicateIncludes;
+

Sounds good to me. I'll get rid of the option then.


https://reviews.llvm.org/D23274



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D23274: Add an style option "DeduplicateIncludes" to clang-format to remove duplicate headers when sorting #includes.

2016-08-10 Thread Eric Liu via cfe-commits
ioeric updated this revision to Diff 67484.
ioeric added a comment.

- Always deduplicate when sorting includes. Get rid of the option.


https://reviews.llvm.org/D23274

Files:
  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));
@@ -57,10 +58,10 @@
   // Identical #includes have led to a failure with an unstable sort.
   std::string Code = "#include \n"
  "#include \n"
- "#include \n"
- "#include \n"
- "#include \n"
- "#include \n";
+ "#include \n"
+ "#include \n"
+ "#include \n"
+ "#include \n";
   EXPECT_TRUE(sortIncludes(Style, Code, GetCodeRange(Code), "a.cc").empty());
 }
 
@@ -286,6 +287,82 @@
   EXPECT_EQ(10u, newCursor(Code, 43));
 }
 
+TEST_F(SortIncludesTest, DeduplicateIncludes) {
+  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) {
+  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) {
+  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) {
+  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) {
+  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);
   b

Re: [PATCH] D23274: Make clang-format remove duplicate headers when sorting #includes.

2016-08-10 Thread Daniel Jasper via cfe-commits
djasper added inline comments.


Comment at: lib/Format/Format.cpp:1224
@@ -1222,1 +1223,3 @@
 
+// Finds the index of the #include on which the cursor will be put after
+// sorting/deduplicating.

First off, make this return a pair with the two values.

And then the comment needs to be more precise:

  // Returns a pair (Index, OffsetToEOL) describing the position of the cursor
  // before sorting/deduplicating. Index is the index of the include under the 
cursor
  // in the original set of includes. If this include has duplicates, it is the 
index of
  // the first of the duplicates as the others are going to be removed. 
OffsetToEOL
  // Describes the cursor's position relative to the end of its current line.


Comment at: lib/Format/Format.cpp:1260
@@ -1226,3 +1259,3 @@
 static void sortCppIncludes(const FormatStyle &Style,
  const SmallVectorImpl &Includes,
  ArrayRef Ranges, StringRef FileName,

Fix indent while here.


Comment at: lib/Format/Format.cpp:1306
@@ -1263,7 +1305,3 @@
 
-  // Sorting #includes shouldn't change their total number of characters.
-  // This would otherwise mess up 'Ranges'.
-  assert(result.size() ==
- Includes.back().Offset + Includes.back().Text.size() -
- Includes.front().Offset);
-
+  unsigned num_chars_replaced = Includes.back().Offset +
+Includes.back().Text.size() -

This can also be calculated upfront with:

  Includes.back().Offset + Includes.back().Text.size() - Includes.front().Offset

And maybe you can pull some of those out into variables as they are also used 
in the very first line of this function.
  


https://reviews.llvm.org/D23274



___
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

2016-08-10 Thread Alexander Droste via cfe-commits
Alexander_Droste added a comment.

No but I guess it would be a good idea to ask for commit access.
I'll proceed like suggested here: 
http://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access.


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] D17990: [clang-tidy] minor improvements in modernise-deprecated-headers check

2016-08-10 Thread Kirill Bobyrev via cfe-commits
omtcyf0 updated this revision to Diff 67488.
omtcyf0 marked 2 inline comments as done.
omtcyf0 added a comment.

Address comments.


https://reviews.llvm.org/D17990

Files:
  clang-tidy/modernize/DeprecatedHeadersCheck.cpp
  docs/clang-tidy/checks/modernize-deprecated-headers.rst
  test/clang-tidy/modernize-deprecated-headers-cxx03.cpp
  test/clang-tidy/modernize-deprecated-headers-cxx11.cpp

Index: test/clang-tidy/modernize-deprecated-headers-cxx11.cpp
===
--- test/clang-tidy/modernize-deprecated-headers-cxx11.cpp
+++ test/clang-tidy/modernize-deprecated-headers-cxx11.cpp
@@ -1,163 +1,163 @@
 // RUN: %check_clang_tidy %s modernize-deprecated-headers %t -- -extra-arg-before=-isystem%S/Inputs/modernize-deprecated-headers -- -std=c++11 -v
 
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'assert.h'; consider using 'cassert' instead [modernize-deprecated-headers]
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'complex.h'; consider using 'complex' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'ctype.h'; consider using 'cctype' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'errno.h'; consider using 'cerrno' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'fenv.h'; consider using 'cfenv' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'float.h'; consider using 'cfloat' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
-#include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'inttypes.h'; consider using 'cinttypes' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'limits.h'; consider using 'climits' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'locale.h'; consider using 'clocale' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'math.h'; consider using 'cmath' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'setjmp.h'; consider using 'csetjmp' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
-#include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'signal.h'; consider using 'csignal' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
-#include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'stdarg.h'; consider using 'cstdarg' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'stddef.h'; consider using 'cstddef' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'stdint.h'; consider using 'cstdint' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'stdio.h'; consider using 'cstdio' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'stdlib.h'; consider using 'cstdlib' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'string.h'; consider using 'cstring' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'tgmath.h'; consider using 'ctgmath' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'time.h'; consider using 'ctime' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'uchar.h'; consider using 'cuchar' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'wchar.h'; consider using 'cwchar' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'wctype.h'; consider using 'cwctype' instead
+// CHECK-FIXES: {{^}}#include 
 
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'assert.h'; consider using 'cassert' instead [modernize-depr

Re: [PATCH] D17990: [clang-tidy] minor improvements in modernise-deprecated-headers check

2016-08-10 Thread Kirill Bobyrev via cfe-commits
omtcyf0 marked 3 inline comments as done.
omtcyf0 added a comment.

https://reviews.llvm.org/D17990



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D23274: Make clang-format remove duplicate headers when sorting #includes.

2016-08-10 Thread Eric Liu via cfe-commits
ioeric updated this revision to Diff 67489.
ioeric marked 3 inline comments as done.
ioeric added a comment.

- Addressed review comments.


https://reviews.llvm.org/D23274

Files:
  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));
@@ -57,10 +58,10 @@
   // Identical #includes have led to a failure with an unstable sort.
   std::string Code = "#include \n"
  "#include \n"
- "#include \n"
- "#include \n"
- "#include \n"
- "#include \n";
+ "#include \n"
+ "#include \n"
+ "#include \n"
+ "#include \n";
   EXPECT_TRUE(sortIncludes(Style, Code, GetCodeRange(Code), "a.cc").empty());
 }
 
@@ -286,6 +287,82 @@
   EXPECT_EQ(10u, newCursor(Code, 43));
 }
 
+TEST_F(SortIncludesTest, DeduplicateIncludes) {
+  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) {
+  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) {
+  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) {
+  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) {
+  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);
 

Re: [PATCH] D23274: Make clang-format remove duplicate headers when sorting #includes.

2016-08-10 Thread Daniel Jasper via cfe-commits
djasper accepted this revision.
djasper added a comment.
This revision is now accepted and ready to land.

Looks good.


https://reviews.llvm.org/D23274



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r278206 - Make clang-format remove duplicate headers when sorting #includes.

2016-08-10 Thread Eric Liu via cfe-commits
Author: ioeric
Date: Wed Aug 10 04:32:23 2016
New Revision: 278206

URL: http://llvm.org/viewvc/llvm-project?rev=278206&view=rev
Log:
Make clang-format remove duplicate headers when sorting #includes.

Summary: When sorting #includes, #include directives that have the same text 
will be deduplicated when sorting #includes, and only the first #include in the 
duplicate #includes remains. If the `Cursor` is provided and put on a deleted 
#include, it will be put on the remaining #include in the duplicate #includes.

Reviewers: djasper

Subscribers: cfe-commits, klimek

Differential Revision: https://reviews.llvm.org/D23274

Added:
cfe/trunk/test/Format/remove-duplicate-includes.cpp
Modified:
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/tools/clang-format/ClangFormat.cpp
cfe/trunk/unittests/Format/SortIncludesTest.cpp

Modified: cfe/trunk/lib/Format/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=278206&r1=278205&r2=278206&view=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Wed Aug 10 04:32:23 2016
@@ -1223,15 +1223,50 @@ static bool affectsRange(ArrayRef
+FindCursorIndex(const SmallVectorImpl &Includes,
+const SmallVectorImpl &Indices, unsigned Cursor) {
+  unsigned CursorIndex = UINT_MAX;
+  unsigned OffsetToEOL = 0;
+  for (int i = 0, e = Includes.size(); i != e; ++i) {
+unsigned Start = Includes[Indices[i]].Offset;
+unsigned End = Start + Includes[Indices[i]].Text.size();
+if (!(Cursor >= Start && Cursor < End))
+  continue;
+CursorIndex = Indices[i];
+OffsetToEOL = End - Cursor;
+// Put the cursor on the only remaining #include among the duplicate
+// #includes.
+while (--i >= 0 && Includes[CursorIndex].Text == Includes[Indices[i]].Text)
+  CursorIndex = i;
+break;
+  }
+  return std::make_pair(CursorIndex, OffsetToEOL);
+}
+
+// Sorts and deduplicate a block of includes given by 'Includes' alphabetically
+// adding the necessary replacement to 'Replaces'. 'Includes' must be in strict
+// source order.
+// #include directives with the same text will be deduplicated, and only the
+// first #include in the duplicate #includes remains. If the `Cursor` is
+// provided and put on a deleted #include, it will be moved to the remaining
+// #include in the duplicate #includes.
 static void sortCppIncludes(const FormatStyle &Style,
- const SmallVectorImpl &Includes,
- ArrayRef Ranges, StringRef FileName,
- tooling::Replacements &Replaces, unsigned *Cursor) {
-  if (!affectsRange(Ranges, Includes.front().Offset,
-Includes.back().Offset + Includes.back().Text.size()))
+const SmallVectorImpl &Includes,
+ArrayRef Ranges, StringRef 
FileName,
+tooling::Replacements &Replaces, unsigned *Cursor) 
{
+  unsigned IncludesBeginOffset = Includes.front().Offset;
+  unsigned IncludesBlockSize = Includes.back().Offset +
+   Includes.back().Text.size() -
+   IncludesBeginOffset;
+  if (!affectsRange(Ranges, IncludesBeginOffset, IncludesBlockSize))
 return;
   SmallVector Indices;
   for (unsigned i = 0, e = Includes.size(); i != e; ++i)
@@ -1241,37 +1276,39 @@ static void sortCppIncludes(const Format
 return std::tie(Includes[LHSI].Category, Includes[LHSI].Filename) <
std::tie(Includes[RHSI].Category, Includes[RHSI].Filename);
   });
+  // The index of the include on which the cursor will be put after
+  // sorting/deduplicating.
+  unsigned CursorIndex;
+  // The offset from cursor to the end of line.
+  unsigned CursorToEOLOffset;
+  if (Cursor)
+std::tie(CursorIndex, CursorToEOLOffset) =
+FindCursorIndex(Includes, Indices, *Cursor);
+
+  // Deduplicate #includes.
+  Indices.erase(std::unique(Indices.begin(), Indices.end(),
+[&](unsigned LHSI, unsigned RHSI) {
+  return Includes[LHSI].Text == 
Includes[RHSI].Text;
+}),
+Indices.end());
 
   // If the #includes are out of order, we generate a single replacement fixing
   // the entire block. Otherwise, no replacement is generated.
-  if (std::is_sorted(Indices.begin(), Indices.end()))
+  if (Indices.size() == Includes.size() &&
+  std::is_sorted(Indices.begin(), Indices.end()))
 return;
 
   std::string result;
-  bool CursorMoved = false;
   for (unsigned Index : Indices) {
 if (!result.empty())
   result += "\n";
 result += Includes[Index].Text;
-
-if (Cursor && !CursorMoved) {
-  unsigned Start = Includes[Index].Offset;
-  unsigned End = Start + Includes[Index].Text.size();
-  if (*Cursor >= Start && *Cursor < End) {
-*Cursor = Includ

[PATCH] D23346: [OpenCL] Change block descriptor address space to constant

2016-08-10 Thread Joey Gouly via cfe-commits
joey created this revision.
joey added a subscriber: cfe-commits.
joey set the repository for this revision to rL LLVM.

The block descriptor is a GlobalVariable in the LLVM IR, so it shouldn't be in 
the private address space.

Repository:
  rL LLVM

https://reviews.llvm.org/D23346

Files:
  lib/CodeGen/CGBlocks.cpp
  test/CodeGenOpenCL/cl20-device-side-enqueue.cl

Index: test/CodeGenOpenCL/cl20-device-side-enqueue.cl
===
--- test/CodeGenOpenCL/cl20-device-side-enqueue.cl
+++ test/CodeGenOpenCL/cl20-device-side-enqueue.cl
@@ -21,7 +21,7 @@
   // CHECK: [[DEF_Q:%[0-9]+]] = load %opencl.queue_t*, %opencl.queue_t** 
%default_queue
   // CHECK: [[FLAGS:%[0-9]+]] = load i32, i32* %flags
   // CHECK: [[NDR:%[0-9]+]] = load %opencl.ndrange_t*, %opencl.ndrange_t** 
%ndrange
-  // CHECK: [[BL:%[0-9]+]] = bitcast <{ i8*, i32, i32, i8*, 
%struct.__block_descriptor*, i32{{.*}}, i32{{.*}}, i32{{.*}} }>* %block to void 
()*
+  // CHECK: [[BL:%[0-9]+]] = bitcast <{ i8*, i32, i32, i8*, 
%struct.__block_descriptor addrspace(3)*, i32{{.*}}, i32{{.*}}, i32{{.*}} }>* 
%block to void ()*
   // CHECK: [[BL_I8:%[0-9]+]] = bitcast void ()* [[BL]] to i8*
   // CHECK: call i32 @__enqueue_kernel_basic(%opencl.queue_t* [[DEF_Q]], i32 
[[FLAGS]], %opencl.ndrange_t* [[NDR]], i8* [[BL_I8]])
   enqueue_kernel(default_queue, flags, ndrange,
@@ -32,7 +32,7 @@
   // CHECK: [[DEF_Q:%[0-9]+]] = load %opencl.queue_t*, %opencl.queue_t** 
%default_queue
   // CHECK: [[FLAGS:%[0-9]+]] = load i32, i32* %flags
   // CHECK: [[NDR:%[0-9]+]] = load %opencl.ndrange_t*, %opencl.ndrange_t** 
%ndrange
-  // CHECK: [[BL:%[0-9]+]] = bitcast <{ i8*, i32, i32, i8*, 
%struct.__block_descriptor*, i32{{.*}}, i32{{.*}}, i32{{.*}} }>* %block3 to 
void ()*
+  // CHECK: [[BL:%[0-9]+]] = bitcast <{ i8*, i32, i32, i8*, 
%struct.__block_descriptor addrspace(3)*, i32{{.*}}, i32{{.*}}, i32{{.*}} }>* 
%block3 to void ()*
   // CHECK: [[BL_I8:%[0-9]+]] = bitcast void ()* [[BL]] to i8*
   // CHECK: call i32 @__enqueue_kernel_basic_events(%opencl.queue_t* 
[[DEF_Q]], i32 [[FLAGS]], %opencl.ndrange_t* [[NDR]], i32 2, 
%opencl.clk_event_t** %event_wait_list, %opencl.clk_event_t** %clk_event, i8* 
[[BL_I8]])
   enqueue_kernel(default_queue, flags, ndrange, 2, &event_wait_list, 
&clk_event,
@@ -43,7 +43,7 @@
   // CHECK: [[DEF_Q:%[0-9]+]] = load %opencl.queue_t*, %opencl.queue_t** 
%default_queue
   // CHECK: [[FLAGS:%[0-9]+]] = load i32, i32* %flags
   // CHECK: [[NDR:%[0-9]+]] = load %opencl.ndrange_t*, %opencl.ndrange_t** 
%ndrange
-  // CHECK: call i32 (%opencl.queue_t*, i32, %opencl.ndrange_t*, i8*, i32, 
...) @__enqueue_kernel_vaargs(%opencl.queue_t* [[DEF_Q]], i32 [[FLAGS]], 
%opencl.ndrange_t* [[NDR]], i8* bitcast ({ i8**, i32, i32, i8*, 
%struct.__block_descriptor* }* @__block_literal_global{{(.[0-9]+)?}} to i8*), 
i32 1, i32 256)
+  // CHECK: call i32 (%opencl.queue_t*, i32, %opencl.ndrange_t*, i8*, i32, 
...) @__enqueue_kernel_vaargs(%opencl.queue_t* [[DEF_Q]], i32 [[FLAGS]], 
%opencl.ndrange_t* [[NDR]], i8* bitcast ({ i8**, i32, i32, i8*, 
%struct.__block_descriptor addrspace(3)* }* 
@__block_literal_global{{(.[0-9]+)?}} to i8*), i32 1, i32 256)
   enqueue_kernel(default_queue, flags, ndrange,
  ^(local void *p) {
return;
@@ -54,7 +54,7 @@
   // CHECK: [[FLAGS:%[0-9]+]] = load i32, i32* %flags
   // CHECK: [[NDR:%[0-9]+]] = load %opencl.ndrange_t*, %opencl.ndrange_t** 
%ndrange
   // CHECK: [[SIZE:%[0-9]+]] = zext i8 {{%[0-9]+}} to i32
-  // CHECK: call i32 (%opencl.queue_t*, i32, %opencl.ndrange_t*, i8*, i32, 
...) @__enqueue_kernel_vaargs(%opencl.queue_t* [[DEF_Q]], i32 [[FLAGS]], 
%opencl.ndrange_t* [[NDR]], i8* bitcast ({ i8**, i32, i32, i8*, 
%struct.__block_descriptor* }* @__block_literal_global{{(.[0-9]+)?}} to i8*), 
i32 1, i32 [[SIZE]])
+  // CHECK: call i32 (%opencl.queue_t*, i32, %opencl.ndrange_t*, i8*, i32, 
...) @__enqueue_kernel_vaargs(%opencl.queue_t* [[DEF_Q]], i32 [[FLAGS]], 
%opencl.ndrange_t* [[NDR]], i8* bitcast ({ i8**, i32, i32, i8*, 
%struct.__block_descriptor addrspace(3)* }* 
@__block_literal_global{{(.[0-9]+)?}} to i8*), i32 1, i32 [[SIZE]])
   enqueue_kernel(default_queue, flags, ndrange,
  ^(local void *p) {
return;
@@ -65,7 +65,7 @@
   // CHECK: [[FLAGS:%[0-9]+]] = load i32, i32* %flags
   // CHECK: [[NDR:%[0-9]+]] = load %opencl.ndrange_t*, %opencl.ndrange_t** 
%ndrange
   // CHECK: [[AD:%arraydecay[0-9]*]] = getelementptr inbounds [1 x 
%opencl.clk_event_t*], [1 x %opencl.clk_event_t*]* %event_wait_list2, i32 0, 
i32 0
-  // CHECK: call i32 (%opencl.queue_t*, i32, %opencl.ndrange_t*, i32, 
%opencl.clk_event_t**, %opencl.clk_event_t**, i8*, i32, ...) 
@__enqueue_kernel_events_vaargs(%opencl.queue_t* [[DEF_Q]], i32 [[FLAGS]], 
%opencl.ndrange_t* [[NDR]], i32 2, %opencl.clk_event_t** [[AD]], 
%opencl.clk_event_t** %clk_event, i8* bitcast ({ i8**, i32, i32, i8*, 
%struct.__block_descriptor* }* @__block_li

Re: [PATCH] D23274: Make clang-format remove duplicate headers when sorting #includes.

2016-08-10 Thread Eric Liu via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL278206: Make clang-format remove duplicate headers when 
sorting #includes. (authored by ioeric).

Changed prior to commit:
  https://reviews.llvm.org/D23274?vs=67489&id=67492#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D23274

Files:
  cfe/trunk/lib/Format/Format.cpp
  cfe/trunk/test/Format/remove-duplicate-includes.cpp
  cfe/trunk/tools/clang-format/ClangFormat.cpp
  cfe/trunk/unittests/Format/SortIncludesTest.cpp

Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -1223,15 +1223,50 @@
   return false;
 }
 
-// Sorts a block of includes given by 'Includes' alphabetically adding the
-// necessary replacement to 'Replaces'. 'Includes' must be in strict source
-// order.
+// Returns a pair (Index, OffsetToEOL) describing the position of the cursor
+// before sorting/deduplicating. Index is the index of the include under the
+// cursor in the original set of includes. If this include has duplicates, it is
+// the index of the first of the duplicates as the others are going to be
+// removed. OffsetToEOL describes the cursor's position relative to the end of
+// its current line.
+// If `Cursor` is not on any #include, `Index` will be UINT_MAX.
+static std::pair
+FindCursorIndex(const SmallVectorImpl &Includes,
+const SmallVectorImpl &Indices, unsigned Cursor) {
+  unsigned CursorIndex = UINT_MAX;
+  unsigned OffsetToEOL = 0;
+  for (int i = 0, e = Includes.size(); i != e; ++i) {
+unsigned Start = Includes[Indices[i]].Offset;
+unsigned End = Start + Includes[Indices[i]].Text.size();
+if (!(Cursor >= Start && Cursor < End))
+  continue;
+CursorIndex = Indices[i];
+OffsetToEOL = End - Cursor;
+// Put the cursor on the only remaining #include among the duplicate
+// #includes.
+while (--i >= 0 && Includes[CursorIndex].Text == Includes[Indices[i]].Text)
+  CursorIndex = i;
+break;
+  }
+  return std::make_pair(CursorIndex, OffsetToEOL);
+}
+
+// Sorts and deduplicate a block of includes given by 'Includes' alphabetically
+// adding the necessary replacement to 'Replaces'. 'Includes' must be in strict
+// source order.
+// #include directives with the same text will be deduplicated, and only the
+// first #include in the duplicate #includes remains. If the `Cursor` is
+// provided and put on a deleted #include, it will be moved to the remaining
+// #include in the duplicate #includes.
 static void sortCppIncludes(const FormatStyle &Style,
- const SmallVectorImpl &Includes,
- ArrayRef Ranges, StringRef FileName,
- tooling::Replacements &Replaces, unsigned *Cursor) {
-  if (!affectsRange(Ranges, Includes.front().Offset,
-Includes.back().Offset + Includes.back().Text.size()))
+const SmallVectorImpl &Includes,
+ArrayRef Ranges, StringRef FileName,
+tooling::Replacements &Replaces, unsigned *Cursor) {
+  unsigned IncludesBeginOffset = Includes.front().Offset;
+  unsigned IncludesBlockSize = Includes.back().Offset +
+   Includes.back().Text.size() -
+   IncludesBeginOffset;
+  if (!affectsRange(Ranges, IncludesBeginOffset, IncludesBlockSize))
 return;
   SmallVector Indices;
   for (unsigned i = 0, e = Includes.size(); i != e; ++i)
@@ -1241,37 +1276,39 @@
 return std::tie(Includes[LHSI].Category, Includes[LHSI].Filename) <
std::tie(Includes[RHSI].Category, Includes[RHSI].Filename);
   });
+  // The index of the include on which the cursor will be put after
+  // sorting/deduplicating.
+  unsigned CursorIndex;
+  // The offset from cursor to the end of line.
+  unsigned CursorToEOLOffset;
+  if (Cursor)
+std::tie(CursorIndex, CursorToEOLOffset) =
+FindCursorIndex(Includes, Indices, *Cursor);
+
+  // Deduplicate #includes.
+  Indices.erase(std::unique(Indices.begin(), Indices.end(),
+[&](unsigned LHSI, unsigned RHSI) {
+  return Includes[LHSI].Text == Includes[RHSI].Text;
+}),
+Indices.end());
 
   // If the #includes are out of order, we generate a single replacement fixing
   // the entire block. Otherwise, no replacement is generated.
-  if (std::is_sorted(Indices.begin(), Indices.end()))
+  if (Indices.size() == Includes.size() &&
+  std::is_sorted(Indices.begin(), Indices.end()))
 return;
 
   std::string result;
-  bool CursorMoved = false;
   for (unsigned Index : Indices) {
 if (!result.empty())
   result += "\n";
 result += Includes[Index].Text;
-
-if (Cursor && !CursorMoved) {
-  unsigned Start = Includes[Index].Offset;
-  u

r278208 - [X86][AVX] Ensure we only match against 1-byte alignment

2016-08-10 Thread Simon Pilgrim via cfe-commits
Author: rksimon
Date: Wed Aug 10 04:59:49 2016
New Revision: 278208

URL: http://llvm.org/viewvc/llvm-project?rev=278208&view=rev
Log:
[X86][AVX] Ensure we only match against 1-byte alignment

Modified:
cfe/trunk/test/CodeGen/avx-builtins.c

Modified: cfe/trunk/test/CodeGen/avx-builtins.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/avx-builtins.c?rev=278208&r1=278207&r2=278208&view=diff
==
--- cfe/trunk/test/CodeGen/avx-builtins.c (original)
+++ cfe/trunk/test/CodeGen/avx-builtins.c Wed Aug 10 04:59:49 2016
@@ -84,14 +84,14 @@ __m256 test_mm256_blendv_ps(__m256 V1, _
 
 __m256d test_mm256_broadcast_pd(__m128d* A) {
   // CHECK-LABEL: test_mm256_broadcast_pd
-  // CHECK: load <2 x double>, <2 x double>* %{{.*}}, align 1
+  // CHECK: load <2 x double>, <2 x double>* %{{.*}}, align 1{{$}}
   // CHECK: shufflevector <2 x double> %{{.*}}, <2 x double> %{{.*}}, <4 x 
i32> 
   return _mm256_broadcast_pd(A);
 }
 
 __m256 test_mm256_broadcast_ps(__m128* A) {
   // CHECK-LABEL: test_mm256_broadcast_ps
-  // CHECK: load <4 x float>, <4 x float>* %{{.*}}, align 1
+  // CHECK: load <4 x float>, <4 x float>* %{{.*}}, align 1{{$}}
   // CHECK: shufflevector <4 x float> %{{.*}}, <4 x float> %{{.*}}, <8 x i32> 

   return _mm256_broadcast_ps(A);
 }


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D17815: [libc++abi] Use fallback_malloc to allocate __cxa_eh_globals in case of dynamic memory exhaustion.

2016-08-10 Thread Igor Kudrin via cfe-commits
ikudrin added a comment.

Hi,

Ping.

The issue in the current implementation of the libc++abi library looks like a 
time bomb. Even though its probability could be considered as very low, in 
fact, it depends on the type of an application. At least, we ran into it in our 
environment.

I'd guess that no one would expect their program to terminate when a legal 
exception is thrown, or when memory is allocated. I'd like this issue to be 
fixed in one or another way.

@EricWF, can you look at the patch again, please?


https://reviews.llvm.org/D17815



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

2016-08-10 Thread Artem Dergachev via cfe-commits
NoQ added a comment.

Whoops, forgot to answer:

In https://reviews.llvm.org/D23112#508333, @xazax.hun wrote:

> I am not sure that the checker is the appropriate way to fix the remaining 
> issue with this checker.


Yeah, there are anyway more problems that require this functionality in the 
`RangeConstraintManager` (the code throws complicated equations into it anyway, 
and there are false positives reported), so i think we should try to fix it 
there anyway, some day.


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] D15332: new clang-tidy checker readability-non-const-parameter

2016-08-10 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki updated this revision to Diff 67500.
danielmarjamaki added a comment.

Fix patch so it can be applied in latest trunk. Tried to fix review comments.


https://reviews.llvm.org/D15332

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/NonConstParameterCheck.cpp
  clang-tidy/readability/NonConstParameterCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-non-const-parameter.rst
  test/clang-tidy/readability-non-const-parameter.cpp

Index: test/clang-tidy/readability-non-const-parameter.cpp
===
--- test/clang-tidy/readability-non-const-parameter.cpp
+++ test/clang-tidy/readability-non-const-parameter.cpp
@@ -0,0 +1,251 @@
+// RUN: %check_clang_tidy %s readability-non-const-parameter %t
+
+// Currently the checker only warns about pointer arguments.
+//
+// It can be defined both that the data is const and that the pointer is const,
+// the checker only checks if the data can be const-specified.
+//
+// It does not warn about pointers to records or function pointers.
+
+// Some external function where first argument is nonconst and second is const.
+char *strcpy1(char *dest, const char *src);
+unsigned my_strcpy(char *buf, const char *s);
+unsigned my_strlen(const char *buf);
+
+// CHECK-MESSAGES: :[[@LINE+1]]:29: warning: pointer parameter 'last' can be pointer to const [readability-non-const-parameter]
+void warn1(int *first, int *last) {
+  // CHECK-FIXES: {{^}}void warn1(int *first, const int *last) {{{$}}
+  *first = 0;
+  if (first < last) {
+  } // <- last can be const
+}
+
+// TODO: warning should be written here
+void warn2(char *p) {
+  char buf[10];
+  strcpy1(buf, p);
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:19: warning: pointer parameter 'p' can be
+void assign1(int *p) {
+  // CHECK-FIXES: {{^}}void assign1(const int *p) {{{$}}
+  const int *q;
+  q = p;
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:19: warning: pointer parameter 'p' can be
+void assign2(int *p) {
+  // CHECK-FIXES: {{^}}void assign2(const int *p) {{{$}}
+  const int *q;
+  q = p + 1;
+}
+
+void assign3(int *p) {
+  *p = 0;
+}
+
+void assign4(int *p) {
+  *p += 2;
+}
+
+void assign5(char *p) {
+  p[0] = 0;
+}
+
+void assign6(int *p) {
+  int *q;
+  q = p++;
+}
+
+void assign7(char *p) {
+  char *a, *b;
+  a = b = p;
+}
+
+void assign8(char *a, char *b) {
+  char *x;
+  x = (a ? a : b);
+}
+
+void assign9(unsigned char *str, const unsigned int i) {
+  unsigned char *p;
+  for (p = str + i; *p;) {
+  }
+}
+
+void assign10(int *buf) {
+  int i, *p;
+  for (i = 0, p = buf; i < 10; i++, p++) {
+*p = 1;
+  }
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:17: warning: pointer parameter 'p' can be
+void init1(int *p) {
+  // CHECK-FIXES: {{^}}void init1(const int *p) {{{$}}
+  const int *q = p;
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:17: warning: pointer parameter 'p' can be
+void init2(int *p) {
+  // CHECK-FIXES: {{^}}void init2(const int *p) {{{$}}
+  const int *q = p + 1;
+}
+
+void init3(int *p) {
+  int *q = p;
+}
+
+void init4(float *p) {
+  int *q = (int *)p;
+}
+
+void init5(int *p) {
+  int *i = p ? p : 0;
+}
+
+void init6(int *p) {
+  int *a[] = {p, p, 0};
+}
+
+void init7(int *p, int x) {
+  for (int *q = p + x - 1; 0; q++)
+;
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:18: warning: pointer parameter 'p' can be
+int return1(int *p) {
+  // CHECK-FIXES: {{^}}int return1(const int *p) {{{$}}
+  return *p;
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:25: warning: pointer parameter 'p' can be
+const int *return2(int *p) {
+  // CHECK-FIXES: {{^}}const int *return2(const int *p) {{{$}}
+  return p;
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:25: warning: pointer parameter 'p' can be
+const int *return3(int *p) {
+  // CHECK-FIXES: {{^}}const int *return3(const int *p) {{{$}}
+  return p + 1;
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:27: warning: pointer parameter 'p' can be
+const char *return4(char *p) {
+  // CHECK-FIXES: {{^}}const char *return4(const char *p) {{{$}}
+  return p ? p : "";
+}
+
+char *return5(char *s) {
+  return s;
+}
+
+char *return6(char *s) {
+  return s + 1;
+}
+
+char *return7(char *a, char *b) {
+  return a ? a : b;
+}
+
+char return8(int *p) {
+  return ++(*p);
+}
+
+void dontwarn1(int *p) {
+  ++(*p);
+}
+
+void dontwarn2(int *p) {
+  (*p)++;
+}
+
+int dontwarn3(_Atomic(int) * p) {
+  return *p;
+}
+
+void callFunction1(char *p) {
+  strcpy1(p, "abc");
+}
+
+void callFunction2(char *p) {
+  strcpy1(&p[0], "abc");
+}
+
+void callFunction3(char *p) {
+  strcpy1(p + 2, "abc");
+}
+
+char *callFunction4(char *p) {
+  return strcpy1(p, "abc");
+}
+
+unsigned callFunction5(char *buf) {
+  unsigned len = my_strlen(buf);
+  return len + my_strcpy(buf, "abc");
+}
+
+void f6(int **p);
+void callFunction6(int *p) { f6(&p); }
+
+typedef union { void *v; } t;
+void f7(t obj);
+void callFunction7(int *p) {
+  f7((t){p});
+}
+
+void f8(int &x);
+void callFunction8(int *p) {
+  

Re: [PATCH] D15332: new clang-tidy checker readability-non-const-parameter

2016-08-10 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki marked 10 inline comments as done.


Comment at: test/clang-tidy/readability-non-const-parameter.cpp:117-135
@@ +116,21 @@
+// CHECK-MESSAGES: :[[@LINE+1]]:18: warning: pointer parameter 'p' can be
+int return1(int *p) {
+  // CHECK-FIXES: {{^}}int return1(const int *p) {{{$}}
+  return *p;
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:25: warning: pointer parameter 'p' can be
+const int *return2(int *p) {
+  // CHECK-FIXES: {{^}}const int *return2(const int *p) {{{$}}
+  return p;
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:25: warning: pointer parameter 'p' can be
+const int *return3(int *p) {
+  // CHECK-FIXES: {{^}}const int *return3(const int *p) {{{$}}
+  return p + 1;
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:27: warning: pointer parameter 'p' can be
+const char *return4(char *p) {
+  // CHECK-FIXES: {{^}}const char *return4(const char *p) {{{$}}

I have changed the message now. pointer parameter 'p' can be pointer to const

The name is the same. Do you think it would be better with 
PointerParameterConstnessCheck() perhaps?


https://reviews.llvm.org/D15332



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D23346: [OpenCL] Change block descriptor address space to constant

2016-08-10 Thread Anastasia Stulova via cfe-commits
Anastasia accepted this revision.
Anastasia added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rL LLVM

https://reviews.llvm.org/D23346



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D23158: [clang-rename] merge tests when possible

2016-08-10 Thread Kirill Bobyrev via cfe-commits
omtcyfz updated this revision to Diff 67501.
omtcyfz added a comment.

Address comments.


https://reviews.llvm.org/D23158

Files:
  test/clang-rename/ClassAsTemplateArgument.cpp
  test/clang-rename/ClassFindByName.cpp
  test/clang-rename/ClassSimpleRenaming.cpp
  test/clang-rename/ClassTestMulti.cpp
  test/clang-rename/ClassTestMultiByName.cpp
  test/clang-rename/ComplexFunctionOverride.cpp
  test/clang-rename/ComplicatedClassType.cpp
  test/clang-rename/ConstructExpr.cpp
  test/clang-rename/CtorInitializer.cpp
  test/clang-rename/DeclRefExpr.cpp
  test/clang-rename/DtorDeclaration.cpp
  test/clang-rename/DtorDefinition.cpp
  test/clang-rename/FunctionMacro.cpp
  test/clang-rename/FunctionOverride.cpp
  test/clang-rename/FunctionWithClassFindByName.cpp
  test/clang-rename/MemberExprMacro.cpp
  test/clang-rename/Namespace.cpp
  test/clang-rename/TemplateClassInstantiation.cpp
  test/clang-rename/TemplateTypename.cpp
  test/clang-rename/UserDefinedConversion.cpp
  test/clang-rename/Variable.cpp
  test/clang-rename/VariableMacro.cpp

Index: test/clang-rename/VariableMacro.cpp
===
--- test/clang-rename/VariableMacro.cpp
+++ test/clang-rename/VariableMacro.cpp
@@ -1,18 +1,21 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=208 -new-name=Z %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
-#define Y X // CHECK: #define Y Z
+#define Baz Foo // CHECK: #define Baz Bar
 
 void foo(int value) {}
 
 void macro() {
-  int X;// CHECK: int Z;
-  X = 42;   // CHECK: Z = 42;
-  Y -= 0;
-  foo(X);   // CHECK: foo(Z);
-  foo(Y);
+  int Foo;  /* Test 1 */  // CHECK: int Bar;
+  Foo = 42; /* Test 2 */  // CHECK: Bar = 42;
+  Baz -= 0;
+  foo(Foo); /* Test 3 */  // CHECK: foo(Bar);
+  foo(Baz);
 }
 
-// Use grep -FUbo 'foo;'  to get the correct offset of foo when changing
-// this file.
+// Test 1.
+// RUN: clang-rename -offset=88 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// Test 2.
+// RUN: clang-rename -offset=129 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// Test 3.
+// RUN: clang-rename -offset=191 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+
+// To find offsets after modifying the file, use:
+//   grep -Ubo 'Foo.*' 
Index: test/clang-rename/Variable.cpp
===
--- test/clang-rename/Variable.cpp
+++ test/clang-rename/Variable.cpp
@@ -1,27 +1,32 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=148 -new-name=Bar %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
 namespace A {
-int Foo;// CHECK: int Bar;
+int Foo;  /* Test 1 */// CHECK: int Bar;
 }
-int Foo;// CHECK: int Foo;
-int Qux = Foo;  // CHECK: int Qux = Foo;
-int Baz = A::Foo;   // CHECK: Baz = A::Bar;
+int Foo;  // CHECK: int Foo;
+int Qux = Foo;// CHECK: int Qux = Foo;
+int Baz = A::Foo; /* Test 2 */// CHECK: Baz = A::Bar;
 void fun() {
   struct {
-int Foo;// CHECK: int Foo;
+int Foo;  // CHECK: int Foo;
   } b = {100};
-  int Foo = 100;// CHECK: int Foo = 100;
-  Baz = Foo;// CHECK: Baz = Foo;
+  int Foo = 100;  // CHECK: int Foo = 100;
+  Baz = Foo;  // CHECK: Baz = Foo;
   {
-extern int Foo; // CHECK: extern int Foo;
-Baz = Foo;  // CHECK: Baz = Foo;
-Foo = A::Foo + Baz; // CHECK: Foo = A::Bar + Baz;
-A::Foo = b.Foo; // CHECK: A::Bar = b.Foo;
+extern int Foo;   // CHECK: extern int Foo;
+Baz = Foo;// CHECK: Baz = Foo;
+Foo = A::Foo /* Test 3 */ + Baz;  // CHECK: Foo = A::Bar /* Test 3 */ + Baz;
+A::Foo /* Test 4 */ = b.Foo;  // CHECK: A::Bar /* Test 4 */ = b.Foo;
   }
-  Foo = b.Foo;  // Foo = b.Foo;
+  Foo = b.Foo;// Foo = b.Foo;
 }
 
-// Use grep -FUbo 'Foo'  to get the correct offset of foo when changing
-// this file.
+// Test 1.
+// RUN: clang-rename -offset=18 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// Test 2.
+// RUN: clang-rename -offset=206 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// Test 3.
+// RUN: clang-rename -offset=613 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// Test 4.
+// RUN: clang-rename -offset=688 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+
+// To find offsets after modifying the file, use:
+//   grep -Ubo 'Foo.*' 
Index: test/clang-rename/UserDefinedConversion.cpp
===
--- test/clang-rename/UserDefinedConversion.cpp
+++ test/clang-rename/UserDefinedConversion.cpp
@@ -1,13 +1,26 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=205 -new-name=Bar %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
-class Foo {}; // CHECK: class Bar {};
+class Foo {   /* Test 1 

Re: r278139 - [clang-cl] Make -gline-tables-only imply -gcodeview

2016-08-10 Thread Nico Weber via cfe-commits
Since this flag is new in 3.9, should we merge this there?

On Tue, Aug 9, 2016 at 1:23 PM, Reid Kleckner via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rnk
> Date: Tue Aug  9 12:23:56 2016
> New Revision: 278139
>
> URL: http://llvm.org/viewvc/llvm-project?rev=278139&view=rev
> Log:
> [clang-cl] Make -gline-tables-only imply -gcodeview
>
> It's surprising that you have to pass /Z7 in addition to -gcodeview to
> get debug info. The sanitizer runtime, for example, expects that if the
> compiler supports the -gline-tables-only flag, then it will emit debug
> info.
>
> Modified:
> cfe/trunk/lib/Driver/Tools.cpp
> cfe/trunk/test/Driver/cl-options.c
>
> Modified: cfe/trunk/lib/Driver/Tools.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/
> Tools.cpp?rev=278139&r1=278138&r2=278139&view=diff
> 
> ==
> --- cfe/trunk/lib/Driver/Tools.cpp (original)
> +++ cfe/trunk/lib/Driver/Tools.cpp Tue Aug  9 12:23:56 2016
> @@ -4611,8 +4611,8 @@ void Clang::ConstructJob(Compilation &C,
> options::OPT_gdwarf_4,
> options::OPT_gdwarf_5))
>  DwarfVersion = DwarfVersionNum(A->getSpelling());
>
> -  // Forward -gcodeview.
> -  // 'EmitCodeView might have been set by CL-compatibility argument
> parsing.
> +  // Forward -gcodeview. EmitCodeView might have been set by
> CL-compatibility
> +  // argument parsing.
>if (Args.hasArg(options::OPT_gcodeview) || EmitCodeView) {
>  // DwarfVersion remains at 0 if no explicit choice was made.
>  CmdArgs.push_back("-gcodeview");
> @@ -6402,9 +6402,10 @@ void Clang::AddClangCLArgs(const ArgList
>  CmdArgs.push_back(Args.MakeArgString(Twine(LangOptions::SSPStrong)));
>}
>
> -  // Emit CodeView if -Z7 or -Zd are present.
> +  // Emit CodeView if -Z7, -Zd, or -gline-tables-only are present.
>if (Arg *DebugInfoArg =
> -  Args.getLastArg(options::OPT__SLASH_Z7,
> options::OPT__SLASH_Zd)) {
> +  Args.getLastArg(options::OPT__SLASH_Z7, options::OPT__SLASH_Zd,
> +  options::OPT_gline_tables_only)) {
>  *EmitCodeView = true;
>  if (DebugInfoArg->getOption().matches(options::OPT__SLASH_Z7))
>*DebugInfoKind = codegenoptions::LimitedDebugInfo;
>
> Modified: cfe/trunk/test/Driver/cl-options.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/
> cl-options.c?rev=278139&r1=278138&r2=278139&view=diff
> 
> ==
> --- cfe/trunk/test/Driver/cl-options.c (original)
> +++ cfe/trunk/test/Driver/cl-options.c Tue Aug  9 12:23:56 2016
> @@ -424,6 +424,10 @@
>  // Z7GMLT: "-gcodeview"
>  // Z7GMLT: "-debug-info-kind=line-tables-only"
>
> +// RUN: %clang_cl -gline-tables-only /c -### -- %s 2>&1 | FileCheck
> -check-prefix=ZGMLT %s
> +// ZGMLT: "-gcodeview"
> +// ZGMLT: "-debug-info-kind=line-tables-only"
> +
>  // RUN: %clang_cl /c -### -- %s 2>&1 | FileCheck
> -check-prefix=BreproDefault %s
>  // BreproDefault: "-mincremental-linker-compatible"
>
>
>
> ___
> 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


r278213 - [ASTMatchers] Extend documentation for match()

2016-08-10 Thread Martin Bohme via cfe-commits
Author: mboehme
Date: Wed Aug 10 06:22:57 2016
New Revision: 278213

URL: http://llvm.org/viewvc/llvm-project?rev=278213&view=rev
Log:
[ASTMatchers] Extend documentation for match()

Summary:
Adds an explanation of how to use findAll() to find all matches in a subtree.

Modified:
cfe/trunk/include/clang/ASTMatchers/ASTMatchFinder.h

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchFinder.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchFinder.h?rev=278213&r1=278212&r2=278213&view=diff
==
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchFinder.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchFinder.h Wed Aug 10 06:22:57 
2016
@@ -229,6 +229,10 @@ private:
 /// Multiple results occur when using matchers like \c forEachDescendant,
 /// which generate a result for each sub-match.
 ///
+/// If you want to find all matches on the sub-tree rooted at \c Node (rather
+/// than only the matches on \c Node itself), surround the \c Matcher with a
+/// \c findAll().
+///
 /// \see selectFirst
 /// @{
 template 


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D15332: new clang-tidy checker readability-non-const-parameter

2016-08-10 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki updated this revision to Diff 67504.
danielmarjamaki added a comment.

fixed more review comments


https://reviews.llvm.org/D15332

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/NonConstParameterCheck.cpp
  clang-tidy/readability/NonConstParameterCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-non-const-parameter.rst
  test/clang-tidy/readability-non-const-parameter.cpp

Index: test/clang-tidy/readability-non-const-parameter.cpp
===
--- test/clang-tidy/readability-non-const-parameter.cpp
+++ test/clang-tidy/readability-non-const-parameter.cpp
@@ -0,0 +1,283 @@
+// RUN: %check_clang_tidy %s readability-non-const-parameter %t
+
+// Currently the checker only warns about pointer arguments.
+//
+// It can be defined both that the data is const and that the pointer is const,
+// the checker only checks if the data can be const-specified.
+//
+// It does not warn about pointers to records or function pointers.
+
+// Some external function where first argument is nonconst and second is const.
+char *strcpy1(char *dest, const char *src);
+unsigned my_strcpy(char *buf, const char *s);
+unsigned my_strlen(const char *buf);
+
+// CHECK-MESSAGES: :[[@LINE+1]]:29: warning: pointer parameter 'last' can be pointer to const [readability-non-const-parameter]
+void warn1(int *first, int *last) {
+  // CHECK-FIXES: {{^}}void warn1(int *first, const int *last) {{{$}}
+  *first = 0;
+  if (first < last) {
+  } // <- last can be const
+}
+
+// TODO: warning should be written here
+void warn2(char *p) {
+  char buf[10];
+  strcpy1(buf, p);
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:19: warning: pointer parameter 'p' can be
+void assign1(int *p) {
+  // CHECK-FIXES: {{^}}void assign1(const int *p) {{{$}}
+  const int *q;
+  q = p;
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:19: warning: pointer parameter 'p' can be
+void assign2(int *p) {
+  // CHECK-FIXES: {{^}}void assign2(const int *p) {{{$}}
+  const int *q;
+  q = p + 1;
+}
+
+void assign3(int *p) {
+  *p = 0;
+}
+
+void assign4(int *p) {
+  *p += 2;
+}
+
+void assign5(char *p) {
+  p[0] = 0;
+}
+
+void assign6(int *p) {
+  int *q;
+  q = p++;
+}
+
+void assign7(char *p) {
+  char *a, *b;
+  a = b = p;
+}
+
+void assign8(char *a, char *b) {
+  char *x;
+  x = (a ? a : b);
+}
+
+void assign9(unsigned char *str, const unsigned int i) {
+  unsigned char *p;
+  for (p = str + i; *p;) {
+  }
+}
+
+void assign10(int *buf) {
+  int i, *p;
+  for (i = 0, p = buf; i < 10; i++, p++) {
+*p = 1;
+  }
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:17: warning: pointer parameter 'p' can be
+void init1(int *p) {
+  // CHECK-FIXES: {{^}}void init1(const int *p) {{{$}}
+  const int *q = p;
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:17: warning: pointer parameter 'p' can be
+void init2(int *p) {
+  // CHECK-FIXES: {{^}}void init2(const int *p) {{{$}}
+  const int *q = p + 1;
+}
+
+void init3(int *p) {
+  int *q = p;
+}
+
+void init4(float *p) {
+  int *q = (int *)p;
+}
+
+void init5(int *p) {
+  int *i = p ? p : 0;
+}
+
+void init6(int *p) {
+  int *a[] = {p, p, 0};
+}
+
+void init7(int *p, int x) {
+  for (int *q = p + x - 1; 0; q++)
+;
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:18: warning: pointer parameter 'p' can be
+int return1(int *p) {
+  // CHECK-FIXES: {{^}}int return1(const int *p) {{{$}}
+  return *p;
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:25: warning: pointer parameter 'p' can be
+const int *return2(int *p) {
+  // CHECK-FIXES: {{^}}const int *return2(const int *p) {{{$}}
+  return p;
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:25: warning: pointer parameter 'p' can be
+const int *return3(int *p) {
+  // CHECK-FIXES: {{^}}const int *return3(const int *p) {{{$}}
+  return p + 1;
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:27: warning: pointer parameter 'p' can be
+const char *return4(char *p) {
+  // CHECK-FIXES: {{^}}const char *return4(const char *p) {{{$}}
+  return p ? p : "";
+}
+
+char *return5(char *s) {
+  return s;
+}
+
+char *return6(char *s) {
+  return s + 1;
+}
+
+char *return7(char *a, char *b) {
+  return a ? a : b;
+}
+
+char return8(int *p) {
+  return ++(*p);
+}
+
+void dontwarn1(int *p) {
+  ++(*p);
+}
+
+void dontwarn2(int *p) {
+  (*p)++;
+}
+
+int dontwarn3(_Atomic(int) * p) {
+  return *p;
+}
+
+void callFunction1(char *p) {
+  strcpy1(p, "abc");
+}
+
+void callFunction2(char *p) {
+  strcpy1(&p[0], "abc");
+}
+
+void callFunction3(char *p) {
+  strcpy1(p + 2, "abc");
+}
+
+char *callFunction4(char *p) {
+  return strcpy1(p, "abc");
+}
+
+unsigned callFunction5(char *buf) {
+  unsigned len = my_strlen(buf);
+  return len + my_strcpy(buf, "abc");
+}
+
+void f6(int **p);
+void callFunction6(int *p) { f6(&p); }
+
+typedef union { void *v; } t;
+void f7(t obj);
+void callFunction7(int *p) {
+  f7((t){p});
+}
+
+void f8(int &x);
+void callFunction8(int *p) {
+  f8(*p);
+}
+
+// Don't warn about nonconst function

r278209 - [X86][AVX512] lower __mm512_andnot_ps/__mm512_andnot_pd to IR

2016-08-10 Thread Lama Saba via cfe-commits
Author: lsaba
Date: Wed Aug 10 05:34:45 2016
New Revision: 278209

URL: http://llvm.org/viewvc/llvm-project?rev=278209&view=rev
Log:
[X86][AVX512] lower __mm512_andnot_ps/__mm512_andnot_pd to IR
Differential revision: https://reviews.llvm.org/D23262
 

Modified:
cfe/trunk/lib/Headers/avx512dqintrin.h
cfe/trunk/test/CodeGen/avx512dq-builtins.c

Modified: cfe/trunk/lib/Headers/avx512dqintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avx512dqintrin.h?rev=278209&r1=278208&r2=278209&view=diff
==
--- cfe/trunk/lib/Headers/avx512dqintrin.h (original)
+++ cfe/trunk/lib/Headers/avx512dqintrin.h Wed Aug 10 05:34:45 2016
@@ -187,11 +187,7 @@ _mm512_maskz_and_ps (__mmask16 __U, __m5
 
 static __inline__ __m512d __DEFAULT_FN_ATTRS
 _mm512_andnot_pd (__m512d __A, __m512d __B) {
-  return (__m512d) __builtin_ia32_andnpd512_mask ((__v8df) __A,
-  (__v8df) __B,
-  (__v8df)
-  _mm512_setzero_pd (),
-  (__mmask8) -1);
+  return (__m512d)(~(__v8du)__A & (__v8du)__B);
 }
 
 static __inline__ __m512d __DEFAULT_FN_ATTRS
@@ -213,11 +209,7 @@ _mm512_maskz_andnot_pd (__mmask8 __U, __
 
 static __inline__ __m512 __DEFAULT_FN_ATTRS
 _mm512_andnot_ps (__m512 __A, __m512 __B) {
-  return (__m512) __builtin_ia32_andnps512_mask ((__v16sf) __A,
- (__v16sf) __B,
- (__v16sf)
- _mm512_setzero_ps (),
- (__mmask16) -1);
+  return (__m512)(~(__v16su)__A & (__v16su)__B);
 }
 
 static __inline__ __m512 __DEFAULT_FN_ATTRS

Modified: cfe/trunk/test/CodeGen/avx512dq-builtins.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/avx512dq-builtins.c?rev=278209&r1=278208&r2=278209&view=diff
==
--- cfe/trunk/test/CodeGen/avx512dq-builtins.c (original)
+++ cfe/trunk/test/CodeGen/avx512dq-builtins.c Wed Aug 10 05:34:45 2016
@@ -133,7 +133,8 @@ __m512 test_mm512_maskz_and_ps (__mmask1
 
 __m512d test_mm512_andnot_pd (__m512d __A, __m512d __B) {
   // CHECK-LABEL: @test_mm512_andnot_pd
-  // CHECK: @llvm.x86.avx512.mask.andn.pd.512
+  // CHECK: xor <8 x i64> %{{.*}}, 
+  // CHECK: and <8 x i64>
   return (__m512d) _mm512_andnot_pd(__A, __B);
 }
 
@@ -151,7 +152,8 @@ __m512d test_mm512_maskz_andnot_pd (__mm
 
 __m512 test_mm512_andnot_ps (__m512 __A, __m512 __B) {
   // CHECK-LABEL: @test_mm512_andnot_ps
-  // CHECK: @llvm.x86.avx512.mask.andn.ps.512
+  // CHECK: xor <16 x i32> %{{.*}}, 
+  // CHECK: and <16 x i32>
   return (__m512) _mm512_andnot_ps(__A, __B);
 }
 


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D23262: lower __mm512_andnot_ps/__mm512_andnot_pd to IR

2016-08-10 Thread Lama via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL278209: [X86][AVX512] lower 
__mm512_andnot_ps/__mm512_andnot_pd to IR (authored by lsaba).

Changed prior to commit:
  https://reviews.llvm.org/D23262?vs=67152&id=67502#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D23262

Files:
  cfe/trunk/lib/Headers/avx512dqintrin.h
  cfe/trunk/test/CodeGen/avx512dq-builtins.c

Index: cfe/trunk/test/CodeGen/avx512dq-builtins.c
===
--- cfe/trunk/test/CodeGen/avx512dq-builtins.c
+++ cfe/trunk/test/CodeGen/avx512dq-builtins.c
@@ -133,7 +133,8 @@
 
 __m512d test_mm512_andnot_pd (__m512d __A, __m512d __B) {
   // CHECK-LABEL: @test_mm512_andnot_pd
-  // CHECK: @llvm.x86.avx512.mask.andn.pd.512
+  // CHECK: xor <8 x i64> %{{.*}}, 
+  // CHECK: and <8 x i64>
   return (__m512d) _mm512_andnot_pd(__A, __B);
 }
 
@@ -151,7 +152,8 @@
 
 __m512 test_mm512_andnot_ps (__m512 __A, __m512 __B) {
   // CHECK-LABEL: @test_mm512_andnot_ps
-  // CHECK: @llvm.x86.avx512.mask.andn.ps.512
+  // CHECK: xor <16 x i32> %{{.*}}, 
+  // CHECK: and <16 x i32>
   return (__m512) _mm512_andnot_ps(__A, __B);
 }
 
Index: cfe/trunk/lib/Headers/avx512dqintrin.h
===
--- cfe/trunk/lib/Headers/avx512dqintrin.h
+++ cfe/trunk/lib/Headers/avx512dqintrin.h
@@ -187,11 +187,7 @@
 
 static __inline__ __m512d __DEFAULT_FN_ATTRS
 _mm512_andnot_pd (__m512d __A, __m512d __B) {
-  return (__m512d) __builtin_ia32_andnpd512_mask ((__v8df) __A,
-  (__v8df) __B,
-  (__v8df)
-  _mm512_setzero_pd (),
-  (__mmask8) -1);
+  return (__m512d)(~(__v8du)__A & (__v8du)__B);
 }
 
 static __inline__ __m512d __DEFAULT_FN_ATTRS
@@ -213,11 +209,7 @@
 
 static __inline__ __m512 __DEFAULT_FN_ATTRS
 _mm512_andnot_ps (__m512 __A, __m512 __B) {
-  return (__m512) __builtin_ia32_andnps512_mask ((__v16sf) __A,
- (__v16sf) __B,
- (__v16sf)
- _mm512_setzero_ps (),
- (__mmask16) -1);
+  return (__m512)(~(__v16su)__A & (__v16su)__B);
 }
 
 static __inline__ __m512 __DEFAULT_FN_ATTRS


Index: cfe/trunk/test/CodeGen/avx512dq-builtins.c
===
--- cfe/trunk/test/CodeGen/avx512dq-builtins.c
+++ cfe/trunk/test/CodeGen/avx512dq-builtins.c
@@ -133,7 +133,8 @@
 
 __m512d test_mm512_andnot_pd (__m512d __A, __m512d __B) {
   // CHECK-LABEL: @test_mm512_andnot_pd
-  // CHECK: @llvm.x86.avx512.mask.andn.pd.512
+  // CHECK: xor <8 x i64> %{{.*}}, 
+  // CHECK: and <8 x i64>
   return (__m512d) _mm512_andnot_pd(__A, __B);
 }
 
@@ -151,7 +152,8 @@
 
 __m512 test_mm512_andnot_ps (__m512 __A, __m512 __B) {
   // CHECK-LABEL: @test_mm512_andnot_ps
-  // CHECK: @llvm.x86.avx512.mask.andn.ps.512
+  // CHECK: xor <16 x i32> %{{.*}}, 
+  // CHECK: and <16 x i32>
   return (__m512) _mm512_andnot_ps(__A, __B);
 }
 
Index: cfe/trunk/lib/Headers/avx512dqintrin.h
===
--- cfe/trunk/lib/Headers/avx512dqintrin.h
+++ cfe/trunk/lib/Headers/avx512dqintrin.h
@@ -187,11 +187,7 @@
 
 static __inline__ __m512d __DEFAULT_FN_ATTRS
 _mm512_andnot_pd (__m512d __A, __m512d __B) {
-  return (__m512d) __builtin_ia32_andnpd512_mask ((__v8df) __A,
-  (__v8df) __B,
-  (__v8df)
-  _mm512_setzero_pd (),
-  (__mmask8) -1);
+  return (__m512d)(~(__v8du)__A & (__v8du)__B);
 }
 
 static __inline__ __m512d __DEFAULT_FN_ATTRS
@@ -213,11 +209,7 @@
 
 static __inline__ __m512 __DEFAULT_FN_ATTRS
 _mm512_andnot_ps (__m512 __A, __m512 __B) {
-  return (__m512) __builtin_ia32_andnps512_mask ((__v16sf) __A,
- (__v16sf) __B,
- (__v16sf)
- _mm512_setzero_ps (),
- (__mmask16) -1);
+  return (__m512)(~(__v16su)__A & (__v16su)__B);
 }
 
 static __inline__ __m512 __DEFAULT_FN_ATTRS
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D15332: new clang-tidy checker readability-non-const-parameter

2016-08-10 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki marked 2 inline comments as done.


Comment at: test/clang-tidy/readability-non-const-parameter.cpp:245
@@ +244,3 @@
+  C c(p);
+}
+

I have added tests and fixed FPs in latest diff.


Comment at: test/clang-tidy/readability-non-const-parameter.cpp:252
@@ +251,3 @@
+
+class Warn {
+public:

I have added a test in latest diff. but there is a false positive. I believe I 
heard about whole program analysis in clang-tidy sometime.. is that something 
worked on / implemented yet?



https://reviews.llvm.org/D15332



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D17990: [clang-tidy] minor improvements in modernise-deprecated-headers check

2016-08-10 Thread Alexander Kornienko via cfe-commits
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.

LG


https://reviews.llvm.org/D17990



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D15332: new clang-tidy checker readability-non-const-parameter

2016-08-10 Thread Daniel Marjamäki via cfe-commits
danielmarjamaki updated this revision to Diff 67506.
danielmarjamaki marked 2 inline comments as done.
danielmarjamaki added a comment.

added this check to the release notes. run clang-format.


https://reviews.llvm.org/D15332

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/NonConstParameterCheck.cpp
  clang-tidy/readability/NonConstParameterCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-non-const-parameter.rst
  test/clang-tidy/readability-non-const-parameter.cpp

Index: test/clang-tidy/readability-non-const-parameter.cpp
===
--- test/clang-tidy/readability-non-const-parameter.cpp
+++ test/clang-tidy/readability-non-const-parameter.cpp
@@ -0,0 +1,283 @@
+// RUN: %check_clang_tidy %s readability-non-const-parameter %t
+
+// Currently the checker only warns about pointer arguments.
+//
+// It can be defined both that the data is const and that the pointer is const,
+// the checker only checks if the data can be const-specified.
+//
+// It does not warn about pointers to records or function pointers.
+
+// Some external function where first argument is nonconst and second is const.
+char *strcpy1(char *dest, const char *src);
+unsigned my_strcpy(char *buf, const char *s);
+unsigned my_strlen(const char *buf);
+
+// CHECK-MESSAGES: :[[@LINE+1]]:29: warning: pointer parameter 'last' can be pointer to const [readability-non-const-parameter]
+void warn1(int *first, int *last) {
+  // CHECK-FIXES: {{^}}void warn1(int *first, const int *last) {{{$}}
+  *first = 0;
+  if (first < last) {
+  } // <- last can be const
+}
+
+// TODO: warning should be written here
+void warn2(char *p) {
+  char buf[10];
+  strcpy1(buf, p);
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:19: warning: pointer parameter 'p' can be
+void assign1(int *p) {
+  // CHECK-FIXES: {{^}}void assign1(const int *p) {{{$}}
+  const int *q;
+  q = p;
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:19: warning: pointer parameter 'p' can be
+void assign2(int *p) {
+  // CHECK-FIXES: {{^}}void assign2(const int *p) {{{$}}
+  const int *q;
+  q = p + 1;
+}
+
+void assign3(int *p) {
+  *p = 0;
+}
+
+void assign4(int *p) {
+  *p += 2;
+}
+
+void assign5(char *p) {
+  p[0] = 0;
+}
+
+void assign6(int *p) {
+  int *q;
+  q = p++;
+}
+
+void assign7(char *p) {
+  char *a, *b;
+  a = b = p;
+}
+
+void assign8(char *a, char *b) {
+  char *x;
+  x = (a ? a : b);
+}
+
+void assign9(unsigned char *str, const unsigned int i) {
+  unsigned char *p;
+  for (p = str + i; *p;) {
+  }
+}
+
+void assign10(int *buf) {
+  int i, *p;
+  for (i = 0, p = buf; i < 10; i++, p++) {
+*p = 1;
+  }
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:17: warning: pointer parameter 'p' can be
+void init1(int *p) {
+  // CHECK-FIXES: {{^}}void init1(const int *p) {{{$}}
+  const int *q = p;
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:17: warning: pointer parameter 'p' can be
+void init2(int *p) {
+  // CHECK-FIXES: {{^}}void init2(const int *p) {{{$}}
+  const int *q = p + 1;
+}
+
+void init3(int *p) {
+  int *q = p;
+}
+
+void init4(float *p) {
+  int *q = (int *)p;
+}
+
+void init5(int *p) {
+  int *i = p ? p : 0;
+}
+
+void init6(int *p) {
+  int *a[] = {p, p, 0};
+}
+
+void init7(int *p, int x) {
+  for (int *q = p + x - 1; 0; q++)
+;
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:18: warning: pointer parameter 'p' can be
+int return1(int *p) {
+  // CHECK-FIXES: {{^}}int return1(const int *p) {{{$}}
+  return *p;
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:25: warning: pointer parameter 'p' can be
+const int *return2(int *p) {
+  // CHECK-FIXES: {{^}}const int *return2(const int *p) {{{$}}
+  return p;
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:25: warning: pointer parameter 'p' can be
+const int *return3(int *p) {
+  // CHECK-FIXES: {{^}}const int *return3(const int *p) {{{$}}
+  return p + 1;
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:27: warning: pointer parameter 'p' can be
+const char *return4(char *p) {
+  // CHECK-FIXES: {{^}}const char *return4(const char *p) {{{$}}
+  return p ? p : "";
+}
+
+char *return5(char *s) {
+  return s;
+}
+
+char *return6(char *s) {
+  return s + 1;
+}
+
+char *return7(char *a, char *b) {
+  return a ? a : b;
+}
+
+char return8(int *p) {
+  return ++(*p);
+}
+
+void dontwarn1(int *p) {
+  ++(*p);
+}
+
+void dontwarn2(int *p) {
+  (*p)++;
+}
+
+int dontwarn3(_Atomic(int) * p) {
+  return *p;
+}
+
+void callFunction1(char *p) {
+  strcpy1(p, "abc");
+}
+
+void callFunction2(char *p) {
+  strcpy1(&p[0], "abc");
+}
+
+void callFunction3(char *p) {
+  strcpy1(p + 2, "abc");
+}
+
+char *callFunction4(char *p) {
+  return strcpy1(p, "abc");
+}
+
+unsigned callFunction5(char *buf) {
+  unsigned len = my_strlen(buf);
+  return len + my_strcpy(buf, "abc");
+}
+
+void f6(int **p);
+void callFunction6(int *p) { f6(&p); }
+
+typedef union { void *v; } t;
+void f7(t obj);
+void callFunction7(int *p) {
+  f7((t){p});
+}

Re: [PATCH] D23343: [clang-tidy] modernize-make-{smart_ptr} private ctor bugfix

2016-08-10 Thread Aaron Ballman via cfe-commits
aaron.ballman added a comment.

Good catch! Some small nits.



Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:32-33
@@ -31,1 +31,4 @@
 
+  // It is possible to make smart ptr calling private ctor inside of a member
+  // function. Change to make_smart_ptr will be invalid.
+  auto CallsPrivateCtor = has(

Nit: this reads a bit strangely, how about: `Calling make_smart_ptr from within 
a member function of a type with a private constructor would be ill-formed.`


Comment at: test/clang-tidy/modernize-make-shared.cpp:109
@@ +108,3 @@
+  void create() {
+auto ptr = std::shared_ptr(new Private(42));
+  }

Add comments explaining why make_shared is not correct. Also, please add a case 
showing that protected constructors still get the proper fix applied.


Comment at: test/clang-tidy/modernize-make-unique.cpp:112
@@ +111,3 @@
+  void create() {
+auto ptr = std::unique_ptr(new Private(42));
+  }

Add comments as above.


https://reviews.llvm.org/D23343



___
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

2016-08-10 Thread Aaron Ballman via cfe-commits
aaron.ballman added inline comments.


Comment at: docs/clang-tidy/checks/readability-else-after-return.rst:8
@@ +7,3 @@
+reduce indentation where possible and where it makes understanding code easier.
+Early exit is one of the suggested enforcement of that. Please do not use 
`else`
+or `else if` after something that interrupts control flow - like `return`,

enforcements (plural)


Comment at: docs/clang-tidy/checks/readability-else-after-return.rst:10
@@ -6,1 +9,3 @@
+or `else if` after something that interrupts control flow - like `return`,
+`break`, `continue`, `throw`, etc.
 

I would remove the "etc", since this check does not handle such cases.


Comment at: docs/clang-tidy/checks/readability-else-after-return.rst:12
@@ -6,3 +11,3 @@
 
-Flags the usages of ``else`` after ``return``.
+Following piece of code illustrates how check would work. This piece of code:
 

Following -> The following
check would work -> the check works


Comment at: docs/clang-tidy/checks/readability-else-after-return.rst:14
@@ -8,2 +13,3 @@
 
-http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return
+``` c++
+void foo(int Value) {

Since we're talking about LLVM coding conventions, can you format this like 
LLVM code (elide braces, etc)?


Comment at: docs/clang-tidy/checks/readability-else-after-return.rst:14
@@ -8,2 +13,3 @@
 
-http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return
+``` c++
+void foo(int Value) {

aaron.ballman wrote:
> Since we're talking about LLVM coding conventions, can you format this like 
> LLVM code (elide braces, etc)?
This is not the proper way to introduce a code block. Should use `.. 
code-block:: c++` and then indent the code blocks.


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] D23158: [clang-rename] merge tests when possible

2016-08-10 Thread Alexander Kornienko via cfe-commits
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.

Looks good. Thank you!


https://reviews.llvm.org/D23158



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D17990: [clang-tidy] minor improvements in modernise-deprecated-headers check

2016-08-10 Thread Aaron Ballman via cfe-commits
aaron.ballman added a subscriber: aaron.ballman.
aaron.ballman requested changes to this revision.
aaron.ballman added a reviewer: aaron.ballman.
This revision now requires changes to proceed.


Comment at: clang-tidy/modernize/DeprecatedHeadersCheck.cpp:79
@@ -69,3 +78,3 @@
 for (const auto &KeyValue :
  std::vector>(
  {{"fenv.h", "cfenv"},

This will definitely fail in MSVC 2013.


Comment at: clang-tidy/modernize/DeprecatedHeadersCheck.cpp:89
@@ +88,3 @@
+  for (const auto &Key :
+   std::vector({"stdalign.h", "stdbool.h", "iso646.h"})) {
+DeleteHeaders.insert(Key);

This one will work fine, but this pattern does not seem safe to me (same as 
above). StringSet::insert expects a StringRef. This vector (and all the 
std::string elements) will be destroyed at the end of the for loop, which means 
the StringSet will have dangling references.


https://reviews.llvm.org/D17990



___
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

2016-08-10 Thread Alexander Kornienko via cfe-commits
alexfh added inline comments.


Comment at: clang-tidy/readability/ElseAfterReturnCheck.cpp:40
@@ +39,3 @@
+  for (const auto *BindingName : {"return", "continue", "break", "throw"}) {
+if (Result.Nodes.getNodeAs(BindingName)) {
+  ControlFlowInterruptor = BindingName;

No braces around single-line `if` bodies, please. Clang-tidy code is mostly 
consistent wrt this.


Comment at: docs/clang-tidy/checks/readability-else-after-return.rst:14
@@ -8,2 +13,3 @@
 
-http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return
+``` c++
+void foo(int Value) {

aaron.ballman wrote:
> aaron.ballman wrote:
> > Since we're talking about LLVM coding conventions, can you format this like 
> > LLVM code (elide braces, etc)?
> This is not the proper way to introduce a code block. Should use `.. 
> code-block:: c++` and then indent the code blocks.
I think, the braces are fine, since the example needs to demonstrate that the 
check is able to remove them.


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] D23265: [clang-tidy] enhance readability-else-after-return

2016-08-10 Thread Aaron Ballman via cfe-commits
aaron.ballman added inline comments.


Comment at: docs/clang-tidy/checks/readability-else-after-return.rst:14
@@ -8,2 +13,3 @@
 
-http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return
+``` c++
+void foo(int Value) {

alexfh wrote:
> aaron.ballman wrote:
> > aaron.ballman wrote:
> > > Since we're talking about LLVM coding conventions, can you format this 
> > > like LLVM code (elide braces, etc)?
> > This is not the proper way to introduce a code block. Should use `.. 
> > code-block:: c++` and then indent the code blocks.
> I think, the braces are fine, since the example needs to demonstrate that the 
> check is able to remove them.
Okay, that's a fair, but subtle, reason to leave them in.


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] D23265: [clang-tidy] enhance readability-else-after-return

2016-08-10 Thread Kirill Bobyrev via cfe-commits
omtcyfz updated this revision to Diff 67512.
omtcyfz marked 4 inline comments as done.
omtcyfz added a comment.

Address comments.


https://reviews.llvm.org/D23265

Files:
  clang-tidy/readability/ElseAfterReturnCheck.cpp
  docs/clang-tidy/checks/readability-else-after-return.rst
  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
@@ -3,16 +3,16 @@
 void f(int a) {
   if (a > 0)
 return;
-  else // comment
-// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: don't use else after return
-// CHECK-FIXES: {{^}}  // comment
+  else // comment-0
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'else' after 'return'
+  // CHECK-FIXES: {{^}}  // comment-0
 return;
 
   if (a > 0) {
 return;
-  } else { // comment
-// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: don't use else after return
-// CHECK-FIXES:  } // comment
+  } else { // comment-1
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return'
+  // CHECK-FIXES: {{^}}  } // comment-1
 return;
   }
 
@@ -28,7 +28,34 @@
 f(0);
   else if (a > 10)
 return;
-  else
+  else // comment-2
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'else' after 'return'
+  // CHECK-FIXES: {{^}}  // comment-2
 f(0);
 }
 
+void foo() {
+  for (unsigned x = 0; x < 42; ++x) {
+if (x) {
+  continue;
+} else { // comment-3
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'continue'
+// CHECK-FIXES: {{^}}} // comment-3
+  x++;
+}
+if (x) {
+  break;
+} else { // comment-4
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'break'
+// CHECK-FIXES: {{^}}} // comment-4
+  x++;
+}
+if (x) {
+  throw 42;
+} else { // comment-5
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'throw'
+// CHECK-FIXES: {{^}}} // comment-5
+  x++;
+}
+  }
+}
Index: docs/clang-tidy/checks/readability-else-after-return.rst
===
--- docs/clang-tidy/checks/readability-else-after-return.rst
+++ docs/clang-tidy/checks/readability-else-after-return.rst
@@ -3,7 +3,62 @@
 readability-else-after-return
 =
 
+`LLVM Coding Standards `_ advises to
+reduce indentation where possible and where it makes understanding code easier.
+Early exit is one of the suggested enforcements of that. Please do not use
+`else` or `else if` after something that interrupts control flow - like
+`return`, `break`, `continue`, `throw`.
 
-Flags the usages of ``else`` after ``return``.
+The following piece of code illustrates how the check works. This piece of code:
 
-http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return
+.. code-block:: c++
+
+void foo(int Value) {
+  int Local = 0;
+  for (int i = 0; i < 42; i++) {
+if (Value == 1) {
+  return;
+} else {
+  Local++;
+}
+
+if (Value == 2)
+  continue;
+else
+  Local++;
+
+if (Value == 3) {
+  throw 42;
+} else {
+  Local++;
+}
+  }
+}
+
+
+Would be transformed into:
+
+.. code-block:: c++
+
+void foo(int Value) {
+  int Local = 0;
+  for (int i = 0; i < 42; i++) {
+if (Value == 1) {
+  return;
+}
+Local++;
+
+if (Value == 2)
+  continue;
+Local++;
+
+if (Value == 3) {
+  throw 42;
+}
+Local++;
+  }
+}
+
+
+This checks helps to enforce this `Coding Standars recommendation
+`_.
Index: clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -19,20 +19,31 @@
 namespace readability {
 
 void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) {
-  // FIXME: Support continue, break and throw.
+  const auto ControlFlowInterruptorMatcher =
+  stmt(anyOf(returnStmt().bind("return"), continueStmt().bind("continue"),
+ breakStmt().bind("break"), cxxThrowExpr().bind("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(ControlFlowInterruptorMatcher,
+   compoundStmt(has(ControlFlowInte

Re: [PATCH] D23265: [clang-tidy] enhance readability-else-after-return

2016-08-10 Thread Aaron Ballman via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM!


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] D23265: [clang-tidy] enhance readability-else-after-return

2016-08-10 Thread Kirill Bobyrev via cfe-commits
omtcyfz updated this revision to Diff 67513.
omtcyfz added a comment.

Remove braces around single statement in `for` and `if` as Alex proposed.


https://reviews.llvm.org/D23265

Files:
  clang-tidy/readability/ElseAfterReturnCheck.cpp
  docs/clang-tidy/checks/readability-else-after-return.rst
  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
@@ -3,16 +3,16 @@
 void f(int a) {
   if (a > 0)
 return;
-  else // comment
-// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: don't use else after return
-// CHECK-FIXES: {{^}}  // comment
+  else // comment-0
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'else' after 'return'
+  // CHECK-FIXES: {{^}}  // comment-0
 return;
 
   if (a > 0) {
 return;
-  } else { // comment
-// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: don't use else after return
-// CHECK-FIXES:  } // comment
+  } else { // comment-1
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return'
+  // CHECK-FIXES: {{^}}  } // comment-1
 return;
   }
 
@@ -28,7 +28,34 @@
 f(0);
   else if (a > 10)
 return;
-  else
+  else // comment-2
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'else' after 'return'
+  // CHECK-FIXES: {{^}}  // comment-2
 f(0);
 }
 
+void foo() {
+  for (unsigned x = 0; x < 42; ++x) {
+if (x) {
+  continue;
+} else { // comment-3
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'continue'
+// CHECK-FIXES: {{^}}} // comment-3
+  x++;
+}
+if (x) {
+  break;
+} else { // comment-4
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'break'
+// CHECK-FIXES: {{^}}} // comment-4
+  x++;
+}
+if (x) {
+  throw 42;
+} else { // comment-5
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'throw'
+// CHECK-FIXES: {{^}}} // comment-5
+  x++;
+}
+  }
+}
Index: docs/clang-tidy/checks/readability-else-after-return.rst
===
--- docs/clang-tidy/checks/readability-else-after-return.rst
+++ docs/clang-tidy/checks/readability-else-after-return.rst
@@ -3,7 +3,62 @@
 readability-else-after-return
 =
 
+`LLVM Coding Standards `_ advises to
+reduce indentation where possible and where it makes understanding code easier.
+Early exit is one of the suggested enforcements of that. Please do not use
+`else` or `else if` after something that interrupts control flow - like
+`return`, `break`, `continue`, `throw`.
 
-Flags the usages of ``else`` after ``return``.
+The following piece of code illustrates how the check works. This piece of code:
 
-http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return
+.. code-block:: c++
+
+void foo(int Value) {
+  int Local = 0;
+  for (int i = 0; i < 42; i++) {
+if (Value == 1) {
+  return;
+} else {
+  Local++;
+}
+
+if (Value == 2)
+  continue;
+else
+  Local++;
+
+if (Value == 3) {
+  throw 42;
+} else {
+  Local++;
+}
+  }
+}
+
+
+Would be transformed into:
+
+.. code-block:: c++
+
+void foo(int Value) {
+  int Local = 0;
+  for (int i = 0; i < 42; i++) {
+if (Value == 1) {
+  return;
+}
+Local++;
+
+if (Value == 2)
+  continue;
+Local++;
+
+if (Value == 3) {
+  throw 42;
+}
+Local++;
+  }
+}
+
+
+This checks helps to enforce this `Coding Standars recommendation
+`_.
Index: clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -19,20 +19,29 @@
 namespace readability {
 
 void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) {
-  // FIXME: Support continue, break and throw.
+  const auto ControlFlowInterruptorMatcher =
+  stmt(anyOf(returnStmt().bind("return"), continueStmt().bind("continue"),
+ breakStmt().bind("break"), cxxThrowExpr().bind("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(ControlFlowInterruptorMatcher,
+   compoundStmt(has(C

Re: [PATCH] D23158: [clang-rename] merge tests when possible

2016-08-10 Thread Kirill Bobyrev via cfe-commits
omtcyfz updated this revision to Diff 67515.
omtcyfz added a comment.

Update files not previouysly added with `git add`.


https://reviews.llvm.org/D23158

Files:
  test/clang-rename/ClassAsTemplateArgument.cpp
  test/clang-rename/ClassAsTemplateArgumentFindByClass.cpp
  test/clang-rename/ClassAsTemplateArgumentFindByTemplateArgument.cpp
  test/clang-rename/ClassFindByName.cpp
  test/clang-rename/ClassNameInFunctionDefinition.cpp
  test/clang-rename/ClassSimpleRenaming.cpp
  test/clang-rename/ClassTestMulti.cpp
  test/clang-rename/ClassTestMultiByName.cpp
  test/clang-rename/ClassTestMultiByNameYAML.cpp
  test/clang-rename/ComplexFunctionOverride.cpp
  test/clang-rename/ComplicatedClassType.cpp
  test/clang-rename/ConstCastExpr.cpp
  test/clang-rename/ConstructExpr.cpp
  test/clang-rename/Ctor.cpp
  test/clang-rename/CtorFindByDeclaration.cpp
  test/clang-rename/CtorFindByDefinition.cpp
  test/clang-rename/CtorInitializer.cpp
  test/clang-rename/DeclRefExpr.cpp
  test/clang-rename/DtorDeclaration.cpp
  test/clang-rename/DtorDefinition.cpp
  test/clang-rename/DynamicCastExpr.cpp
  test/clang-rename/FunctionMacro.cpp
  test/clang-rename/FunctionOverride.cpp
  test/clang-rename/FunctionWithClassFindByName.cpp
  test/clang-rename/MemberExprMacro.cpp
  test/clang-rename/Namespace.cpp
  test/clang-rename/ReinterpretCastExpr.cpp
  test/clang-rename/StaticCastExpr.cpp
  test/clang-rename/TemplateClassInstantiation.cpp
  test/clang-rename/TemplateFunctionFindByDeclaration.cpp
  test/clang-rename/TemplateFunctionFindByUse.cpp
  test/clang-rename/TemplateTypename.cpp
  test/clang-rename/UserDefinedConversion.cpp
  test/clang-rename/UserDefinedConversionFindByTypeDeclaration.cpp
  test/clang-rename/Variable.cpp
  test/clang-rename/VariableMacro.cpp

Index: test/clang-rename/VariableMacro.cpp
===
--- test/clang-rename/VariableMacro.cpp
+++ test/clang-rename/VariableMacro.cpp
@@ -1,18 +1,21 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=208 -new-name=Z %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
-#define Y X // CHECK: #define Y Z
+#define Baz Foo // CHECK: #define Baz Bar
 
 void foo(int value) {}
 
 void macro() {
-  int X;// CHECK: int Z;
-  X = 42;   // CHECK: Z = 42;
-  Y -= 0;
-  foo(X);   // CHECK: foo(Z);
-  foo(Y);
+  int Foo;  /* Test 1 */  // CHECK: int Bar;
+  Foo = 42; /* Test 2 */  // CHECK: Bar = 42;
+  Baz -= 0;
+  foo(Foo); /* Test 3 */  // CHECK: foo(Bar);
+  foo(Baz);
 }
 
-// Use grep -FUbo 'foo;'  to get the correct offset of foo when changing
-// this file.
+// Test 1.
+// RUN: clang-rename -offset=88 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// Test 2.
+// RUN: clang-rename -offset=129 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// Test 3.
+// RUN: clang-rename -offset=191 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+
+// To find offsets after modifying the file, use:
+//   grep -Ubo 'Foo.*' 
Index: test/clang-rename/Variable.cpp
===
--- test/clang-rename/Variable.cpp
+++ test/clang-rename/Variable.cpp
@@ -1,27 +1,32 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=148 -new-name=Bar %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
 namespace A {
-int Foo;// CHECK: int Bar;
+int Foo;  /* Test 1 */// CHECK: int Bar;
 }
-int Foo;// CHECK: int Foo;
-int Qux = Foo;  // CHECK: int Qux = Foo;
-int Baz = A::Foo;   // CHECK: Baz = A::Bar;
+int Foo;  // CHECK: int Foo;
+int Qux = Foo;// CHECK: int Qux = Foo;
+int Baz = A::Foo; /* Test 2 */// CHECK: Baz = A::Bar;
 void fun() {
   struct {
-int Foo;// CHECK: int Foo;
+int Foo;  // CHECK: int Foo;
   } b = {100};
-  int Foo = 100;// CHECK: int Foo = 100;
-  Baz = Foo;// CHECK: Baz = Foo;
+  int Foo = 100;  // CHECK: int Foo = 100;
+  Baz = Foo;  // CHECK: Baz = Foo;
   {
-extern int Foo; // CHECK: extern int Foo;
-Baz = Foo;  // CHECK: Baz = Foo;
-Foo = A::Foo + Baz; // CHECK: Foo = A::Bar + Baz;
-A::Foo = b.Foo; // CHECK: A::Bar = b.Foo;
+extern int Foo;   // CHECK: extern int Foo;
+Baz = Foo;// CHECK: Baz = Foo;
+Foo = A::Foo /* Test 3 */ + Baz;  // CHECK: Foo = A::Bar /* Test 3 */ + Baz;
+A::Foo /* Test 4 */ = b.Foo;  // CHECK: A::Bar /* Test 4 */ = b.Foo;
   }
-  Foo = b.Foo;  // Foo = b.Foo;
+  Foo = b.Foo;// Foo = b.Foo;
 }
 
-// Use grep -FUbo 'Foo'  to get the correct offset of foo when changing
-// this file.
+// Test 1.
+// RUN: clang-rename -offset=18 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// Test 2.
+// RUN: clang-rename -offset=206 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s

Re: [PATCH] D23158: [clang-rename] merge tests when possible

2016-08-10 Thread Kirill Bobyrev via cfe-commits
omtcyfz updated this revision to Diff 67516.
omtcyfz added a comment.

Delete two more files.


https://reviews.llvm.org/D23158

Files:
  test/clang-rename/ClassAsTemplateArgument.cpp
  test/clang-rename/ClassAsTemplateArgumentFindByClass.cpp
  test/clang-rename/ClassAsTemplateArgumentFindByTemplateArgument.cpp
  test/clang-rename/ClassFindByName.cpp
  test/clang-rename/ClassNameInFunctionDefinition.cpp
  test/clang-rename/ClassSimpleRenaming.cpp
  test/clang-rename/ClassTestMulti.cpp
  test/clang-rename/ClassTestMultiByName.cpp
  test/clang-rename/ClassTestMultiByNameYAML.cpp
  test/clang-rename/ComplexFunctionOverride.cpp
  test/clang-rename/ComplicatedClassType.cpp
  test/clang-rename/ConstCastExpr.cpp
  test/clang-rename/ConstructExpr.cpp
  test/clang-rename/Ctor.cpp
  test/clang-rename/CtorFindByDeclaration.cpp
  test/clang-rename/CtorFindByDefinition.cpp
  test/clang-rename/CtorInitializer.cpp
  test/clang-rename/DeclRefExpr.cpp
  test/clang-rename/DtorDeclaration.cpp
  test/clang-rename/DtorDefinition.cpp
  test/clang-rename/DynamicCastExpr.cpp
  test/clang-rename/FunctionMacro.cpp
  test/clang-rename/FunctionOverride.cpp
  test/clang-rename/FunctionWithClassFindByName.cpp
  test/clang-rename/MemberExprMacro.cpp
  test/clang-rename/Namespace.cpp
  test/clang-rename/ReinterpretCastExpr.cpp
  test/clang-rename/StaticCastExpr.cpp
  test/clang-rename/TemplateClassInstantiation.cpp
  test/clang-rename/TemplateFunctionFindByDeclaration.cpp
  test/clang-rename/TemplateFunctionFindByUse.cpp
  test/clang-rename/TemplateTypename.cpp
  test/clang-rename/TemplateTypenameFindByTemplateParam.cpp
  test/clang-rename/TemplateTypenameFindByTypeInside.cpp
  test/clang-rename/UserDefinedConversion.cpp
  test/clang-rename/UserDefinedConversionFindByTypeDeclaration.cpp
  test/clang-rename/Variable.cpp
  test/clang-rename/VariableMacro.cpp

Index: test/clang-rename/VariableMacro.cpp
===
--- test/clang-rename/VariableMacro.cpp
+++ test/clang-rename/VariableMacro.cpp
@@ -1,18 +1,21 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=208 -new-name=Z %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
-#define Y X // CHECK: #define Y Z
+#define Baz Foo // CHECK: #define Baz Bar
 
 void foo(int value) {}
 
 void macro() {
-  int X;// CHECK: int Z;
-  X = 42;   // CHECK: Z = 42;
-  Y -= 0;
-  foo(X);   // CHECK: foo(Z);
-  foo(Y);
+  int Foo;  /* Test 1 */  // CHECK: int Bar;
+  Foo = 42; /* Test 2 */  // CHECK: Bar = 42;
+  Baz -= 0;
+  foo(Foo); /* Test 3 */  // CHECK: foo(Bar);
+  foo(Baz);
 }
 
-// Use grep -FUbo 'foo;'  to get the correct offset of foo when changing
-// this file.
+// Test 1.
+// RUN: clang-rename -offset=88 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// Test 2.
+// RUN: clang-rename -offset=129 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// Test 3.
+// RUN: clang-rename -offset=191 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+
+// To find offsets after modifying the file, use:
+//   grep -Ubo 'Foo.*' 
Index: test/clang-rename/Variable.cpp
===
--- test/clang-rename/Variable.cpp
+++ test/clang-rename/Variable.cpp
@@ -1,27 +1,32 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=148 -new-name=Bar %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
 namespace A {
-int Foo;// CHECK: int Bar;
+int Foo;  /* Test 1 */// CHECK: int Bar;
 }
-int Foo;// CHECK: int Foo;
-int Qux = Foo;  // CHECK: int Qux = Foo;
-int Baz = A::Foo;   // CHECK: Baz = A::Bar;
+int Foo;  // CHECK: int Foo;
+int Qux = Foo;// CHECK: int Qux = Foo;
+int Baz = A::Foo; /* Test 2 */// CHECK: Baz = A::Bar;
 void fun() {
   struct {
-int Foo;// CHECK: int Foo;
+int Foo;  // CHECK: int Foo;
   } b = {100};
-  int Foo = 100;// CHECK: int Foo = 100;
-  Baz = Foo;// CHECK: Baz = Foo;
+  int Foo = 100;  // CHECK: int Foo = 100;
+  Baz = Foo;  // CHECK: Baz = Foo;
   {
-extern int Foo; // CHECK: extern int Foo;
-Baz = Foo;  // CHECK: Baz = Foo;
-Foo = A::Foo + Baz; // CHECK: Foo = A::Bar + Baz;
-A::Foo = b.Foo; // CHECK: A::Bar = b.Foo;
+extern int Foo;   // CHECK: extern int Foo;
+Baz = Foo;// CHECK: Baz = Foo;
+Foo = A::Foo /* Test 3 */ + Baz;  // CHECK: Foo = A::Bar /* Test 3 */ + Baz;
+A::Foo /* Test 4 */ = b.Foo;  // CHECK: A::Bar /* Test 4 */ = b.Foo;
   }
-  Foo = b.Foo;  // Foo = b.Foo;
+  Foo = b.Foo;// Foo = b.Foo;
 }
 
-// Use grep -FUbo 'Foo'  to get the correct offset of foo when changing
-// this file.
+// Test 1.
+// RUN: clang-rename -offset=18 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// Test 

[clang-tools-extra] r278221 - [clang-rename] merge tests when possible

2016-08-10 Thread Kirill Bobyrev via cfe-commits
Author: omtcyfz
Date: Wed Aug 10 08:28:30 2016
New Revision: 278221

URL: http://llvm.org/viewvc/llvm-project?rev=278221&view=rev
Log:
[clang-rename] merge tests when possible

The only difference between some tests is -offset passed to clang-rename. It
makes sense to merge them into a single file and add multiple tool invocations.

Reviewers: alexfh

Differential Revision: https://reviews.llvm.org/D23158

Added:
clang-tools-extra/trunk/test/clang-rename/ClassAsTemplateArgument.cpp
clang-tools-extra/trunk/test/clang-rename/Ctor.cpp
Modified:
clang-tools-extra/trunk/test/clang-rename/ClassFindByName.cpp
clang-tools-extra/trunk/test/clang-rename/ClassSimpleRenaming.cpp
clang-tools-extra/trunk/test/clang-rename/ClassTestMulti.cpp
clang-tools-extra/trunk/test/clang-rename/ClassTestMultiByName.cpp
clang-tools-extra/trunk/test/clang-rename/ClassTestMultiByNameYAML.cpp
clang-tools-extra/trunk/test/clang-rename/ComplexFunctionOverride.cpp
clang-tools-extra/trunk/test/clang-rename/ComplicatedClassType.cpp
clang-tools-extra/trunk/test/clang-rename/CtorInitializer.cpp
clang-tools-extra/trunk/test/clang-rename/DeclRefExpr.cpp
clang-tools-extra/trunk/test/clang-rename/FunctionMacro.cpp
clang-tools-extra/trunk/test/clang-rename/FunctionOverride.cpp
clang-tools-extra/trunk/test/clang-rename/FunctionWithClassFindByName.cpp
clang-tools-extra/trunk/test/clang-rename/MemberExprMacro.cpp
clang-tools-extra/trunk/test/clang-rename/Namespace.cpp
clang-tools-extra/trunk/test/clang-rename/TemplateClassInstantiation.cpp
clang-tools-extra/trunk/test/clang-rename/TemplateTypename.cpp
clang-tools-extra/trunk/test/clang-rename/UserDefinedConversion.cpp
clang-tools-extra/trunk/test/clang-rename/Variable.cpp
clang-tools-extra/trunk/test/clang-rename/VariableMacro.cpp

Added: clang-tools-extra/trunk/test/clang-rename/ClassAsTemplateArgument.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-rename/ClassAsTemplateArgument.cpp?rev=278221&view=auto
==
--- clang-tools-extra/trunk/test/clang-rename/ClassAsTemplateArgument.cpp 
(added)
+++ clang-tools-extra/trunk/test/clang-rename/ClassAsTemplateArgument.cpp Wed 
Aug 10 08:28:30 2016
@@ -0,0 +1,21 @@
+class Foo /* Test 1 */ {};// CHECK: class Bar /* Test 1 */ {};
+
+template 
+void func() {}
+
+template 
+class Baz {};
+
+int main() {
+  func();// CHECK: func();
+  Baz /* Test 2 */ obj;  // CHECK: Baz /* Test 2 */ obj;
+  return 0;
+}
+
+// Test 1.
+// RUN: clang-rename -offset=7 -new-name=Bar %s -- | sed 's,//.*,,' | 
FileCheck %s
+// Test 2.
+// RUN: clang-rename -offset=215 -new-name=Bar %s -- | sed 's,//.*,,' | 
FileCheck %s
+
+// To find offsets after modifying the file, use:
+//   grep -Ubo 'Foo.*' 

Modified: clang-tools-extra/trunk/test/clang-rename/ClassFindByName.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-rename/ClassFindByName.cpp?rev=278221&r1=278220&r2=278221&view=diff
==
--- clang-tools-extra/trunk/test/clang-rename/ClassFindByName.cpp (original)
+++ clang-tools-extra/trunk/test/clang-rename/ClassFindByName.cpp Wed Aug 10 
08:28:30 2016
@@ -1,11 +1,10 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename rename-all -old-name=Foo -new-name=Bar %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
-class Foo { // CHECK: class Bar
+class Foo { // CHECK: class Bar {
 };
 
 int main() {
   Foo *Pointer = 0; // CHECK: Bar *Pointer = 0;
   return 0;
 }
+
+// Test 1.
+// RUN: clang-rename rename-all -old-name=Foo -new-name=Bar %s -- | sed 
's,//.*,,' | FileCheck %s

Modified: clang-tools-extra/trunk/test/clang-rename/ClassSimpleRenaming.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-rename/ClassSimpleRenaming.cpp?rev=278221&r1=278220&r2=278221&view=diff
==
--- clang-tools-extra/trunk/test/clang-rename/ClassSimpleRenaming.cpp (original)
+++ clang-tools-extra/trunk/test/clang-rename/ClassSimpleRenaming.cpp Wed Aug 
10 08:28:30 2016
@@ -1,13 +1,14 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=136 -new-name=Bar %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+class Foo /* Test 1 */ {  // CHECK: class Bar /* Test 1 */ {
+public:
+  void foo(int x);
+};
 
-class Foo {};   // CHECK: class Bar
+void Foo::foo(int x) /* Test 2 */ {}  // CHECK: void Bar::foo(int x) /* Test 2 
*/ {}
 
-int main() {
-  Foo *Pointer = 0; // CHECK: Bar *Pointer = 0;
-  return 0;
-}
+// Test 1.
+// RUN: clang-rename -offset=6 -new-name=Bar %s -- | sed 's,//.*,,' | 
FileCheck %s
+// Test 2.
+// RUN: clang-rename -offset=109 -new-name=Bar %s -- | sed 's,//.*,,' | 
FileCheck %s
 
-// Use grep -FUbo 'Foo'  to get the correct offset of Cla whe

Re: [PATCH] D23158: [clang-rename] merge tests when possible

2016-08-10 Thread Kirill Bobyrev via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL278221: [clang-rename] merge tests when possible (authored 
by omtcyfz).

Changed prior to commit:
  https://reviews.llvm.org/D23158?vs=67516&id=67517#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D23158

Files:
  clang-tools-extra/trunk/test/clang-rename/ClassAsTemplateArgument.cpp
  clang-tools-extra/trunk/test/clang-rename/ClassFindByName.cpp
  clang-tools-extra/trunk/test/clang-rename/ClassSimpleRenaming.cpp
  clang-tools-extra/trunk/test/clang-rename/ClassTestMulti.cpp
  clang-tools-extra/trunk/test/clang-rename/ClassTestMultiByName.cpp
  clang-tools-extra/trunk/test/clang-rename/ClassTestMultiByNameYAML.cpp
  clang-tools-extra/trunk/test/clang-rename/ComplexFunctionOverride.cpp
  clang-tools-extra/trunk/test/clang-rename/ComplicatedClassType.cpp
  clang-tools-extra/trunk/test/clang-rename/Ctor.cpp
  clang-tools-extra/trunk/test/clang-rename/CtorInitializer.cpp
  clang-tools-extra/trunk/test/clang-rename/DeclRefExpr.cpp
  clang-tools-extra/trunk/test/clang-rename/FunctionMacro.cpp
  clang-tools-extra/trunk/test/clang-rename/FunctionOverride.cpp
  clang-tools-extra/trunk/test/clang-rename/FunctionWithClassFindByName.cpp
  clang-tools-extra/trunk/test/clang-rename/MemberExprMacro.cpp
  clang-tools-extra/trunk/test/clang-rename/Namespace.cpp
  clang-tools-extra/trunk/test/clang-rename/TemplateClassInstantiation.cpp
  clang-tools-extra/trunk/test/clang-rename/TemplateTypename.cpp
  clang-tools-extra/trunk/test/clang-rename/UserDefinedConversion.cpp
  clang-tools-extra/trunk/test/clang-rename/Variable.cpp
  clang-tools-extra/trunk/test/clang-rename/VariableMacro.cpp

Index: clang-tools-extra/trunk/test/clang-rename/Ctor.cpp
===
--- clang-tools-extra/trunk/test/clang-rename/Ctor.cpp
+++ clang-tools-extra/trunk/test/clang-rename/Ctor.cpp
@@ -0,0 +1,14 @@
+class Foo {   // CHECK: class Bar {
+public:
+  Foo();/* Test 1 */  // CHECK: Bar();
+};
+
+Foo::Foo()  /* Test 2 */ {}   // CHECK: Bar::Bar()  /* Test 2 */ {}
+
+// Test 1.
+// RUN: clang-rename -offset=62 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// Test 2.
+// RUN: clang-rename -offset=116 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+
+// To find offsets after modifying the file, use:
+//   grep -Ubo 'Foo.*' 
Index: clang-tools-extra/trunk/test/clang-rename/FunctionMacro.cpp
===
--- clang-tools-extra/trunk/test/clang-rename/FunctionMacro.cpp
+++ clang-tools-extra/trunk/test/clang-rename/FunctionMacro.cpp
@@ -1,21 +1,20 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=199 -new-name=macro_function %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+#define moo foo   // CHECK: #define moo macro_function
 
-#define moo foo // CHECK: #define moo macro_function
-
-int foo() { // CHECK: int macro_function() {
+int foo() /* Test 1 */ {  // CHECK: int macro_function() /* Test 1 */ {
   return 42;
 }
 
 void boo(int value) {}
 
 void qoo() {
-  foo();// CHECK: macro_function();
-  boo(foo());   // CHECK: boo(macro_function());
+  foo();  // CHECK: macro_function();
+  boo(foo()); // CHECK: boo(macro_function());
   moo();
   boo(moo());
 }
 
-// Use grep -FUbo 'foo;'  to get the correct offset of foo when changing
-// this file.
+// Test 1.
+// RUN: clang-rename -offset=68 -new-name=macro_function %s -- | sed 's,//.*,,' | FileCheck %s
+
+// To find offsets after modifying the file, use:
+//   grep -Ubo 'foo.*' 
Index: clang-tools-extra/trunk/test/clang-rename/TemplateClassInstantiation.cpp
===
--- clang-tools-extra/trunk/test/clang-rename/TemplateClassInstantiation.cpp
+++ clang-tools-extra/trunk/test/clang-rename/TemplateClassInstantiation.cpp
@@ -1,5 +1,5 @@
 template 
-class Foo {   // CHECK: class Bar {
+class Foo { /* Test 1 */   // CHECK: class Bar { /* Test 1 */
 public:
   T foo(T arg, T& ref, T* ptr) {
 T value;
@@ -14,31 +14,29 @@
 
 template 
 void func() {
-  Foo obj; // CHECK: Bar obj;
+  Foo obj; /* Test 2 */  // CHECK: Bar obj;
   obj.member = T();
-  Foo::foo();  // CHECK: Bar::foo();
+  Foo::foo();// CHECK: Bar::foo();
 }
 
 int main() {
-  Foo i; // CHECK: Bar i;
+  Foo i; /* Test 3 */  // CHECK: Bar i;
   i.member = 0;
-  Foo::foo(0);   // CHECK: Bar::foo(0);
+  Foo::foo(0); // CHECK: Bar::foo(0);
 
-  Foo b;// CHECK: Bar b;
+  Foo b;  // CHECK: Bar b;
   b.member = false;
-  Foo::foo(false);  // CHECK: Bar::foo(false);
+  Foo::foo(false);// CHECK: Bar::foo(false);
 
   return 0;
 }
 
-// RUN: cat %s > %t-0.cpp
-// RUN: clang-rename -offset=29 -new-name=Bar %t-0.cpp -i -- -fno-delayed-template-parsing
-// RUN: sed 's,//.*,,' %t-0.cpp | FileCheck %s
-
-// 

[clang-tools-extra] r278223 - [clang-rename] remove bunch of deprecated tests

2016-08-10 Thread Kirill Bobyrev via cfe-commits
Author: omtcyfz
Date: Wed Aug 10 08:32:37 2016
New Revision: 278223

URL: http://llvm.org/viewvc/llvm-project?rev=278223&view=rev
Log:
[clang-rename] remove bunch of deprecated tests

Removed:

clang-tools-extra/trunk/test/clang-rename/ClassAsTemplateArgumentFindByClass.cpp

clang-tools-extra/trunk/test/clang-rename/ClassAsTemplateArgumentFindByTemplateArgument.cpp
clang-tools-extra/trunk/test/clang-rename/ClassNameInFunctionDefinition.cpp
clang-tools-extra/trunk/test/clang-rename/ConstCastExpr.cpp
clang-tools-extra/trunk/test/clang-rename/ConstructExpr.cpp
clang-tools-extra/trunk/test/clang-rename/CtorFindByDeclaration.cpp
clang-tools-extra/trunk/test/clang-rename/CtorFindByDefinition.cpp
clang-tools-extra/trunk/test/clang-rename/DtorDeclaration.cpp
clang-tools-extra/trunk/test/clang-rename/DtorDefinition.cpp
clang-tools-extra/trunk/test/clang-rename/DynamicCastExpr.cpp
clang-tools-extra/trunk/test/clang-rename/ReinterpretCastExpr.cpp
clang-tools-extra/trunk/test/clang-rename/StaticCastExpr.cpp

clang-tools-extra/trunk/test/clang-rename/TemplateFunctionFindByDeclaration.cpp
clang-tools-extra/trunk/test/clang-rename/TemplateFunctionFindByUse.cpp

clang-tools-extra/trunk/test/clang-rename/TemplateTypenameFindByTemplateParam.cpp

clang-tools-extra/trunk/test/clang-rename/TemplateTypenameFindByTypeInside.cpp

clang-tools-extra/trunk/test/clang-rename/UserDefinedConversionFindByTypeDeclaration.cpp

Removed: 
clang-tools-extra/trunk/test/clang-rename/ClassAsTemplateArgumentFindByClass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-rename/ClassAsTemplateArgumentFindByClass.cpp?rev=278222&view=auto
==
--- 
clang-tools-extra/trunk/test/clang-rename/ClassAsTemplateArgumentFindByClass.cpp
 (original)
+++ 
clang-tools-extra/trunk/test/clang-rename/ClassAsTemplateArgumentFindByClass.cpp
 (removed)
@@ -1,20 +0,0 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=136 -new-name=Bar %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
-class Foo {};   // CHECK: class Bar {};
-
-template 
-void func() {}
-
-template 
-class Baz {};
-
-int main() {
-  func();  // CHECK: func();
-  Baz obj; // CHECK: Baz obj;
-  return 0;
-}
-
-// Use grep -FUbo 'Foo'  to get the correct offset of Foo when changing
-// this file.

Removed: 
clang-tools-extra/trunk/test/clang-rename/ClassAsTemplateArgumentFindByTemplateArgument.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-rename/ClassAsTemplateArgumentFindByTemplateArgument.cpp?rev=278222&view=auto
==
--- 
clang-tools-extra/trunk/test/clang-rename/ClassAsTemplateArgumentFindByTemplateArgument.cpp
 (original)
+++ 
clang-tools-extra/trunk/test/clang-rename/ClassAsTemplateArgumentFindByTemplateArgument.cpp
 (removed)
@@ -1,20 +0,0 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=304 -new-name=Bar %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
-class Foo {};   // CHECK: class Bar {};
-
-template 
-void func() {}
-
-template 
-class Baz {};
-
-int main() {
-  func();  // CHECK: func();
-  Baz obj; // CHECK: Baz obj;
-  return 0;
-}
-
-// Use grep -FUbo 'Foo'  to get the correct offset of Cla when changing
-// this file.

Removed: 
clang-tools-extra/trunk/test/clang-rename/ClassNameInFunctionDefinition.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-rename/ClassNameInFunctionDefinition.cpp?rev=278222&view=auto
==
--- clang-tools-extra/trunk/test/clang-rename/ClassNameInFunctionDefinition.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-rename/ClassNameInFunctionDefinition.cpp 
(removed)
@@ -1,10 +0,0 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=136 -new-name=Bar %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
-class Foo { // CHECK: class Bar {
-public:
-  void foo(int x);
-};
-
-void Foo::foo(int x) {} // CHECK: void Bar::foo(int x) {}

Removed: clang-tools-extra/trunk/test/clang-rename/ConstCastExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-rename/ConstCastExpr.cpp?rev=278222&view=auto
==
--- clang-tools-extra/trunk/test/clang-rename/ConstCastExpr.cpp (original)
+++ clang-tools-extra/trunk/test/clang-rename/ConstCastExpr.cpp (removed)
@@ -1,18 +0,0 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=136 -new-name=Bar %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
-class Foo { // CHECK: class Bar {
-public:
-  int getValue() {
-return 0;
-  }
-};
-
-int main() {
-  const Foo *C = new Foo(); // CHECK: const Bar *C = new Bar();
-  const_cast(C)->getValue(); // 

Re: [PATCH] D22220: [clang-tidy] Add check 'misc-move-forwarding-reference'

2016-08-10 Thread Martin Böhme via cfe-commits
mboehme added a comment.

Now that https://reviews.llvm.org/D23004 is submitted, can you take another 
look?


https://reviews.llvm.org/D0



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] r278225 - [clang-rename] fix test introduced in r278221

2016-08-10 Thread Kirill Bobyrev via cfe-commits
Author: omtcyfz
Date: Wed Aug 10 08:46:36 2016
New Revision: 278225

URL: http://llvm.org/viewvc/llvm-project?rev=278225&view=rev
Log:
[clang-rename] fix test introduced in r278221

Modified:
clang-tools-extra/trunk/test/clang-rename/ComplicatedClassType.cpp

Modified: clang-tools-extra/trunk/test/clang-rename/ComplicatedClassType.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-rename/ComplicatedClassType.cpp?rev=278225&r1=278224&r2=278225&view=diff
==
--- clang-tools-extra/trunk/test/clang-rename/ComplicatedClassType.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-rename/ComplicatedClassType.cpp Wed Aug 
10 08:46:36 2016
@@ -45,19 +45,19 @@ int main() {
 }
 
 // Test 1.
-// RUN: clang-rename -offset=30 -new-name=Bar %s -- | sed 's,//.*,,' | 
FileCheck %s
+// RUN: clang-rename -offset=30 -new-name=Bar %s -- -frtti | sed 's,//.*,,' | 
FileCheck %s
 // Test 2.
-// RUN: clang-rename -offset=155 -new-name=Bar %s -- | sed 's,//.*,,' | 
FileCheck %s
+// RUN: clang-rename -offset=155 -new-name=Bar %s -- -frtti | sed 's,//.*,,' | 
FileCheck %s
 // Test 3.
-// RUN: clang-rename -offset=1133 -new-name=Bar %s -- | sed 's,//.*,,' | 
FileCheck %s
+// RUN: clang-rename -offset=1133 -new-name=Bar %s -- -frtti | sed 's,//.*,,' 
| FileCheck %s
 // Test 4.
-// RUN: clang-rename -offset=1266 -new-name=Bar %s -- | sed 's,//.*,,' | 
FileCheck %s
+// RUN: clang-rename -offset=1266 -new-name=Bar %s -- -frtti | sed 's,//.*,,' 
| FileCheck %s
 // Test 5.
-// RUN: clang-rename -offset=1402 -new-name=Bar %s -- | sed 's,//.*,,' | 
FileCheck %s
+// RUN: clang-rename -offset=1402 -new-name=Bar %s -- -frtti | sed 's,//.*,,' 
| FileCheck %s
 // Test 6.
-// RUN: clang-rename -offset=1533 -new-name=Bar %s -- | sed 's,//.*,,' | 
FileCheck %s
+// RUN: clang-rename -offset=1533 -new-name=Bar %s -- -frtti | sed 's,//.*,,' 
| FileCheck %s
 // Test 7.
-// RUN: clang-rename -offset=1665 -new-name=Bar %s -- | sed 's,//.*,,' | 
FileCheck %s
+// RUN: clang-rename -offset=1665 -new-name=Bar %s -- -frtti | sed 's,//.*,,' 
| FileCheck %s
 
 // To find offsets after modifying the file, use:
 //   grep -Ubo 'Foo.*' 


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [clang-tools-extra] r278221 - [clang-rename] merge tests when possible

2016-08-10 Thread Aaron Ballman via cfe-commits
On Wed, Aug 10, 2016 at 9:28 AM, Kirill Bobyrev via cfe-commits
 wrote:
> Author: omtcyfz
> Date: Wed Aug 10 08:28:30 2016
> New Revision: 278221
>
> URL: http://llvm.org/viewvc/llvm-project?rev=278221&view=rev
> Log:
> [clang-rename] merge tests when possible
>
> The only difference between some tests is -offset passed to clang-rename. It
> makes sense to merge them into a single file and add multiple tool 
> invocations.

This appears to have broken some of the bots.

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/9739/steps/test/logs/stdio
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/18013/steps/test/logs/stdio

~Aaron

>
> Reviewers: alexfh
>
> Differential Revision: https://reviews.llvm.org/D23158
>
> Added:
> clang-tools-extra/trunk/test/clang-rename/ClassAsTemplateArgument.cpp
> clang-tools-extra/trunk/test/clang-rename/Ctor.cpp
> Modified:
> clang-tools-extra/trunk/test/clang-rename/ClassFindByName.cpp
> clang-tools-extra/trunk/test/clang-rename/ClassSimpleRenaming.cpp
> clang-tools-extra/trunk/test/clang-rename/ClassTestMulti.cpp
> clang-tools-extra/trunk/test/clang-rename/ClassTestMultiByName.cpp
> clang-tools-extra/trunk/test/clang-rename/ClassTestMultiByNameYAML.cpp
> clang-tools-extra/trunk/test/clang-rename/ComplexFunctionOverride.cpp
> clang-tools-extra/trunk/test/clang-rename/ComplicatedClassType.cpp
> clang-tools-extra/trunk/test/clang-rename/CtorInitializer.cpp
> clang-tools-extra/trunk/test/clang-rename/DeclRefExpr.cpp
> clang-tools-extra/trunk/test/clang-rename/FunctionMacro.cpp
> clang-tools-extra/trunk/test/clang-rename/FunctionOverride.cpp
> clang-tools-extra/trunk/test/clang-rename/FunctionWithClassFindByName.cpp
> clang-tools-extra/trunk/test/clang-rename/MemberExprMacro.cpp
> clang-tools-extra/trunk/test/clang-rename/Namespace.cpp
> clang-tools-extra/trunk/test/clang-rename/TemplateClassInstantiation.cpp
> clang-tools-extra/trunk/test/clang-rename/TemplateTypename.cpp
> clang-tools-extra/trunk/test/clang-rename/UserDefinedConversion.cpp
> clang-tools-extra/trunk/test/clang-rename/Variable.cpp
> clang-tools-extra/trunk/test/clang-rename/VariableMacro.cpp
>
> Added: clang-tools-extra/trunk/test/clang-rename/ClassAsTemplateArgument.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-rename/ClassAsTemplateArgument.cpp?rev=278221&view=auto
> ==
> --- clang-tools-extra/trunk/test/clang-rename/ClassAsTemplateArgument.cpp 
> (added)
> +++ clang-tools-extra/trunk/test/clang-rename/ClassAsTemplateArgument.cpp Wed 
> Aug 10 08:28:30 2016
> @@ -0,0 +1,21 @@
> +class Foo /* Test 1 */ {};// CHECK: class Bar /* Test 1 */ {};
> +
> +template 
> +void func() {}
> +
> +template 
> +class Baz {};
> +
> +int main() {
> +  func();// CHECK: func();
> +  Baz /* Test 2 */ obj;  // CHECK: Baz /* Test 2 */ obj;
> +  return 0;
> +}
> +
> +// Test 1.
> +// RUN: clang-rename -offset=7 -new-name=Bar %s -- | sed 's,//.*,,' | 
> FileCheck %s
> +// Test 2.
> +// RUN: clang-rename -offset=215 -new-name=Bar %s -- | sed 's,//.*,,' | 
> FileCheck %s
> +
> +// To find offsets after modifying the file, use:
> +//   grep -Ubo 'Foo.*' 
>
> Modified: clang-tools-extra/trunk/test/clang-rename/ClassFindByName.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-rename/ClassFindByName.cpp?rev=278221&r1=278220&r2=278221&view=diff
> ==
> --- clang-tools-extra/trunk/test/clang-rename/ClassFindByName.cpp (original)
> +++ clang-tools-extra/trunk/test/clang-rename/ClassFindByName.cpp Wed Aug 10 
> 08:28:30 2016
> @@ -1,11 +1,10 @@
> -// RUN: cat %s > %t.cpp
> -// RUN: clang-rename rename-all -old-name=Foo -new-name=Bar %t.cpp -i --
> -// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
> -
> -class Foo { // CHECK: class Bar
> +class Foo { // CHECK: class Bar {
>  };
>
>  int main() {
>Foo *Pointer = 0; // CHECK: Bar *Pointer = 0;
>return 0;
>  }
> +
> +// Test 1.
> +// RUN: clang-rename rename-all -old-name=Foo -new-name=Bar %s -- | sed 
> 's,//.*,,' | FileCheck %s
>
> Modified: clang-tools-extra/trunk/test/clang-rename/ClassSimpleRenaming.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-rename/ClassSimpleRenaming.cpp?rev=278221&r1=278220&r2=278221&view=diff
> ==
> --- clang-tools-extra/trunk/test/clang-rename/ClassSimpleRenaming.cpp 
> (original)
> +++ clang-tools-extra/trunk/test/clang-rename/ClassSimpleRenaming.cpp Wed Aug 
> 10 08:28:30 2016
> @@ -1,13 +1,14 @@
> -// RUN: cat %s > %t.cpp
> -// RUN: clang-rename -offset=136 -new-name=Bar %t.cpp -i --
> -// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
> +class

Re: [PATCH] D23204: Visit lambda capture inits from RecursiveASTVisitor::TraverseLambdaCapture().

2016-08-10 Thread Manuel Klimek via cfe-commits
klimek added inline comments.


Comment at: include/clang/AST/RecursiveASTVisitor.h:892
@@ -891,1 +891,3 @@
+  else
+TRY_TO(TraverseStmt(LE->capture_init_begin()[C - LE->capture_begin()]));
   return true;

I'd rather pass in the offset than doing math with what's semantically a 
pointer vs an iterator.


Comment at: unittests/Tooling/RecursiveASTVisitorTestExprVisitor.cpp:236
@@ +235,3 @@
+TEST(RecursiveASTVisitor, VisitsLambdaInitCaptureInit) {
+  {
+DeclRefExprVisitor Visitor;

I'd remove the extra scope.


https://reviews.llvm.org/D23204



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [clang-tools-extra] r278221 - [clang-rename] merge tests when possible

2016-08-10 Thread Aaron Ballman via cfe-commits
On Wed, Aug 10, 2016 at 9:54 AM, Aaron Ballman  wrote:
> On Wed, Aug 10, 2016 at 9:28 AM, Kirill Bobyrev via cfe-commits
>  wrote:
>> Author: omtcyfz
>> Date: Wed Aug 10 08:28:30 2016
>> New Revision: 278221
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=278221&view=rev
>> Log:
>> [clang-rename] merge tests when possible
>>
>> The only difference between some tests is -offset passed to clang-rename. It
>> makes sense to merge them into a single file and add multiple tool 
>> invocations.
>
> This appears to have broken some of the bots.
>
> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/9739/steps/test/logs/stdio
> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/18013/steps/test/logs/stdio

Oops, you're too fast (you already commit a fix for these issues). :-D

~Aaron

>
> ~Aaron
>
>>
>> Reviewers: alexfh
>>
>> Differential Revision: https://reviews.llvm.org/D23158
>>
>> Added:
>> clang-tools-extra/trunk/test/clang-rename/ClassAsTemplateArgument.cpp
>> clang-tools-extra/trunk/test/clang-rename/Ctor.cpp
>> Modified:
>> clang-tools-extra/trunk/test/clang-rename/ClassFindByName.cpp
>> clang-tools-extra/trunk/test/clang-rename/ClassSimpleRenaming.cpp
>> clang-tools-extra/trunk/test/clang-rename/ClassTestMulti.cpp
>> clang-tools-extra/trunk/test/clang-rename/ClassTestMultiByName.cpp
>> clang-tools-extra/trunk/test/clang-rename/ClassTestMultiByNameYAML.cpp
>> clang-tools-extra/trunk/test/clang-rename/ComplexFunctionOverride.cpp
>> clang-tools-extra/trunk/test/clang-rename/ComplicatedClassType.cpp
>> clang-tools-extra/trunk/test/clang-rename/CtorInitializer.cpp
>> clang-tools-extra/trunk/test/clang-rename/DeclRefExpr.cpp
>> clang-tools-extra/trunk/test/clang-rename/FunctionMacro.cpp
>> clang-tools-extra/trunk/test/clang-rename/FunctionOverride.cpp
>> clang-tools-extra/trunk/test/clang-rename/FunctionWithClassFindByName.cpp
>> clang-tools-extra/trunk/test/clang-rename/MemberExprMacro.cpp
>> clang-tools-extra/trunk/test/clang-rename/Namespace.cpp
>> clang-tools-extra/trunk/test/clang-rename/TemplateClassInstantiation.cpp
>> clang-tools-extra/trunk/test/clang-rename/TemplateTypename.cpp
>> clang-tools-extra/trunk/test/clang-rename/UserDefinedConversion.cpp
>> clang-tools-extra/trunk/test/clang-rename/Variable.cpp
>> clang-tools-extra/trunk/test/clang-rename/VariableMacro.cpp
>>
>> Added: clang-tools-extra/trunk/test/clang-rename/ClassAsTemplateArgument.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-rename/ClassAsTemplateArgument.cpp?rev=278221&view=auto
>> ==
>> --- clang-tools-extra/trunk/test/clang-rename/ClassAsTemplateArgument.cpp 
>> (added)
>> +++ clang-tools-extra/trunk/test/clang-rename/ClassAsTemplateArgument.cpp 
>> Wed Aug 10 08:28:30 2016
>> @@ -0,0 +1,21 @@
>> +class Foo /* Test 1 */ {};// CHECK: class Bar /* Test 1 */ {};
>> +
>> +template 
>> +void func() {}
>> +
>> +template 
>> +class Baz {};
>> +
>> +int main() {
>> +  func();// CHECK: func();
>> +  Baz /* Test 2 */ obj;  // CHECK: Baz /* Test 2 */ obj;
>> +  return 0;
>> +}
>> +
>> +// Test 1.
>> +// RUN: clang-rename -offset=7 -new-name=Bar %s -- | sed 's,//.*,,' | 
>> FileCheck %s
>> +// Test 2.
>> +// RUN: clang-rename -offset=215 -new-name=Bar %s -- | sed 's,//.*,,' | 
>> FileCheck %s
>> +
>> +// To find offsets after modifying the file, use:
>> +//   grep -Ubo 'Foo.*' 
>>
>> Modified: clang-tools-extra/trunk/test/clang-rename/ClassFindByName.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-rename/ClassFindByName.cpp?rev=278221&r1=278220&r2=278221&view=diff
>> ==
>> --- clang-tools-extra/trunk/test/clang-rename/ClassFindByName.cpp (original)
>> +++ clang-tools-extra/trunk/test/clang-rename/ClassFindByName.cpp Wed Aug 10 
>> 08:28:30 2016
>> @@ -1,11 +1,10 @@
>> -// RUN: cat %s > %t.cpp
>> -// RUN: clang-rename rename-all -old-name=Foo -new-name=Bar %t.cpp -i --
>> -// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
>> -
>> -class Foo { // CHECK: class Bar
>> +class Foo { // CHECK: class Bar {
>>  };
>>
>>  int main() {
>>Foo *Pointer = 0; // CHECK: Bar *Pointer = 0;
>>return 0;
>>  }
>> +
>> +// Test 1.
>> +// RUN: clang-rename rename-all -old-name=Foo -new-name=Bar %s -- | sed 
>> 's,//.*,,' | FileCheck %s
>>
>> Modified: clang-tools-extra/trunk/test/clang-rename/ClassSimpleRenaming.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-rename/ClassSimpleRenaming.cpp?rev=278221&r1=278220&r2=278221&view=diff
>> ==
>> --- clang-tools-extra/trunk/test/clang-rename/ClassSimpleRenaming.cpp 
>> (original)
>> +++ cla

Re: [PATCH] D17990: [clang-tidy] minor improvements in modernise-deprecated-headers check

2016-08-10 Thread Kirill Bobyrev via cfe-commits
omtcyfz added a subscriber: omtcyfz.


Comment at: clang-tidy/modernize/DeprecatedHeadersCheck.cpp:79
@@ -69,3 +78,3 @@
 for (const auto &KeyValue :
  std::vector>(
  {{"fenv.h", "cfenv"},

aaron.ballman wrote:
> This will definitely fail in MSVC 2013.
See the left side of `diff`. It already is in source tree, buildbots never 
complained.

Thus, it won't.


Comment at: docs/clang-tidy/checks/modernize-deprecated-headers.rst:39
@@ +38,3 @@
+* `` -> ``
+* `` -> `` // deprecated since C++11
+* `` -> ``

alexfh wrote:
> Not sure if "deprecated" is the right word here. The wording seems to be 
> slightly different: "This header simply includes  and ."
at least http://en.cppreference.com/w/cpp/header says so...


https://reviews.llvm.org/D17990



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D17990: [clang-tidy] minor improvements in modernise-deprecated-headers check

2016-08-10 Thread Aaron Ballman via cfe-commits
aaron.ballman added inline comments.


Comment at: clang-tidy/modernize/DeprecatedHeadersCheck.cpp:79
@@ -69,3 +78,3 @@
 for (const auto &KeyValue :
  std::vector>(
  {{"fenv.h", "cfenv"},

omtcyfz wrote:
> aaron.ballman wrote:
> > This will definitely fail in MSVC 2013.
> See the left side of `diff`. It already is in source tree, buildbots never 
> complained.
> 
> Thus, it won't.
Ah shoot, I was misremembering a different case with a similar construct, 
except it was using std::make_pair, which caused deduction to fail. Le sigh.


https://reviews.llvm.org/D17990



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D22220: [clang-tidy] Add check 'misc-move-forwarding-reference'

2016-08-10 Thread Samuel Benzaquen via cfe-commits
sbenza added inline comments.


Comment at: clang-tidy/misc/MoveForwardingReferenceCheck.cpp:64
@@ +63,3 @@
+void MoveForwardingReferenceCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus11)
+return;

I'm guessing this is checking Lang==C++11, but we actually want Lang>=C++11, 
right?
I'm not sure if there is a way to check that.


Comment at: test/clang-tidy/misc-move-forwarding-reference.cpp:49
@@ +48,3 @@
+// Create a correct fix if there are spaces around the overload resolution
+// operator.
+template  void f5(U &&SomeU) {

I don't understand what this is trying to test.
What is the overload resolution operator?
Also, these two cases look exactly the same as the previous two.


Comment at: test/clang-tidy/misc-move-forwarding-reference.cpp:117
@@ +116,2 @@
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: forwarding reference passed to
+}

This is missing one case for C++14's generic lambdas.

```
[&] (auto&& x) { y = std::move(x); }

```
It might already detect correctly, but it will need a special case for 
generating the `std::forward`
It might need to be something like:

```
[&] (auto&& x) { y = std::forward(x); }

```
We can also just leave a TODO for later.


https://reviews.llvm.org/D0



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D17990: [clang-tidy] minor improvements in modernise-deprecated-headers check

2016-08-10 Thread Kirill Bobyrev via cfe-commits
omtcyf0 updated this revision to Diff 67519.
omtcyf0 added a comment.

Address comments.


https://reviews.llvm.org/D17990

Files:
  clang-tidy/modernize/DeprecatedHeadersCheck.cpp
  docs/clang-tidy/checks/modernize-deprecated-headers.rst
  test/clang-tidy/modernize-deprecated-headers-cxx03.cpp
  test/clang-tidy/modernize-deprecated-headers-cxx11.cpp

Index: test/clang-tidy/modernize-deprecated-headers-cxx11.cpp
===
--- test/clang-tidy/modernize-deprecated-headers-cxx11.cpp
+++ test/clang-tidy/modernize-deprecated-headers-cxx11.cpp
@@ -1,163 +1,163 @@
 // RUN: %check_clang_tidy %s modernize-deprecated-headers %t -- -extra-arg-before=-isystem%S/Inputs/modernize-deprecated-headers -- -std=c++11 -v
 
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'assert.h'; consider using 'cassert' instead [modernize-deprecated-headers]
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'complex.h'; consider using 'complex' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'ctype.h'; consider using 'cctype' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'errno.h'; consider using 'cerrno' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'fenv.h'; consider using 'cfenv' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'float.h'; consider using 'cfloat' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
-#include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'inttypes.h'; consider using 'cinttypes' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'limits.h'; consider using 'climits' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'locale.h'; consider using 'clocale' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'math.h'; consider using 'cmath' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'setjmp.h'; consider using 'csetjmp' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
-#include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'signal.h'; consider using 'csignal' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
-#include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'stdarg.h'; consider using 'cstdarg' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'stddef.h'; consider using 'cstddef' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'stdint.h'; consider using 'cstdint' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'stdio.h'; consider using 'cstdio' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'stdlib.h'; consider using 'cstdlib' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'string.h'; consider using 'cstring' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'tgmath.h'; consider using 'ctgmath' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'time.h'; consider using 'ctime' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'uchar.h'; consider using 'cuchar' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'wchar.h'; consider using 'cwchar' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'wctype.h'; consider using 'cwctype' instead
+// CHECK-FIXES: {{^}}#include 
 
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'assert.h'; consider using 'cassert' instead [modernize-deprecated-headers]
-// CHECK-MESSAGES: :[[@LI

Re: [PATCH] D17990: [clang-tidy] minor improvements in modernise-deprecated-headers check

2016-08-10 Thread Kirill Bobyrev via cfe-commits
omtcyf0 marked an inline comment as done.
omtcyf0 added a comment.

https://reviews.llvm.org/D17990



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D23279: clang-reorder-fields

2016-08-10 Thread Kirill Bobyrev via cfe-commits
omtcyfz added a comment.

In https://reviews.llvm.org/D23279#510234, @alexshap wrote:

> > I do think that an automated tool will do a better job of changing field 
> > orderings in a non-breaking way than most people would, mostly due to 
> > initializer lists.
>
>
> yeah, that was the original motivation


How does that justify it's current state, in which it does not work on projects 
with more than 1 TU?


Repository:
  rL LLVM

https://reviews.llvm.org/D23279



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D22220: [clang-tidy] Add check 'misc-move-forwarding-reference'

2016-08-10 Thread Aaron Ballman via cfe-commits
aaron.ballman added inline comments.


Comment at: clang-tidy/misc/MoveForwardingReferenceCheck.cpp:64
@@ +63,3 @@
+void MoveForwardingReferenceCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus11)
+return;

sbenza wrote:
> I'm guessing this is checking Lang==C++11, but we actually want Lang>=C++11, 
> right?
> I'm not sure if there is a way to check that.
CPlusPlus11 will be true for C++ >= 11.


https://reviews.llvm.org/D0



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D23331: Implement LWG 2148: Make non-enum default hash specialization well-formed

2016-08-10 Thread Marshall Clow via cfe-commits
mclow.lists added a comment.

This looks good to me.
However, we don't //usually// change libc++ for issue resolutions that have not 
yet adopted. We wait until they've "official".


https://reviews.llvm.org/D23331



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D23279: clang-reorder-fields

2016-08-10 Thread Ben Craig via cfe-commits
bcraig added a comment.

In https://reviews.llvm.org/D23279#511187, @omtcyfz wrote:

> In https://reviews.llvm.org/D23279#510234, @alexshap wrote:
>
> > > I do think that an automated tool will do a better job of changing field 
> > > orderings in a non-breaking way than most people would, mostly due to 
> > > initializer lists.
> >
> >
> > yeah, that was the original motivation
>
>
> How does that justify its current state, in which it does breaks codebases 
> with more than 1 TU?


Yeah, that aspect probably needs work :).  Accepting a compilation database 
seems like a good way to correct that.


Repository:
  rL LLVM

https://reviews.llvm.org/D23279



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D23353: [clang-tidy] Add check 'misc-use-after-move'

2016-08-10 Thread Martin Böhme via cfe-commits
mboehme created this revision.
mboehme added a reviewer: alexfh.
mboehme added a subscriber: cfe-commits.

The check warns if an object is used after it has been moved, without an
intervening reinitialization.

See user-facing documentation for details.

https://reviews.llvm.org/D23353

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/UseAfterMoveCheck.cpp
  clang-tidy/misc/UseAfterMoveCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-use-after-move.rst
  test/clang-tidy/misc-use-after-move.cpp

Index: test/clang-tidy/misc-use-after-move.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-use-after-move.cpp
@@ -0,0 +1,1013 @@
+// RUN: %check_clang_tidy %s misc-use-after-move %t
+
+typedef decltype(nullptr) nullptr_t;
+
+namespace std {
+typedef unsigned size_t;
+
+template 
+struct unique_ptr {
+  unique_ptr();
+  T *get() const;
+};
+
+template 
+struct shared_ptr {
+  shared_ptr();
+  T *get() const;
+};
+
+#define DECLARE_STANDARD_CONTAINER(name) \
+  template   \
+  struct name {  \
+name();  \
+void clear();\
+bool empty();\
+  }
+
+#define DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(name) \
+  template   \
+  struct name {  \
+name();  \
+void clear();\
+bool empty();\
+void assign(size_t, const T &);  \
+  }
+
+DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(basic_string);
+DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(vector);
+DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(deque);
+DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(forward_list);
+DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(list);
+DECLARE_STANDARD_CONTAINER(set);
+DECLARE_STANDARD_CONTAINER(map);
+DECLARE_STANDARD_CONTAINER(multiset);
+DECLARE_STANDARD_CONTAINER(multimap);
+DECLARE_STANDARD_CONTAINER(unordered_set);
+DECLARE_STANDARD_CONTAINER(unordered_map);
+DECLARE_STANDARD_CONTAINER(unordered_multiset);
+DECLARE_STANDARD_CONTAINER(unordered_multimap);
+
+typedef basic_string string;
+
+template 
+struct remove_reference;
+
+template 
+struct remove_reference {
+  typedef _Tp type;
+};
+
+template 
+struct remove_reference<_Tp &> {
+  typedef _Tp type;
+};
+
+template 
+struct remove_reference<_Tp &&> {
+  typedef _Tp type;
+};
+
+template 
+constexpr typename std::remove_reference<_Tp>::type &&move(_Tp &&__t) noexcept {
+  return static_cast::type &&>(__t);
+}
+
+} // namespace std
+
+class A {
+public:
+  A();
+  A(const A &);
+  A(A &&);
+
+  A &operator=(const A &);
+  A &operator=(A &&);
+
+  void foo() const;
+  int getInt() const;
+
+  operator bool() const;
+
+  int i;
+};
+
+
+// General tests.
+
+// Simple case.
+void simple() {
+  A a;
+  a.foo();
+  A other_a = std::move(a);
+  a.foo();
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'a' used after it was moved
+  // CHECK-MESSAGES: [[@LINE-3]]:15: note: move occurred here
+}
+
+// A warning should only be emitted for one use-after-move.
+void onlyFlagOneUseAfterMove() {
+  A a;
+  a.foo();
+  A other_a = std::move(a);
+  a.foo();
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'a' used after it was moved
+  // CHECK-MESSAGES: [[@LINE-3]]:15: note: move occurred here
+  a.foo();
+}
+
+void moveAfterMove() {
+  // Move-after-move also counts as a use.
+  {
+A a;
+std::move(a);
+std::move(a);
+// CHECK-MESSAGES: [[@LINE-1]]:15: warning: 'a' used after it was moved
+// CHECK-MESSAGES: [[@LINE-3]]:5: note: move occurred here
+  }
+  // This is also true if the move itself turns into the use on the second loop
+  // iteration.
+  {
+A a;
+for (int i = 0; i < 10; ++i) {
+  std::move(a);
+  // CHECK-MESSAGES: [[@LINE-1]]:17: warning: 'a' used after it was moved
+  // CHECK-MESSAGES: [[@LINE-2]]:7: note: move occurred here
+}
+  }
+}
+
+// Checks also works on function parameters that have a use-after move.
+void parameters(A a) {
+  std::move(a);
+  a.foo();
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'a' used after it was moved
+  // CHECK-MESSAGES: [[@LINE-3]]:3: note: move occurred here
+}
+
+void uniquePtrAndSharedPtr() {
+  // Use-after-moves on std::unique_ptr<> or std::shared_ptr<> aren't flagged.
+  {
+std::unique_ptr ptr;
+std::move(ptr);
+ptr.get();
+  }
+  {
+std::shared_ptr ptr;
+std::move(ptr);
+ptr.get();
+  }
+  // This is also true if the std::unique_ptr<> or std::shared_ptr<> is wrapped
+  // in a typedef.
+  {
+typedef std::unique_ptr PtrToA;
+PtrToA ptr;
+std::move(ptr);
+ptr.get();
+  }
+  {
+typedef std::shared_ptr PtrToA;
+PtrToA ptr;
+std::move(p

Re: [PATCH] D23353: [clang-tidy] Add check 'misc-use-after-move'

2016-08-10 Thread Martin Böhme via cfe-commits
mboehme added a comment.

Apologies for the size of this patch. alexfh@ said he was willing to review it 
as-is -- however, I'm happy to resubmit in smaller pieces if necessary.


https://reviews.llvm.org/D23353



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D23279: clang-reorder-fields

2016-08-10 Thread Kirill Bobyrev via cfe-commits
omtcyfz added a comment.

In https://reviews.llvm.org/D23279#511202, @bcraig wrote:

> In https://reviews.llvm.org/D23279#511187, @omtcyfz wrote:
>
> > In https://reviews.llvm.org/D23279#510234, @alexshap wrote:
> >
> > > > I do think that an automated tool will do a better job of changing 
> > > > field orderings in a non-breaking way than most people would, mostly 
> > > > due to initializer lists.
> > >
> > >
> > > yeah, that was the original motivation
> >
> >
> > How does that justify its current state, in which it does breaks codebases 
> > with more than 1 TU?
>
>
> Yeah, that aspect probably needs work :).  Accepting a compilation database 
> seems like a good way to correct that.


Accepting compilation database doesn't solve the problem completely. Most of 
the existing Clang-based tools accept compilation database, but they do not 
pass information between translation units, which is the most important part.


Repository:
  rL LLVM

https://reviews.llvm.org/D23279



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r278234 - [OpenCL] Change block descriptor address space to constant.

2016-08-10 Thread Joey Gouly via cfe-commits
Author: joey
Date: Wed Aug 10 10:57:02 2016
New Revision: 278234

URL: http://llvm.org/viewvc/llvm-project?rev=278234&view=rev
Log:
[OpenCL] Change block descriptor address space to constant.

The block descriptor is a GlobalVariable in the LLVM IR, so it shouldn't be
in the private address space.

Modified:
cfe/trunk/lib/CodeGen/CGBlocks.cpp
cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl

Modified: cfe/trunk/lib/CodeGen/CGBlocks.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBlocks.cpp?rev=278234&r1=278233&r2=278234&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBlocks.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBlocks.cpp Wed Aug 10 10:57:02 2016
@@ -125,10 +125,15 @@ static llvm::Constant *buildBlockDescrip
 
   llvm::Constant *init = llvm::ConstantStruct::getAnon(elements);
 
+  unsigned AddrSpace = 0;
+  if (C.getLangOpts().OpenCL)
+AddrSpace = C.getTargetAddressSpace(LangAS::opencl_constant);
   llvm::GlobalVariable *global =
 new llvm::GlobalVariable(CGM.getModule(), init->getType(), true,
  llvm::GlobalValue::InternalLinkage,
- init, "__block_descriptor_tmp");
+ init, "__block_descriptor_tmp", nullptr,
+ llvm::GlobalValue::NotThreadLocal,
+ AddrSpace);
 
   return llvm::ConstantExpr::getBitCast(global, CGM.getBlockDescriptorType());
 }
@@ -927,7 +932,10 @@ llvm::Type *CodeGenModule::getBlockDescr
  UnsignedLongTy, UnsignedLongTy, nullptr);
 
   // Now form a pointer to that.
-  BlockDescriptorType = llvm::PointerType::getUnqual(BlockDescriptorType);
+  unsigned AddrSpace = 0;
+  if (getLangOpts().OpenCL)
+AddrSpace = getContext().getTargetAddressSpace(LangAS::opencl_constant);
+  BlockDescriptorType = llvm::PointerType::get(BlockDescriptorType, AddrSpace);
   return BlockDescriptorType;
 }
 

Modified: cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl?rev=278234&r1=278233&r2=278234&view=diff
==
--- cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl Wed Aug 10 
10:57:02 2016
@@ -5,7 +5,7 @@ typedef void (^bl_t)(local void *);
 const bl_t block_G = (bl_t) ^ (local void *a) {};
 
 kernel void device_side_enqueue(global int *a, global int *b, int i) {
-  // CHECK: %default_queue = alloca %opencl.queue_t*
+  // CHECK: %deafault_queue = alloca %opencl.queue_t*
   queue_t default_queue;
   // CHECK: %flags = alloca i32
   unsigned flags = 0;
@@ -21,7 +21,7 @@ kernel void device_side_enqueue(global i
   // CHECK: [[DEF_Q:%[0-9]+]] = load %opencl.queue_t*, %opencl.queue_t** 
%default_queue
   // CHECK: [[FLAGS:%[0-9]+]] = load i32, i32* %flags
   // CHECK: [[NDR:%[0-9]+]] = load %opencl.ndrange_t*, %opencl.ndrange_t** 
%ndrange
-  // CHECK: [[BL:%[0-9]+]] = bitcast <{ i8*, i32, i32, i8*, 
%struct.__block_descriptor*, i32{{.*}}, i32{{.*}}, i32{{.*}} }>* %block to void 
()*
+  // CHECK: [[BL:%[0-9]+]] = bitcast <{ i8*, i32, i32, i8*, 
%struct.__block_descriptor addrspace(3)*, i32{{.*}}, i32{{.*}}, i32{{.*}} }>* 
%block to void ()*
   // CHECK: [[BL_I8:%[0-9]+]] = bitcast void ()* [[BL]] to i8*
   // CHECK: call i32 @__enqueue_kernel_basic(%opencl.queue_t* [[DEF_Q]], i32 
[[FLAGS]], %opencl.ndrange_t* [[NDR]], i8* [[BL_I8]])
   enqueue_kernel(default_queue, flags, ndrange,
@@ -32,7 +32,7 @@ kernel void device_side_enqueue(global i
   // CHECK: [[DEF_Q:%[0-9]+]] = load %opencl.queue_t*, %opencl.queue_t** 
%default_queue
   // CHECK: [[FLAGS:%[0-9]+]] = load i32, i32* %flags
   // CHECK: [[NDR:%[0-9]+]] = load %opencl.ndrange_t*, %opencl.ndrange_t** 
%ndrange
-  // CHECK: [[BL:%[0-9]+]] = bitcast <{ i8*, i32, i32, i8*, 
%struct.__block_descriptor*, i32{{.*}}, i32{{.*}}, i32{{.*}} }>* %block3 to 
void ()*
+  // CHECK: [[BL:%[0-9]+]] = bitcast <{ i8*, i32, i32, i8*, 
%struct.__block_descriptor addrspace(3)*, i32{{.*}}, i32{{.*}}, i32{{.*}} }>* 
%block3 to void ()*
   // CHECK: [[BL_I8:%[0-9]+]] = bitcast void ()* [[BL]] to i8*
   // CHECK: call i32 @__enqueue_kernel_basic_events(%opencl.queue_t* 
[[DEF_Q]], i32 [[FLAGS]], %opencl.ndrange_t* [[NDR]], i32 2, 
%opencl.clk_event_t** %event_wait_list, %opencl.clk_event_t** %clk_event, i8* 
[[BL_I8]])
   enqueue_kernel(default_queue, flags, ndrange, 2, &event_wait_list, 
&clk_event,
@@ -43,7 +43,7 @@ kernel void device_side_enqueue(global i
   // CHECK: [[DEF_Q:%[0-9]+]] = load %opencl.queue_t*, %opencl.queue_t** 
%default_queue
   // CHECK: [[FLAGS:%[0-9]+]] = load i32, i32* %flags
   // CHECK: [[NDR:%[0-9]+]] = load %opencl.ndrange_t*, %opencl.ndrange_t** 
%ndrange
-  // CHECK: call i32 (%opencl.queue_t*, i32, %opencl.ndrange_t*

Re: [PATCH] D23353: [clang-tidy] Add check 'misc-use-after-move'

2016-08-10 Thread Matt Kulukundis via cfe-commits
fowles added a subscriber: fowles.


Comment at: test/clang-tidy/misc-use-after-move.cpp:158
@@ +157,3 @@
+std::move(ptr);
+ptr.get();
+  }

would this warn on:

std::unique_ptr ptr;
std::move(ptr);
ptr->Foo();

I would like it to since that is a likely segfault.


Comment at: test/clang-tidy/misc-use-after-move.cpp:279
@@ +278,3 @@
+A a;
+auto lambda = [=]() { a.foo(); };
+std::move(a);

can you add tests with reference capture?

also what about:

A a;
auto lambda = [&]() { a.foo(); };
std::move(a);
lambda();


https://reviews.llvm.org/D23353



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D23346: [OpenCL] Change block descriptor address space to constant

2016-08-10 Thread Joey Gouly via cfe-commits
joey closed this revision.
joey added a comment.

Committed r278234.

2 years since my last commit!


Repository:
  rL LLVM

https://reviews.llvm.org/D23346



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: Modify MallocChecker.cpp to recognize kfree( )

2016-08-10 Thread Artem Dergachev via cfe-commits
Hey, that's a useful patch, i'd like it to be included! There is odd 
indentation in some places, but in general it looks correct.


The usual thing to do nowadays is to put patches on review on 
Phabricator, would you like to do that? 
(http://llvm.org/docs/Phabricator.html, 
http://llvm.org/docs/DeveloperPolicy.html) The code owner for the 
Analyzer is Anna Zaks, you can add her as a reviewer, and also Devin 
Coughlin and/or myself would most likely join.



On 8/10/16 3:10 AM, Andrew Wells via cfe-commits wrote:
This is my first commit, so my apologies if I did anything wrong.  It 
adds kfree( ) to MallocChecker.cpp and changes the kmalloc-linuxs.c to 
test this change appropriately.


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r278235 - [OpenCL] Fix typo in test that I accidentally introduced in my previous commit.

2016-08-10 Thread Joey Gouly via cfe-commits
Author: joey
Date: Wed Aug 10 11:04:14 2016
New Revision: 278235

URL: http://llvm.org/viewvc/llvm-project?rev=278235&view=rev
Log:
[OpenCL] Fix typo in test that I accidentally introduced in my previous commit.

Modified:
cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl

Modified: cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl?rev=278235&r1=278234&r2=278235&view=diff
==
--- cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl Wed Aug 10 
11:04:14 2016
@@ -5,7 +5,7 @@ typedef void (^bl_t)(local void *);
 const bl_t block_G = (bl_t) ^ (local void *a) {};
 
 kernel void device_side_enqueue(global int *a, global int *b, int i) {
-  // CHECK: %deafault_queue = alloca %opencl.queue_t*
+  // CHECK: %default_queue = alloca %opencl.queue_t*
   queue_t default_queue;
   // CHECK: %flags = alloca i32
   unsigned flags = 0;


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D23320: [analyzer] Fixed crash in CloneDetector in function calls.

2016-08-10 Thread Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL278238: [analyzer] Fix a crash in CloneDetector when calling 
functions by pointers. (authored by dergachev).

Changed prior to commit:
  https://reviews.llvm.org/D23320?vs=67345&id=67537#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D23320

Files:
  cfe/trunk/lib/Analysis/CloneDetection.cpp
  cfe/trunk/test/Analysis/copypaste/call.cpp

Index: cfe/trunk/test/Analysis/copypaste/call.cpp
===
--- cfe/trunk/test/Analysis/copypaste/call.cpp
+++ cfe/trunk/test/Analysis/copypaste/call.cpp
@@ -22,3 +22,15 @@
 return b();
   return true;
 }
+
+// Test that we don't crash on function pointer calls
+
+bool (*funcPtr)(int);
+
+bool fooPtr1(int x) {
+  if (x > 0)
+return false;
+  else if (x < 0)
+return funcPtr(1);
+  return true;
+}
Index: cfe/trunk/lib/Analysis/CloneDetection.cpp
===
--- cfe/trunk/lib/Analysis/CloneDetection.cpp
+++ cfe/trunk/lib/Analysis/CloneDetection.cpp
@@ -249,8 +249,11 @@
   })
 
   //--- Calls --//
-  DEF_ADD_DATA(CallExpr,
-   { addData(S->getDirectCallee()->getQualifiedNameAsString()); })
+  DEF_ADD_DATA(CallExpr, {
+// Function pointers don't have a callee and we just skip hashing it.
+if (S->getDirectCallee())
+  addData(S->getDirectCallee()->getQualifiedNameAsString());
+  })
 
   //--- Exceptions -//
   DEF_ADD_DATA(CXXCatchStmt, { addData(S->getCaughtType()); })


Index: cfe/trunk/test/Analysis/copypaste/call.cpp
===
--- cfe/trunk/test/Analysis/copypaste/call.cpp
+++ cfe/trunk/test/Analysis/copypaste/call.cpp
@@ -22,3 +22,15 @@
 return b();
   return true;
 }
+
+// Test that we don't crash on function pointer calls
+
+bool (*funcPtr)(int);
+
+bool fooPtr1(int x) {
+  if (x > 0)
+return false;
+  else if (x < 0)
+return funcPtr(1);
+  return true;
+}
Index: cfe/trunk/lib/Analysis/CloneDetection.cpp
===
--- cfe/trunk/lib/Analysis/CloneDetection.cpp
+++ cfe/trunk/lib/Analysis/CloneDetection.cpp
@@ -249,8 +249,11 @@
   })
 
   //--- Calls --//
-  DEF_ADD_DATA(CallExpr,
-   { addData(S->getDirectCallee()->getQualifiedNameAsString()); })
+  DEF_ADD_DATA(CallExpr, {
+// Function pointers don't have a callee and we just skip hashing it.
+if (S->getDirectCallee())
+  addData(S->getDirectCallee()->getQualifiedNameAsString());
+  })
 
   //--- Exceptions -//
   DEF_ADD_DATA(CXXCatchStmt, { addData(S->getCaughtType()); })
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: r278139 - [clang-cl] Make -gline-tables-only imply -gcodeview

2016-08-10 Thread Hans Wennborg via cfe-commits
Sure, r278240.

On Wed, Aug 10, 2016 at 3:55 AM, Nico Weber  wrote:
> Since this flag is new in 3.9, should we merge this there?
>
> On Tue, Aug 9, 2016 at 1:23 PM, Reid Kleckner via cfe-commits
>  wrote:
>>
>> Author: rnk
>> Date: Tue Aug  9 12:23:56 2016
>> New Revision: 278139
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=278139&view=rev
>> Log:
>> [clang-cl] Make -gline-tables-only imply -gcodeview
>>
>> It's surprising that you have to pass /Z7 in addition to -gcodeview to
>> get debug info. The sanitizer runtime, for example, expects that if the
>> compiler supports the -gline-tables-only flag, then it will emit debug
>> info.
>>
>> Modified:
>> cfe/trunk/lib/Driver/Tools.cpp
>> cfe/trunk/test/Driver/cl-options.c
>>
>> Modified: cfe/trunk/lib/Driver/Tools.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=278139&r1=278138&r2=278139&view=diff
>>
>> ==
>> --- cfe/trunk/lib/Driver/Tools.cpp (original)
>> +++ cfe/trunk/lib/Driver/Tools.cpp Tue Aug  9 12:23:56 2016
>> @@ -4611,8 +4611,8 @@ void Clang::ConstructJob(Compilation &C,
>> options::OPT_gdwarf_4,
>> options::OPT_gdwarf_5))
>>  DwarfVersion = DwarfVersionNum(A->getSpelling());
>>
>> -  // Forward -gcodeview.
>> -  // 'EmitCodeView might have been set by CL-compatibility argument
>> parsing.
>> +  // Forward -gcodeview. EmitCodeView might have been set by
>> CL-compatibility
>> +  // argument parsing.
>>if (Args.hasArg(options::OPT_gcodeview) || EmitCodeView) {
>>  // DwarfVersion remains at 0 if no explicit choice was made.
>>  CmdArgs.push_back("-gcodeview");
>> @@ -6402,9 +6402,10 @@ void Clang::AddClangCLArgs(const ArgList
>>  CmdArgs.push_back(Args.MakeArgString(Twine(LangOptions::SSPStrong)));
>>}
>>
>> -  // Emit CodeView if -Z7 or -Zd are present.
>> +  // Emit CodeView if -Z7, -Zd, or -gline-tables-only are present.
>>if (Arg *DebugInfoArg =
>> -  Args.getLastArg(options::OPT__SLASH_Z7,
>> options::OPT__SLASH_Zd)) {
>> +  Args.getLastArg(options::OPT__SLASH_Z7, options::OPT__SLASH_Zd,
>> +  options::OPT_gline_tables_only)) {
>>  *EmitCodeView = true;
>>  if (DebugInfoArg->getOption().matches(options::OPT__SLASH_Z7))
>>*DebugInfoKind = codegenoptions::LimitedDebugInfo;
>>
>> Modified: cfe/trunk/test/Driver/cl-options.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-options.c?rev=278139&r1=278138&r2=278139&view=diff
>>
>> ==
>> --- cfe/trunk/test/Driver/cl-options.c (original)
>> +++ cfe/trunk/test/Driver/cl-options.c Tue Aug  9 12:23:56 2016
>> @@ -424,6 +424,10 @@
>>  // Z7GMLT: "-gcodeview"
>>  // Z7GMLT: "-debug-info-kind=line-tables-only"
>>
>> +// RUN: %clang_cl -gline-tables-only /c -### -- %s 2>&1 | FileCheck
>> -check-prefix=ZGMLT %s
>> +// ZGMLT: "-gcodeview"
>> +// ZGMLT: "-debug-info-kind=line-tables-only"
>> +
>>  // RUN: %clang_cl /c -### -- %s 2>&1 | FileCheck
>> -check-prefix=BreproDefault %s
>>  // BreproDefault: "-mincremental-linker-compatible"
>>
>>
>>
>> ___
>> 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


r278238 - [analyzer] Fix a crash in CloneDetector when calling functions by pointers.

2016-08-10 Thread Artem Dergachev via cfe-commits
Author: dergachev
Date: Wed Aug 10 11:25:16 2016
New Revision: 278238

URL: http://llvm.org/viewvc/llvm-project?rev=278238&view=rev
Log:
[analyzer] Fix a crash in CloneDetector when calling functions by pointers.

CallExpr may have a null direct callee when the callee function is not
known in compile-time. Do not try to take callee name in this case.

Patch by Raphael Isemann!

Differential Revision: https://reviews.llvm.org/D23320

Modified:
cfe/trunk/lib/Analysis/CloneDetection.cpp
cfe/trunk/test/Analysis/copypaste/call.cpp

Modified: cfe/trunk/lib/Analysis/CloneDetection.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CloneDetection.cpp?rev=278238&r1=278237&r2=278238&view=diff
==
--- cfe/trunk/lib/Analysis/CloneDetection.cpp (original)
+++ cfe/trunk/lib/Analysis/CloneDetection.cpp Wed Aug 10 11:25:16 2016
@@ -249,8 +249,11 @@ public:
   })
 
   //--- Calls --//
-  DEF_ADD_DATA(CallExpr,
-   { addData(S->getDirectCallee()->getQualifiedNameAsString()); })
+  DEF_ADD_DATA(CallExpr, {
+// Function pointers don't have a callee and we just skip hashing it.
+if (S->getDirectCallee())
+  addData(S->getDirectCallee()->getQualifiedNameAsString());
+  })
 
   //--- Exceptions -//
   DEF_ADD_DATA(CXXCatchStmt, { addData(S->getCaughtType()); })

Modified: cfe/trunk/test/Analysis/copypaste/call.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/copypaste/call.cpp?rev=278238&r1=278237&r2=278238&view=diff
==
--- cfe/trunk/test/Analysis/copypaste/call.cpp (original)
+++ cfe/trunk/test/Analysis/copypaste/call.cpp Wed Aug 10 11:25:16 2016
@@ -22,3 +22,15 @@ bool foo2(int x) {
 return b();
   return true;
 }
+
+// Test that we don't crash on function pointer calls
+
+bool (*funcPtr)(int);
+
+bool fooPtr1(int x) {
+  if (x > 0)
+return false;
+  else if (x < 0)
+return funcPtr(1);
+  return true;
+}


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D13126: New static analyzer checker for loss of sign/precision

2016-08-10 Thread Artem Dergachev via cfe-commits
NoQ added a comment.

Wow, i've completely forgot about this one. Sorry about that... I think this 
checker is good to have in the current shape, and probably incrementally 
developed. It'd probably move out of alpha whenever it's tweaked to somehow 
make sure it finds enough real bugs among intentional integer truncations, 
which would most likely require some nontrivial heuristics.

Added a few very minor comments, once they're fixed i'd consider committing(?)



Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:10
@@ +9,3 @@
+//
+// Check that there is not loss of sign/precision in assignments, comparisons
+// and multiplications.

~~not~~ => no


Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:21
@@ +20,3 @@
+// Many compilers and tools have similar checks that are based on semantic
+// analysis. Those checks are sound but has poor precision. ConversionChecker
+// is an alternative to those checks.

~~has~~ => have


Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:66
@@ +65,3 @@
+  const Stmt *Parent = PM.getParent(Cast);
+  if (!Parent)
+return;

I think you can assert that the parent exists here.


Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:87
@@ +86,3 @@
+  // Generate an error node.
+  ExplodedNode *N = C.generateErrorNode(C.getState());
+  if (!N)

You are generating a fatal error node, which stops the analysis on the path on 
which the bug is found. I don't think it's desired, because the program isn't 
known to terminate upon loss of sign/precision, and it suppresses other 
warnings by other checkers. I think you need to use 
`generateNonFatalErrorNode()` here. But then you need to make sure you generate 
this node exactly once per callback and re-use for all reports in case there 
are multiple reports, otherwise the execution path behind this node may be 
duplicated.


https://reviews.llvm.org/D13126



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D23353: [clang-tidy] Add check 'misc-use-after-move'

2016-08-10 Thread Piotr Padlewski via cfe-commits
Prazek added a subscriber: Prazek.
Prazek added a reviewer: Prazek.
Prazek added a comment.

I will review it later, but my first thoughts:

1. I think we should make some other group, because misc seems to be 
overloaded. I discussed it with Alex months ago - something like bugprone would 
be good.
2. Also it would be good to make link in cppcoreguidelines.


https://reviews.llvm.org/D23353



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D23343: [clang-tidy] modernize-make-{smart_ptr} private ctor bugfix

2016-08-10 Thread Piotr Padlewski via cfe-commits
Prazek added inline comments.


Comment at: test/clang-tidy/modernize-make-shared.cpp:109
@@ +108,3 @@
+  void create() {
+auto ptr = std::shared_ptr(new Private(42));
+  }

aaron.ballman wrote:
> Add comments explaining why make_shared is not correct. Also, please add a 
> case showing that protected constructors still get the proper fix applied.
You mean I should also not warn when the constructor is protected? Make sense.


https://reviews.llvm.org/D23343



___
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

2016-08-10 Thread Alexander Kornienko via cfe-commits
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.

LG


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: r278234 - [OpenCL] Change block descriptor address space to constant.

2016-08-10 Thread Anastasia Stulova via cfe-commits
Hi Hans,

Is it still possible to merge this change in release 3.9 branch. This is just a 
minor bug fix we have found with Clang Blocks and only affects OpenCL.

PS, it goes together with a typo fix in the next commit r278235.

Thanks in advance,
Anastasia

-Original Message-
From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of Joey 
Gouly via cfe-commits
Sent: 10 August 2016 16:57
To: cfe-commits@lists.llvm.org
Subject: r278234 - [OpenCL] Change block descriptor address space to constant.

Author: joey
Date: Wed Aug 10 10:57:02 2016
New Revision: 278234

URL: http://llvm.org/viewvc/llvm-project?rev=278234&view=rev
Log:
[OpenCL] Change block descriptor address space to constant.

The block descriptor is a GlobalVariable in the LLVM IR, so it shouldn't be in 
the private address space.

Modified:
cfe/trunk/lib/CodeGen/CGBlocks.cpp
cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl

Modified: cfe/trunk/lib/CodeGen/CGBlocks.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBlocks.cpp?rev=278234&r1=278233&r2=278234&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBlocks.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBlocks.cpp Wed Aug 10 10:57:02 2016
@@ -125,10 +125,15 @@ static llvm::Constant *buildBlockDescrip
 
   llvm::Constant *init = llvm::ConstantStruct::getAnon(elements);
 
+  unsigned AddrSpace = 0;
+  if (C.getLangOpts().OpenCL)
+AddrSpace = C.getTargetAddressSpace(LangAS::opencl_constant);
   llvm::GlobalVariable *global =
 new llvm::GlobalVariable(CGM.getModule(), init->getType(), true,
  llvm::GlobalValue::InternalLinkage,
- init, "__block_descriptor_tmp");
+ init, "__block_descriptor_tmp", nullptr,
+ llvm::GlobalValue::NotThreadLocal,
+ AddrSpace);
 
   return llvm::ConstantExpr::getBitCast(global, CGM.getBlockDescriptorType()); 
 } @@ -927,7 +932,10 @@ llvm::Type *CodeGenModule::getBlockDescr
  UnsignedLongTy, UnsignedLongTy, nullptr);
 
   // Now form a pointer to that.
-  BlockDescriptorType = llvm::PointerType::getUnqual(BlockDescriptorType);
+  unsigned AddrSpace = 0;
+  if (getLangOpts().OpenCL)
+AddrSpace = 
+ getContext().getTargetAddressSpace(LangAS::opencl_constant);
+  BlockDescriptorType = llvm::PointerType::get(BlockDescriptorType, 
+ AddrSpace);
   return BlockDescriptorType;
 }
 

Modified: cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl?rev=278234&r1=278233&r2=278234&view=diff
==
--- cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl Wed Aug 10 
+++ 10:57:02 2016
@@ -5,7 +5,7 @@ typedef void (^bl_t)(local void *);  const bl_t block_G = 
(bl_t) ^ (local void *a) {};
 
 kernel void device_side_enqueue(global int *a, global int *b, int i) {
-  // CHECK: %default_queue = alloca %opencl.queue_t*
+  // CHECK: %deafault_queue = alloca %opencl.queue_t*
   queue_t default_queue;
   // CHECK: %flags = alloca i32
   unsigned flags = 0;
@@ -21,7 +21,7 @@ kernel void device_side_enqueue(global i
   // CHECK: [[DEF_Q:%[0-9]+]] = load %opencl.queue_t*, %opencl.queue_t** 
%default_queue
   // CHECK: [[FLAGS:%[0-9]+]] = load i32, i32* %flags
   // CHECK: [[NDR:%[0-9]+]] = load %opencl.ndrange_t*, %opencl.ndrange_t** 
%ndrange
-  // CHECK: [[BL:%[0-9]+]] = bitcast <{ i8*, i32, i32, i8*, 
%struct.__block_descriptor*, i32{{.*}}, i32{{.*}}, i32{{.*}} }>* %block to void 
()*
+  // CHECK: [[BL:%[0-9]+]] = bitcast <{ i8*, i32, i32, i8*, 
+ %struct.__block_descriptor addrspace(3)*, i32{{.*}}, i32{{.*}}, 
+ i32{{.*}} }>* %block to void ()*
   // CHECK: [[BL_I8:%[0-9]+]] = bitcast void ()* [[BL]] to i8*
   // CHECK: call i32 @__enqueue_kernel_basic(%opencl.queue_t* [[DEF_Q]], i32 
[[FLAGS]], %opencl.ndrange_t* [[NDR]], i8* [[BL_I8]])
   enqueue_kernel(default_queue, flags, ndrange, @@ -32,7 +32,7 @@ kernel void 
device_side_enqueue(global i
   // CHECK: [[DEF_Q:%[0-9]+]] = load %opencl.queue_t*, %opencl.queue_t** 
%default_queue
   // CHECK: [[FLAGS:%[0-9]+]] = load i32, i32* %flags
   // CHECK: [[NDR:%[0-9]+]] = load %opencl.ndrange_t*, %opencl.ndrange_t** 
%ndrange
-  // CHECK: [[BL:%[0-9]+]] = bitcast <{ i8*, i32, i32, i8*, 
%struct.__block_descriptor*, i32{{.*}}, i32{{.*}}, i32{{.*}} }>* %block3 to 
void ()*
+  // CHECK: [[BL:%[0-9]+]] = bitcast <{ i8*, i32, i32, i8*, 
+ %struct.__block_descriptor addrspace(3)*, i32{{.*}}, i32{{.*}}, 
+ i32{{.*}} }>* %block3 to void ()*
   // CHECK: [[BL_I8:%[0-9]+]] = bitcast void ()* [[BL]] to i8*
   // CHECK: call i32 @__enqueue_kernel_basic_events(%opencl.queue_t* 
[[DEF_Q]], i32 [[FLAGS]], %opencl.ndrang

Re: [PATCH] D17990: [clang-tidy] minor improvements in modernise-deprecated-headers check

2016-08-10 Thread Alexander Kornienko via cfe-commits
alexfh added inline comments.


Comment at: clang-tidy/modernize/DeprecatedHeadersCheck.cpp:83
@@ +82,3 @@
+  for (const auto &Key : DeleteHeaders) {
+DeleteHeadersSet.insert(Key);
+  }

No danger of dangling references here, since `StringMap` copies keys. See 
`StringMap` comments and `StringMapEntry::Create` implementation.

So there's no need to store another copy of this data. 

Kirill, please revert the latest change in this constructor.


https://reviews.llvm.org/D17990



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: r278234 - [OpenCL] Change block descriptor address space to constant.

2016-08-10 Thread Hans Wennborg via cfe-commits
Merged together with r278235 in r278248.

Thanks,
Hans

On Wed, Aug 10, 2016 at 10:25 AM, Anastasia Stulova via cfe-commits
 wrote:
> Hi Hans,
>
> Is it still possible to merge this change in release 3.9 branch. This is just 
> a minor bug fix we have found with Clang Blocks and only affects OpenCL.
>
> PS, it goes together with a typo fix in the next commit r278235.
>
> Thanks in advance,
> Anastasia
>
> -Original Message-
> From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of 
> Joey Gouly via cfe-commits
> Sent: 10 August 2016 16:57
> To: cfe-commits@lists.llvm.org
> Subject: r278234 - [OpenCL] Change block descriptor address space to constant.
>
> Author: joey
> Date: Wed Aug 10 10:57:02 2016
> New Revision: 278234
>
> URL: http://llvm.org/viewvc/llvm-project?rev=278234&view=rev
> Log:
> [OpenCL] Change block descriptor address space to constant.
>
> The block descriptor is a GlobalVariable in the LLVM IR, so it shouldn't be 
> in the private address space.
>
> Modified:
> cfe/trunk/lib/CodeGen/CGBlocks.cpp
> cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl
>
> Modified: cfe/trunk/lib/CodeGen/CGBlocks.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBlocks.cpp?rev=278234&r1=278233&r2=278234&view=diff
> ==
> --- cfe/trunk/lib/CodeGen/CGBlocks.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGBlocks.cpp Wed Aug 10 10:57:02 2016
> @@ -125,10 +125,15 @@ static llvm::Constant *buildBlockDescrip
>
>llvm::Constant *init = llvm::ConstantStruct::getAnon(elements);
>
> +  unsigned AddrSpace = 0;
> +  if (C.getLangOpts().OpenCL)
> +AddrSpace = C.getTargetAddressSpace(LangAS::opencl_constant);
>llvm::GlobalVariable *global =
>  new llvm::GlobalVariable(CGM.getModule(), init->getType(), true,
>   llvm::GlobalValue::InternalLinkage,
> - init, "__block_descriptor_tmp");
> + init, "__block_descriptor_tmp", nullptr,
> + llvm::GlobalValue::NotThreadLocal,
> + AddrSpace);
>
>return llvm::ConstantExpr::getBitCast(global, 
> CGM.getBlockDescriptorType());  } @@ -927,7 +932,10 @@ llvm::Type 
> *CodeGenModule::getBlockDescr
>   UnsignedLongTy, UnsignedLongTy, nullptr);
>
>// Now form a pointer to that.
> -  BlockDescriptorType = llvm::PointerType::getUnqual(BlockDescriptorType);
> +  unsigned AddrSpace = 0;
> +  if (getLangOpts().OpenCL)
> +AddrSpace =
> + getContext().getTargetAddressSpace(LangAS::opencl_constant);
> +  BlockDescriptorType = llvm::PointerType::get(BlockDescriptorType,
> + AddrSpace);
>return BlockDescriptorType;
>  }
>
>
> Modified: cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl?rev=278234&r1=278233&r2=278234&view=diff
> ==
> --- cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl (original)
> +++ cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl Wed Aug 10
> +++ 10:57:02 2016
> @@ -5,7 +5,7 @@ typedef void (^bl_t)(local void *);  const bl_t block_G = 
> (bl_t) ^ (local void *a) {};
>
>  kernel void device_side_enqueue(global int *a, global int *b, int i) {
> -  // CHECK: %default_queue = alloca %opencl.queue_t*
> +  // CHECK: %deafault_queue = alloca %opencl.queue_t*
>queue_t default_queue;
>// CHECK: %flags = alloca i32
>unsigned flags = 0;
> @@ -21,7 +21,7 @@ kernel void device_side_enqueue(global i
>// CHECK: [[DEF_Q:%[0-9]+]] = load %opencl.queue_t*, %opencl.queue_t** 
> %default_queue
>// CHECK: [[FLAGS:%[0-9]+]] = load i32, i32* %flags
>// CHECK: [[NDR:%[0-9]+]] = load %opencl.ndrange_t*, %opencl.ndrange_t** 
> %ndrange
> -  // CHECK: [[BL:%[0-9]+]] = bitcast <{ i8*, i32, i32, i8*, 
> %struct.__block_descriptor*, i32{{.*}}, i32{{.*}}, i32{{.*}} }>* %block to 
> void ()*
> +  // CHECK: [[BL:%[0-9]+]] = bitcast <{ i8*, i32, i32, i8*,
> + %struct.__block_descriptor addrspace(3)*, i32{{.*}}, i32{{.*}},
> + i32{{.*}} }>* %block to void ()*
>// CHECK: [[BL_I8:%[0-9]+]] = bitcast void ()* [[BL]] to i8*
>// CHECK: call i32 @__enqueue_kernel_basic(%opencl.queue_t* [[DEF_Q]], i32 
> [[FLAGS]], %opencl.ndrange_t* [[NDR]], i8* [[BL_I8]])
>enqueue_kernel(default_queue, flags, ndrange, @@ -32,7 +32,7 @@ kernel 
> void device_side_enqueue(global i
>// CHECK: [[DEF_Q:%[0-9]+]] = load %opencl.queue_t*, %opencl.queue_t** 
> %default_queue
>// CHECK: [[FLAGS:%[0-9]+]] = load i32, i32* %flags
>// CHECK: [[NDR:%[0-9]+]] = load %opencl.ndrange_t*, %opencl.ndrange_t** 
> %ndrange
> -  // CHECK: [[BL:%[0-9]+]] = bitcast <{ i8*, i32, i32, i8*, 
> %struct.__block_descriptor*, i32{{.*}}, i32{{.*}}, i32{{.*}} }>* %block3 to 
> void ()*
> +  // CHEC

Re: [PATCH] D17990: [clang-tidy] minor improvements in modernise-deprecated-headers check

2016-08-10 Thread Kirill Bobyrev via cfe-commits
omtcyf0 updated this revision to Diff 67551.
omtcyf0 marked an inline comment as not done.
omtcyf0 added a comment.

Revert changes.


https://reviews.llvm.org/D17990

Files:
  clang-tidy/modernize/DeprecatedHeadersCheck.cpp
  docs/clang-tidy/checks/modernize-deprecated-headers.rst
  test/clang-tidy/modernize-deprecated-headers-cxx03.cpp
  test/clang-tidy/modernize-deprecated-headers-cxx11.cpp

Index: test/clang-tidy/modernize-deprecated-headers-cxx11.cpp
===
--- test/clang-tidy/modernize-deprecated-headers-cxx11.cpp
+++ test/clang-tidy/modernize-deprecated-headers-cxx11.cpp
@@ -1,163 +1,163 @@
 // RUN: %check_clang_tidy %s modernize-deprecated-headers %t -- -extra-arg-before=-isystem%S/Inputs/modernize-deprecated-headers -- -std=c++11 -v
 
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'assert.h'; consider using 'cassert' instead [modernize-deprecated-headers]
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'complex.h'; consider using 'complex' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'ctype.h'; consider using 'cctype' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'errno.h'; consider using 'cerrno' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'fenv.h'; consider using 'cfenv' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'float.h'; consider using 'cfloat' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
-#include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'inttypes.h'; consider using 'cinttypes' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'limits.h'; consider using 'climits' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'locale.h'; consider using 'clocale' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'math.h'; consider using 'cmath' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'setjmp.h'; consider using 'csetjmp' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
-#include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'signal.h'; consider using 'csignal' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
-#include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'stdarg.h'; consider using 'cstdarg' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'stddef.h'; consider using 'cstddef' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'stdint.h'; consider using 'cstdint' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'stdio.h'; consider using 'cstdio' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'stdlib.h'; consider using 'cstdlib' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'string.h'; consider using 'cstring' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'tgmath.h'; consider using 'ctgmath' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'time.h'; consider using 'ctime' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'uchar.h'; consider using 'cuchar' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'wchar.h'; consider using 'cwchar' instead
+// CHECK-FIXES: {{^}}#include {{$}}
 #include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'wctype.h'; consider using 'cwctype' instead
+// CHECK-FIXES: {{^}}#include 
 
-// CHECK-MESSAGES: :[[@LINE-27]]:10: warning: inclusion of deprecated C++ header 'assert.h'; consider using 'cassert' instead [modernize-de

Re: [PATCH] D17990: [clang-tidy] minor improvements in modernise-deprecated-headers check

2016-08-10 Thread Alexander Kornienko via cfe-commits
alexfh added a comment.

In https://reviews.llvm.org/D17990#511424, @omtcyf0 wrote:

> Revert changes.


Thanks! If Aaron has no objections, good to go.


https://reviews.llvm.org/D17990



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] trivial documentation fix regarding Obj-C ARC objc_arc_weak_reference_unavailable

2016-08-10 Thread Sean McBride via cfe-commits
Fixed incorrect docs that referred to:
 objc_arc_weak_unavailable
when it should be:
 objc_arc_weak_reference_unavailable

Cheers,

Sean


objc_arc_weak_unavailable-typo.patch
Description: Binary data
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D17990: [clang-tidy] minor improvements in modernise-deprecated-headers check

2016-08-10 Thread Kirill Bobyrev via cfe-commits
omtcyf0 added a comment.

In https://reviews.llvm.org/D17990#511429, @alexfh wrote:

> In https://reviews.llvm.org/D17990#511424, @omtcyf0 wrote:
>
> > Revert changes.
>
>
> Thanks! If Aaron has no objections, good to go.


Great! Thank you!


https://reviews.llvm.org/D17990



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D23279: clang-reorder-fields

2016-08-10 Thread Ben Craig via cfe-commits
bcraig added a comment.

In https://reviews.llvm.org/D23279#511266, @omtcyfz wrote:

> In https://reviews.llvm.org/D23279#511202, @bcraig wrote:
>
> > In https://reviews.llvm.org/D23279#511187, @omtcyfz wrote:
> >
> > > In https://reviews.llvm.org/D23279#510234, @alexshap wrote:
> > >
> > > > > I do think that an automated tool will do a better job of changing 
> > > > > field orderings in a non-breaking way than most people would, mostly 
> > > > > due to initializer lists.
> > > >
> > > >
> > > > yeah, that was the original motivation
> > >
> > >
> > > How does that justify its current state, in which it does breaks 
> > > codebases with more than 1 TU?
> >
> >
> > Yeah, that aspect probably needs work :).  Accepting a compilation database 
> > seems like a good way to correct that.
>
>
> Accepting compilation database doesn't solve the problem completely. Most of 
> the existing Clang-based tools accept compilation database, but they do not 
> pass information between translation units, which is the most important part.


I will agree that compilation databases are a (mostly) necessary, but not 
sufficient part of the solution.  The refactoring tool will need to be careful 
about changing the structure definition.  It needs to remember (across TUs) 
what the old definition was and what the desired definition is, so that it can 
correctly fix up the uses and initializer lists.  If it saves the files too 
aggressively, then there will be some serious problems.

So lots of things for @alexshap to work on.


Repository:
  rL LLVM

https://reviews.llvm.org/D23279



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D23279: clang-reorder-fields

2016-08-10 Thread Kirill Bobyrev via cfe-commits
omtcyf0 added a subscriber: omtcyf0.
omtcyf0 added a comment.

In https://reviews.llvm.org/D23279#511439, @bcraig wrote:

> I will agree that compilation databases are a (mostly) necessary, but not 
> sufficient part of the solution.  The refactoring tool will need to be 
> careful about changing the structure definition.  It needs to remember 
> (across TUs) what the old definition was and what the desired definition is, 
> so that it can correctly fix up the uses and initializer lists.  If it saves 
> the files too aggressively, then there will be some serious problems.
>
> So lots of things for @alexshap to work on.


Well, multi-TU support is needed for many tools and it is far beyond one tool 
implementation details if we want it to be nice. I want to have multi-TU 
support in the clang-refactor, but it is still in designing stage at the 
moment. At first, one translation unit might be okay, though.


Repository:
  rL LLVM

https://reviews.llvm.org/D23279



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D23361: [OpenCL] AMDGCN: Fix size_t type

2016-08-10 Thread Yaxun Liu via cfe-commits
yaxunl created this revision.
yaxunl added reviewers: nhaustov, Anastasia.
yaxunl added subscribers: cfe-commits, tstellarAMD.

Pointers of certain GPUs in AMDGCN target in private address space is 32 bit 
but pointers in other address spaces are 64 bit. size_t type should be defined 
as 64 bit for these GPUs so that it could hold pointers in all address spaces.

https://reviews.llvm.org/D23361

Files:
  include/clang/Basic/TargetInfo.h
  lib/Basic/TargetInfo.cpp
  lib/Basic/Targets.cpp
  lib/CodeGen/CodeGenModule.cpp
  test/CodeGenOpenCL/size_t.cl

Index: test/CodeGenOpenCL/size_t.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/size_t.cl
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -finclude-default-header -emit-llvm -O0 -triple spir-unknown-unknown -o - | FileCheck --check-prefix=SZ32 %s
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -finclude-default-header -emit-llvm -O0 -triple spir64-unknown-unknown -o - | FileCheck --check-prefix=SZ64 %s
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -finclude-default-header -emit-llvm -O0 -triple amdgcn-- -o - | FileCheck --check-prefix=SZ64 %s
+
+//SZ32: define{{.*}} i32 @test_private(i8* %x)
+//SZ32: ptrtoint i8* %{{.*}} to i32
+//SZ64: define{{.*}} i64 @test_private(i8* %x)
+//SZ64: ptrtoint i8* %{{.*}} to i64
+size_t test_private(private char* x) {
+  return (size_t)x;
+}
+
+//SZ32: define{{.*}} i32 @test_global(i8 addrspace(1)* %x)
+//SZ32: ptrtoint i8 addrspace(1)* %{{.*}} to i32
+//SZ64: define{{.*}} i64 @test_global(i8 addrspace(1)* %x)
+//SZ64: ptrtoint i8 addrspace(1)* %{{.*}} to i64
+intptr_t test_global(global char* x) {
+  return (intptr_t)x;
+}
+
+//SZ32: define{{.*}} i32 @test_constant(i8 addrspace(2)* %x)
+//SZ32: ptrtoint i8 addrspace(2)* %{{.*}} to i32
+//SZ64: define{{.*}} i64 @test_constant(i8 addrspace(2)* %x)
+//SZ64: ptrtoint i8 addrspace(2)* %{{.*}} to i64
+uintptr_t test_constant(constant char* x) {
+  return (uintptr_t)x;
+}
+
+//SZ32: define{{.*}} i32 @test_local(i8 addrspace(3)* %x, i8 addrspace(3)* %y)
+//SZ32: ptrtoint i8 addrspace(3)* %{{.*}} to i32
+//SZ32: ptrtoint i8 addrspace(3)* %{{.*}} to i32
+//SZ64: define{{.*}} i64 @test_local(i8 addrspace(3)* %x, i8 addrspace(3)* %y)
+//SZ64: ptrtoint i8 addrspace(3)* %{{.*}} to i64
+//SZ64: ptrtoint i8 addrspace(3)* %{{.*}} to i64
+ptrdiff_t test_local(local char* x, local char *y) {
+  return x - y;
+}
+
+//SZ32: define{{.*}} i32 @test_generic(i8 addrspace(4)* %x)
+//SZ32: ptrtoint i8 addrspace(4)* %{{.*}} to i32
+//SZ64: define{{.*}} i64 @test_generic(i8 addrspace(4)* %x)
+//SZ64: ptrtoint i8 addrspace(4)* %{{.*}} to i64
+size_t test_generic(generic char* x) {
+  return (size_t)x;
+}
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -104,7 +104,8 @@
   IntAlignInBytes =
 C.toCharUnitsFromBits(C.getTargetInfo().getIntAlign()).getQuantity();
   IntTy = llvm::IntegerType::get(LLVMContext, C.getTargetInfo().getIntWidth());
-  IntPtrTy = llvm::IntegerType::get(LLVMContext, PointerWidthInBits);
+  IntPtrTy = llvm::IntegerType::get(LLVMContext, LangOpts.OpenCL ?
+  C.getTargetInfo().getOpenCLMaxPointerWidth() : PointerWidthInBits);
   Int8PtrTy = Int8Ty->getPointerTo(0);
   Int8PtrPtrTy = Int8PtrTy->getPointerTo(0);
 
Index: lib/Basic/Targets.cpp
===
--- lib/Basic/Targets.cpp
+++ lib/Basic/Targets.cpp
@@ -2004,6 +2004,12 @@
 }
   }
 
+  uint64_t getOpenCLMaxPointerWidth() const override {
+if (GPU <= GK_CAYMAN)
+  return 32;
+return 64;
+  }
+
   const char * getClobbers() const override {
 return "";
   }
Index: lib/Basic/TargetInfo.cpp
===
--- lib/Basic/TargetInfo.cpp
+++ lib/Basic/TargetInfo.cpp
@@ -306,8 +306,9 @@
 }
 LongDoubleWidth = LongDoubleAlign = 128;
 
-assert(PointerWidth == 32 || PointerWidth == 64);
-bool Is32BitArch = PointerWidth == 32;
+unsigned MaxPointerWidth = getOpenCLMaxPointerWidth();
+assert(MaxPointerWidth == 32 || MaxPointerWidth == 64);
+bool Is32BitArch = MaxPointerWidth == 32;
 SizeType = Is32BitArch ? UnsignedInt : UnsignedLong;
 PtrDiffType = Is32BitArch ? SignedInt : SignedLong;
 IntPtrType = Is32BitArch ? SignedInt : SignedLong;
Index: include/clang/Basic/TargetInfo.h
===
--- include/clang/Basic/TargetInfo.h
+++ include/clang/Basic/TargetInfo.h
@@ -294,6 +294,11 @@
 return AddrSpace == 0 ? PointerAlign : getPointerAlignV(AddrSpace);
   }
 
+  /// \brief Return the maximum width of pointers for OpenCL on this target.
+  virtual uint64_t getOpenCLMaxPointerWidth() const {
+return PointerWidth;
+  }
+
   /// \brief Return the size of '_Bool' and C++ 'bool' for this target, in bits.
   unsigned getBoolWidth() const { return BoolWid

Re: [PATCH] D23353: [clang-tidy] Add check 'misc-use-after-move'

2016-08-10 Thread Eugene Zelenko via cfe-commits
Eugene.Zelenko added a subscriber: Eugene.Zelenko.


Comment at: docs/ReleaseNotes.rst:75
@@ +74,3 @@
+
+  This check warns if an object is used after it has been moved, without an
+  intervening reinitialization.

Please remove //This check// and capitalize //warns//


Comment at: docs/clang-tidy/checks/misc-use-after-move.rst:15
@@ +14,3 @@
+
+The last line will trigger a warning that ``str`` is used after it has been
+moved.

str is not language construct, please use `.


Comment at: docs/clang-tidy/checks/misc-use-after-move.rst:51
@@ +50,3 @@
+In some cases, the check may not be able to detect that two branches are
+mutually exclusive. For example (assuming that ``i`` is an int):
+

i is not language construct, please use `.


Comment at: docs/clang-tidy/checks/misc-use-after-move.rst:78
@@ +77,3 @@
+
+No warnings are emitted for objects of type std::unique_ptr and 
std::shared_ptr,
+as they have defined move behavior. (Objects of these classes are guaranteed to

Please highlight std::unique_ptr and std::shared_ptr with ``


Comment at: docs/clang-tidy/checks/misc-use-after-move.rst:181
@@ +180,3 @@
+  .. code-block:: c++
+  struct S {
+std::string str;

Please insert empty line and indent code as in previous examples.


Comment at: docs/clang-tidy/checks/misc-use-after-move.rst:190
@@ +189,3 @@
+
+The check will not consider ``s`` to be reinitialized after the last line;
+instead, the line that assigns to ``s.str`` will be flagged as a 
use-after-move.

s, s.str and S are not language construct. Please use `.


https://reviews.llvm.org/D23353



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] r278254 - [clang-tidy] minor improvements in modernise-deprecated-headers check

2016-08-10 Thread Kirill Bobyrev via cfe-commits
Author: omtcyfz
Date: Wed Aug 10 13:01:45 2016
New Revision: 278254

URL: http://llvm.org/viewvc/llvm-project?rev=278254&view=rev
Log:
[clang-tidy] minor improvements in modernise-deprecated-headers check

This patch introduces a minor list of changes as proposed by Richard Smith in
the mailing list.

See original comments with an impact on the future check state below:

[comments.begin

> +  {"complex.h", "ccomplex"},

It'd be better to convert this one to , or leave it alone.
 is an unnecessary wart.

(The contents of C++11's  /  /  (all of
which are identical) aren't comparable to C99's , so if
this was C++98 code using the C99 header, the code will be broken with
or without this transformation.)

> +  {"iso646.h", "ciso646"},

Just delete #includes of this one.  does nothing.

> +  {"stdalign.h", "cstdalign"},
> +  {"stdbool.h", "cstdbool"},

We should just delete these two includes. These headers do nothing in C++.

comments.end]

Reviewers: alexfh, aaron.ballman

Differential Revision: https://reviews.llvm.org/D17990


Modified:
clang-tools-extra/trunk/clang-tidy/modernize/DeprecatedHeadersCheck.cpp

clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-deprecated-headers.rst

clang-tools-extra/trunk/test/clang-tidy/modernize-deprecated-headers-cxx03.cpp

clang-tools-extra/trunk/test/clang-tidy/modernize-deprecated-headers-cxx11.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/DeprecatedHeadersCheck.cpp?rev=278254&r1=278253&r2=278254&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/DeprecatedHeadersCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/DeprecatedHeadersCheck.cpp Wed 
Aug 10 13:01:45 2016
@@ -12,6 +12,7 @@
 #include "clang/Lex/PPCallbacks.h"
 #include "clang/Lex/Preprocessor.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringSet.h"
 
 #include 
 
@@ -35,6 +36,7 @@ private:
   ClangTidyCheck &Check;
   LangOptions LangOpts;
   llvm::StringMap CStyledHeaderToCxx;
+  llvm::StringSet<> DeleteHeaders;
 };
 } // namespace
 
@@ -51,16 +53,23 @@ IncludeModernizePPCallbacks::IncludeMode
 : Check(Check), LangOpts(LangOpts) {
   for (const auto &KeyValue :
std::vector>(
-   {{"assert.h", "cassert"}, {"complex.h", "ccomplex"},
-{"ctype.h", "cctype"},   {"errno.h", "cerrno"},
-{"float.h", "cfloat"},   {"inttypes.h", "cinttypes"},
-{"iso646.h", "ciso646"}, {"limits.h", "climits"},
-{"locale.h", "clocale"}, {"math.h", "cmath"},
-{"setjmp.h", "csetjmp"}, {"signal.h", "csignal"},
-{"stdarg.h", "cstdarg"}, {"stddef.h", "cstddef"},
-{"stdint.h", "cstdint"}, {"stdio.h", "cstdio"},
-{"stdlib.h", "cstdlib"}, {"string.h", "cstring"},
-{"time.h", "ctime"}, {"wchar.h", "cwchar"},
+   {{"assert.h", "cassert"},
+{"complex.h", "complex"},
+{"ctype.h", "cctype"},
+{"errno.h", "cerrno"},
+{"float.h", "cfloat"},
+{"limits.h", "climits"},
+{"locale.h", "clocale"},
+{"math.h", "cmath"},
+{"setjmp.h", "csetjmp"},
+{"signal.h", "csignal"},
+{"stdarg.h", "cstdarg"},
+{"stddef.h", "cstddef"},
+{"stdio.h", "cstdio"},
+{"stdlib.h", "cstdlib"},
+{"string.h", "cstring"},
+{"time.h", "ctime"},
+{"wchar.h", "cwchar"},
 {"wctype.h", "cwctype"}})) {
 CStyledHeaderToCxx.insert(KeyValue);
   }
@@ -69,13 +78,17 @@ IncludeModernizePPCallbacks::IncludeMode
 for (const auto &KeyValue :
  std::vector>(
  {{"fenv.h", "cfenv"},
-  {"stdalign.h", "cstdalign"},
-  {"stdbool.h", "cstdbool"},
+  {"stdint.h", "cstdint"},
+  {"inttypes.h", "cinttypes"},
   {"tgmath.h", "ctgmath"},
   {"uchar.h", "cuchar"}})) {
   CStyledHeaderToCxx.insert(KeyValue);
 }
   }
+  for (const auto &Key :
+   std::vector({"stdalign.h", "stdbool.h", "iso646.h"})) {
+DeleteHeaders.insert(Key);
+  }
 }
 
 void IncludeModernizePPCallbacks::InclusionDirective(
@@ -86,17 +99,22 @@ void IncludeModernizePPCallbacks::Inclus
   //
   // Reasonable options for the check:
   //
-  // 1. Insert std prefix for every such symbol occurance.
+  // 1. Insert std prefix for every such symbol occurrence.
   // 2. Insert `using namespace std;` to the beginning of TU.
   // 3. Do nothing and let the user deal with the migration himself.
   if (CStyledHeaderToCxx.count(FileName) != 0) {
 std::string Replacement =
 (llvm::Twine("<") + CStyledHeaderToCxx[FileName] + ">").str();
-Check.diag(Filenam

[clang-tools-extra] r278255 - [Documentation] Fix grammar mistakes in docs/clang-tidy/index.rst spotted by Alexander Kornienko.

2016-08-10 Thread Eugene Zelenko via cfe-commits
Author: eugenezelenko
Date: Wed Aug 10 13:02:15 2016
New Revision: 278255

URL: http://llvm.org/viewvc/llvm-project?rev=278255&view=rev
Log:
[Documentation] Fix grammar mistakes in docs/clang-tidy/index.rst spotted by 
Alexander Kornienko.

Modified:
clang-tools-extra/trunk/docs/clang-tidy/index.rst

Modified: clang-tools-extra/trunk/docs/clang-tidy/index.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/index.rst?rev=278255&r1=278254&r2=278255&view=diff
==
--- clang-tools-extra/trunk/docs/clang-tidy/index.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/index.rst Wed Aug 10 13:02:15 2016
@@ -246,7 +246,7 @@ rest of this document explains how to do
 There are a few tools particularly useful when developing clang-tidy checks:
   * ``add_new_check.py`` is a script to automate the process of adding a new
 check, it will create the check, update the CMake file and create a test;
-  * ``rename_check.py`` does what the script name suggest, renames an existing
+  * ``rename_check.py`` does what the script name suggests, renames an existing
 check;
   * :program:`clang-query` is invaluable for interactive prototyping of AST
 matchers and exploration of the Clang AST;


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D17990: [clang-tidy] minor improvements in modernise-deprecated-headers check

2016-08-10 Thread Kirill Bobyrev via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL278254: [clang-tidy] minor improvements in 
modernise-deprecated-headers check (authored by omtcyfz).

Changed prior to commit:
  https://reviews.llvm.org/D17990?vs=67551&id=67560#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D17990

Files:
  clang-tools-extra/trunk/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
  
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-deprecated-headers.rst
  clang-tools-extra/trunk/test/clang-tidy/modernize-deprecated-headers-cxx03.cpp
  clang-tools-extra/trunk/test/clang-tidy/modernize-deprecated-headers-cxx11.cpp

Index: clang-tools-extra/trunk/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
@@ -12,6 +12,7 @@
 #include "clang/Lex/PPCallbacks.h"
 #include "clang/Lex/Preprocessor.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringSet.h"
 
 #include 
 
@@ -35,6 +36,7 @@
   ClangTidyCheck &Check;
   LangOptions LangOpts;
   llvm::StringMap CStyledHeaderToCxx;
+  llvm::StringSet<> DeleteHeaders;
 };
 } // namespace
 
@@ -51,31 +53,42 @@
 : Check(Check), LangOpts(LangOpts) {
   for (const auto &KeyValue :
std::vector>(
-   {{"assert.h", "cassert"}, {"complex.h", "ccomplex"},
-{"ctype.h", "cctype"},   {"errno.h", "cerrno"},
-{"float.h", "cfloat"},   {"inttypes.h", "cinttypes"},
-{"iso646.h", "ciso646"}, {"limits.h", "climits"},
-{"locale.h", "clocale"}, {"math.h", "cmath"},
-{"setjmp.h", "csetjmp"}, {"signal.h", "csignal"},
-{"stdarg.h", "cstdarg"}, {"stddef.h", "cstddef"},
-{"stdint.h", "cstdint"}, {"stdio.h", "cstdio"},
-{"stdlib.h", "cstdlib"}, {"string.h", "cstring"},
-{"time.h", "ctime"}, {"wchar.h", "cwchar"},
+   {{"assert.h", "cassert"},
+{"complex.h", "complex"},
+{"ctype.h", "cctype"},
+{"errno.h", "cerrno"},
+{"float.h", "cfloat"},
+{"limits.h", "climits"},
+{"locale.h", "clocale"},
+{"math.h", "cmath"},
+{"setjmp.h", "csetjmp"},
+{"signal.h", "csignal"},
+{"stdarg.h", "cstdarg"},
+{"stddef.h", "cstddef"},
+{"stdio.h", "cstdio"},
+{"stdlib.h", "cstdlib"},
+{"string.h", "cstring"},
+{"time.h", "ctime"},
+{"wchar.h", "cwchar"},
 {"wctype.h", "cwctype"}})) {
 CStyledHeaderToCxx.insert(KeyValue);
   }
   // Add C++ 11 headers.
   if (LangOpts.CPlusPlus11) {
 for (const auto &KeyValue :
  std::vector>(
  {{"fenv.h", "cfenv"},
-  {"stdalign.h", "cstdalign"},
-  {"stdbool.h", "cstdbool"},
+  {"stdint.h", "cstdint"},
+  {"inttypes.h", "cinttypes"},
   {"tgmath.h", "ctgmath"},
   {"uchar.h", "cuchar"}})) {
   CStyledHeaderToCxx.insert(KeyValue);
 }
   }
+  for (const auto &Key :
+   std::vector({"stdalign.h", "stdbool.h", "iso646.h"})) {
+DeleteHeaders.insert(Key);
+  }
 }
 
 void IncludeModernizePPCallbacks::InclusionDirective(
@@ -86,17 +99,22 @@
   //
   // Reasonable options for the check:
   //
-  // 1. Insert std prefix for every such symbol occurance.
+  // 1. Insert std prefix for every such symbol occurrence.
   // 2. Insert `using namespace std;` to the beginning of TU.
   // 3. Do nothing and let the user deal with the migration himself.
   if (CStyledHeaderToCxx.count(FileName) != 0) {
 std::string Replacement =
 (llvm::Twine("<") + CStyledHeaderToCxx[FileName] + ">").str();
-Check.diag(FilenameRange.getBegin(),
-   "inclusion of deprecated C++ header '%0'; consider using '%1' instead")
+Check.diag(FilenameRange.getBegin(), "inclusion of deprecated C++ header "
+ "'%0'; consider using '%1' instead")
 << FileName << CStyledHeaderToCxx[FileName]
 << FixItHint::CreateReplacement(FilenameRange.getAsRange(),
 Replacement);
+  } else if (DeleteHeaders.count(FileName) != 0) {
+Check.diag(FilenameRange.getBegin(),
+   "including '%0' has no effect in C++; consider removing it")
+<< FileName << FixItHint::CreateRemoval(
+   SourceRange(HashLoc, FilenameRange.getEnd()));
   }
 }
 
Index: clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-deprecated-headers.rst
===
--- clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-deprecated-headers.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-deprecated-headers.rst
@@ -4,10 +4,11 @@
 

[clang-tools-extra] r278257 - [clang-tidy] enhance readability-else-after-return

2016-08-10 Thread Kirill Bobyrev via cfe-commits
Author: omtcyfz
Date: Wed Aug 10 13:05:47 2016
New Revision: 278257

URL: http://llvm.org/viewvc/llvm-project?rev=278257&view=rev
Log:
[clang-tidy] enhance readability-else-after-return

`readability-else-after-return` only warns about `return` calls, but LLVM Coding
Standars stat that `throw`, `continue`, `goto`, etc after `return` calls are
bad, too.

Reviwers: alexfh, aaron.ballman

Differential Revision: https://reviews.llvm.org/D23265

Modified:
clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp

clang-tools-extra/trunk/docs/clang-tidy/checks/readability-else-after-return.rst
clang-tools-extra/trunk/test/clang-tidy/readability-else-after-return.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp?rev=278257&r1=278256&r2=278257&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp Wed 
Aug 10 13:05:47 2016
@@ -19,20 +19,29 @@ namespace tidy {
 namespace readability {
 
 void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) {
-  // FIXME: Support continue, break and throw.
+  const auto ControlFlowInterruptorMatcher =
+  stmt(anyOf(returnStmt().bind("return"), continueStmt().bind("continue"),
+ breakStmt().bind("break"), cxxThrowExpr().bind("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(ControlFlowInterruptorMatcher,
+   compoundStmt(has(ControlFlowInterruptorMatcher),
+ 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");
+  std::string ControlFlowInterruptor;
+  for (const auto *BindingName : {"return", "continue", "break", "throw"})
+if (Result.Nodes.getNodeAs(BindingName))
+  ControlFlowInterruptor = BindingName;
+
+  DiagnosticBuilder Diag = diag(ElseLoc, "do not use 'else' after '%0'")
+   << ControlFlowInterruptor;
   Diag << tooling::fixit::createRemoval(ElseLoc);
 
   // FIXME: Removing the braces isn't always safe. Do a more careful analysis.

Modified: 
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-else-after-return.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/readability-else-after-return.rst?rev=278257&r1=278256&r2=278257&view=diff
==
--- 
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-else-after-return.rst
 (original)
+++ 
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-else-after-return.rst
 Wed Aug 10 13:05:47 2016
@@ -3,7 +3,62 @@
 readability-else-after-return
 =
 
+`LLVM Coding Standards `_ advises to
+reduce indentation where possible and where it makes understanding code easier.
+Early exit is one of the suggested enforcements of that. Please do not use
+`else` or `else if` after something that interrupts control flow - like
+`return`, `break`, `continue`, `throw`.
 
-Flags the usages of ``else`` after ``return``.
+The following piece of code illustrates how the check works. This piece of 
code:
 
-http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return
+.. code-block:: c++
+
+void foo(int Value) {
+  int Local = 0;
+  for (int i = 0; i < 42; i++) {
+if (Value == 1) {
+  return;
+} else {
+  Local++;
+}
+
+if (Value == 2)
+  continue;
+else
+  Local++;
+
+if (Value == 3) {
+  throw 42;
+} else {
+  Local++;
+}
+  }
+}
+
+
+Would be transformed into:
+
+.. code-block:: c++
+
+void foo(int Value) {
+  int Local = 0;
+  for (int i = 0; i < 42; i++) {
+if (Value == 1) {
+  return;
+}
+Local++;
+
+if (Value == 2)
+  continue;
+Local++;
+
+if (Value == 3) {
+  throw 42;
+}
+Local++;
+  }
+}
+
+
+This checks helps to enforce this `Coding Standars recommendation
+`_.

Modified: 

Re: [PATCH] D23265: [clang-tidy] enhance readability-else-after-return

2016-08-10 Thread Kirill Bobyrev via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL278257: [clang-tidy] enhance readability-else-after-return 
(authored by omtcyfz).

Changed prior to commit:
  https://reviews.llvm.org/D23265?vs=67513&id=67561#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D23265

Files:
  clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp
  
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-else-after-return.rst
  clang-tools-extra/trunk/test/clang-tidy/readability-else-after-return.cpp

Index: clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -19,20 +19,29 @@
 namespace readability {
 
 void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) {
-  // FIXME: Support continue, break and throw.
+  const auto ControlFlowInterruptorMatcher =
+  stmt(anyOf(returnStmt().bind("return"), continueStmt().bind("continue"),
+ breakStmt().bind("break"), cxxThrowExpr().bind("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(ControlFlowInterruptorMatcher,
+   compoundStmt(has(ControlFlowInterruptorMatcher),
+ 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");
+  std::string ControlFlowInterruptor;
+  for (const auto *BindingName : {"return", "continue", "break", "throw"})
+if (Result.Nodes.getNodeAs(BindingName))
+  ControlFlowInterruptor = BindingName;
+
+  DiagnosticBuilder Diag = diag(ElseLoc, "do not use 'else' after '%0'")
+   << ControlFlowInterruptor;
   Diag << tooling::fixit::createRemoval(ElseLoc);
 
   // FIXME: Removing the braces isn't always safe. Do a more careful analysis.
Index: clang-tools-extra/trunk/docs/clang-tidy/checks/readability-else-after-return.rst
===
--- clang-tools-extra/trunk/docs/clang-tidy/checks/readability-else-after-return.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/readability-else-after-return.rst
@@ -3,7 +3,62 @@
 readability-else-after-return
 =
 
+`LLVM Coding Standards `_ advises to
+reduce indentation where possible and where it makes understanding code easier.
+Early exit is one of the suggested enforcements of that. Please do not use
+`else` or `else if` after something that interrupts control flow - like
+`return`, `break`, `continue`, `throw`.
 
-Flags the usages of ``else`` after ``return``.
+The following piece of code illustrates how the check works. This piece of code:
 
-http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return
+.. code-block:: c++
+
+void foo(int Value) {
+  int Local = 0;
+  for (int i = 0; i < 42; i++) {
+if (Value == 1) {
+  return;
+} else {
+  Local++;
+}
+
+if (Value == 2)
+  continue;
+else
+  Local++;
+
+if (Value == 3) {
+  throw 42;
+} else {
+  Local++;
+}
+  }
+}
+
+
+Would be transformed into:
+
+.. code-block:: c++
+
+void foo(int Value) {
+  int Local = 0;
+  for (int i = 0; i < 42; i++) {
+if (Value == 1) {
+  return;
+}
+Local++;
+
+if (Value == 2)
+  continue;
+Local++;
+
+if (Value == 3) {
+  throw 42;
+}
+Local++;
+  }
+}
+
+
+This checks helps to enforce this `Coding Standars recommendation
+`_.
Index: clang-tools-extra/trunk/test/clang-tidy/readability-else-after-return.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/readability-else-after-return.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/readability-else-after-return.cpp
@@ -3,16 +3,16 @@
 void f(int a) {
   if (a > 0)
 return;
-  else // comment
-// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: don't use else after return
-// CHECK-FIXES: {{^}}  // comment
+  else // comment-0
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'else' after 'return'
+

Re: [PATCH] D23125: Modules: add command line option to support loading prebuilt modules on demand, without parsing any module map

2016-08-10 Thread Manman Ren via cfe-commits
manmanren added a comment.

In https://reviews.llvm.org/D23125#510632, @rsmith wrote:

> This doesn't seem like quite the right user interface for this feature. The 
> module cache is volatile, and it's not really reasonable for people to assume 
> they know what is and isn't within it. Instead, it seems like what we want to 
> provide is zero or more paths to directories containing prebuilt .pcm files 
> whose file names are the same as their top-level module names, completely 
> separate from the module cache.
>
> Specifically, rather than spelling this as `-fmodules-use-prebuilt-modules 
> -fmodules-cache-path=X`, I'd suggest spelling it as 
> `-fprebuilt-module-path=X` or similar (which can be repeated to provide 
> multiple search paths). That would then combine orthogonally with 
> `-fno-implicit-modules` and `-fno-implicit-module-maps`, where implicit 
> modules would still use the module cache. Does that make sense for your use 
> case?


What we want is to achieve performance by prebuilding the modules and using 
@import to load the corresponding pcm without  parsing the module maps. In 
ASTReader, we want to treat these modules in the same way as the explicit 
modules.

-fprebuilt-module-path sounds reasonable. Let me see how the implementation 
goes ...

> You're also missing updates to the modules documentation.


Yes, I will update the document.

Cheers,
Manman



Comment at: lib/Frontend/CompilerInstance.cpp:1502-1510
@@ -1477,1 +1501,11 @@
 case ASTReader::Missing: {
+  if (HSOpts.ModulesUsePrebuiltModules) {
+// We can't rebuild the module without a module map. Throw diagnostics
+// here if using prebuilt modules.
+getDiagnostics().Report(ModuleNameLoc, diag::err_module_not_found)
+<< ModuleName
+<< SourceRange(ImportLoc, ModuleNameLoc);
+ModuleBuildFailed = true;
+return ModuleLoadResult();
+  }
+

rsmith wrote:
> You requested that `ReadAST` produces diagnostics for the `OutOfDate` and 
> `Missing` cases when using prebuilt modules, so this diagnostic is redundant.
Yes, I can remove this diagnostic.


Comment at: lib/Serialization/ASTReader.cpp:495-498
@@ -494,1 +494,6 @@
 
+static bool isPrebuiltModule(Preprocessor &PP, ModuleKind MK) {
+  return (MK == MK_ExplicitModule || MK == MK_ImplicitModule) &&
+  PP.getHeaderSearchInfo().getHeaderSearchOpts().ModulesUsePrebuiltModules;
+}
+

rsmith wrote:
> This seems unnecessarily complicated. Perhaps you could make 
> `CompilerInstance::loadModule` treat all prebuilt modules as 
> `MK_ExplicitModule`, or add a third `ModuleKind` for them?
I tried adding a ModuleKind at some point, but somehow it didn't work out. I 
will take another look at that.


https://reviews.llvm.org/D23125



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D23343: [clang-tidy] modernize-make-{smart_ptr} private ctor bugfix

2016-08-10 Thread Eugene Zelenko via cfe-commits
Eugene.Zelenko added a subscriber: Eugene.Zelenko.


Comment at: docs/ReleaseNotes.rst:78
@@ -77,1 +77,3 @@
 
+- Bugfix for `modernize-make-unique
+  `_

I think will be better to have //Fixed bugs:// section as we had in 3.9 release 
notes.


https://reviews.llvm.org/D23343



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] r278262 - [Release Notes] Consistency in Clang-tidy entries' style.

2016-08-10 Thread Eugene Zelenko via cfe-commits
Author: eugenezelenko
Date: Wed Aug 10 13:15:51 2016
New Revision: 278262

URL: http://llvm.org/viewvc/llvm-project?rev=278262&view=rev
Log:
[Release Notes] Consistency in Clang-tidy entries' style.

Modified:
clang-tools-extra/trunk/docs/ReleaseNotes.rst

Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=278262&r1=278261&r2=278262&view=diff
==
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Wed Aug 10 13:15:51 2016
@@ -72,9 +72,8 @@ Improvements to clang-tidy
 - New `performance-inefficient-string-concatenation
   
`_
 check
 
-  This check warns about the performance overhead arising from concatenating
-  strings using the ``operator+``, instead of ``operator+=``.
-
+  Warns about the performance overhead arising from concatenating strings using
+  the ``operator+``, instead of ``operator+=``.
 
 Improvements to include-fixer
 -


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D23343: [clang-tidy] modernize-make-{smart_ptr} private ctor bugfix

2016-08-10 Thread Aaron Ballman via cfe-commits
aaron.ballman added inline comments.


Comment at: test/clang-tidy/modernize-make-shared.cpp:109
@@ +108,3 @@
+  void create() {
+auto ptr = std::shared_ptr(new Private(42));
+  }

Prazek wrote:
> aaron.ballman wrote:
> > Add comments explaining why make_shared is not correct. Also, please add a 
> > case showing that protected constructors still get the proper fix applied.
> You mean I should also not warn when the constructor is protected? Make sense.
Correct, protected constructors should be handled like private constructors.


https://reviews.llvm.org/D23343



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


  1   2   >