[PATCH] D25897: Add __no_instrument_function__ to _LIBCPP_INLINE_VISIBILITY and _LIBCPP_ALWAYS_INLINE

2016-11-05 Thread Eric Fiselier via cfe-commits
EricWF added a comment.

I created a fix as described above. Could you please tell me if it works for 
you?

It can be found here:

https://reviews.llvm.org/D26324


https://reviews.llvm.org/D25897



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


[PATCH] D26306: [AVX-512] Make VBMI instruction set enabling imply that the BWI instruction set is also enabled.

2016-11-05 Thread Zvi Rackover via cfe-commits
zvi accepted this revision.
zvi added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D26306



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


[PATCH] D26306: [AVX-512] Make VBMI instruction set enabling imply that the BWI instruction set is also enabled.

2016-11-05 Thread Zvi Rackover via cfe-commits
zvi added inline comments.



Comment at: lib/Basic/Targets.cpp:3353
   setSSELevel(Features, AVX512F, Enabled);
+// Enable BWI instruction is VBMI is being enabled.
+if (Name == "avx512vbmi" && Enabled)

is -> if



Comment at: test/Preprocessor/x86_target_features.c:212
 
+// RUN: %clang -target i386-unknown-unknown -march=atom -mavx512vbmi 
-mno-avx512vbmi -x c -E -dM -o - %s | FileCheck -match-full-lines 
--check-prefix=AVX512VBMINOAVX512BW %s
+

Test is turning on and then off the same feature. Did you mean -mavx512bw 
-mno-avx512vbmi ?

If the latter, I would prefer the compiler shout at me for providing 
contradicting flags than let the last flag win. Consider enforcing this in 
handleTargetFeatures().




https://reviews.llvm.org/D26306



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


[PATCH] D26282: [PowerPC] Implement plain VSX load/store builtins.

2016-11-05 Thread Tony Jiang via cfe-commits
jtony updated this revision to Diff 76945.
jtony added a comment.

Reorder the overloads according to their size.


https://reviews.llvm.org/D26282

Files:
  lib/Headers/altivec.h
  test/CodeGen/builtins-ppc-altivec.c
  test/CodeGen/builtins-ppc-quadword.c
  test/CodeGen/builtins-ppc-vsx.c

Index: test/CodeGen/builtins-ppc-vsx.c
===
--- test/CodeGen/builtins-ppc-vsx.c
+++ test/CodeGen/builtins-ppc-vsx.c
@@ -21,6 +21,7 @@
 vector signed long long vsll = { 255LL, -937LL };
 vector unsigned long long vull = { 1447LL, 2894LL };
 double d = 23.4;
+signed long long sll = 618LL;
 float af[4] = {23.4f, 56.7f, 89.0f, 12.3f};
 double ad[2] = {23.4, 56.7};
 signed char asc[16] = { -8,  9, -10, 11, -12, 13, -14, 15,
@@ -31,8 +32,8 @@
 unsigned short aus[8] = { 0, 1, 2, 3, 4, 5, 6, 7 };
 signed int asi[4] = { -1, 2, -3, 4 };
 unsigned int aui[4] = { 0, 1, 2, 3 };
-signed long asl[2] = { -1L, 2L };
-unsigned long aul[2] = { 1L, 2L };
+signed long long asll[2] = { -1L, 2L };
+unsigned long long aull[2] = { 1L, 2L };
 
 vector float res_vf;
 vector double res_vd;
@@ -1248,4 +1249,28 @@
   res_vull = vec_sro(vull, vuc);
 // CHECK: @llvm.ppc.altivec.vsro
 // CHECK-LE: @llvm.ppc.altivec.vsro
+
+res_vsll = vec_xl(sll, asll);
+// CHECK: load <2 x i64>, <2 x i64>* %{{[0-9]+}}, align 16
+// CHECK-LE: load <2 x i64>, <2 x i64>* %{{[0-9]+}}, align 16
+
+res_vull = vec_xl(sll, aull);
+// CHECK: load <2 x i64>, <2 x i64>* %{{[0-9]+}}, align 16
+// CHECK-LE: load <2 x i64>, <2 x i64>* %{{[0-9]+}}, align 16
+
+res_vd = vec_xl(sll, ad);
+// CHECK: load <2 x double>, <2 x double>* %{{[0-9]+}}, align 16
+// CHECK-LE: load <2 x double>, <2 x double>* %{{[0-9]+}}, align 16
+
+vec_xst(vsll, sll, asll);
+// CHECK: store <2 x i64> %{{[0-9]+}}, <2 x i64>* %{{[0-9]+}}, align 16
+// CHECK-LE: store <2 x i64> %{{[0-9]+}}, <2 x i64>* %{{[0-9]+}}, align 16
+
+vec_xst(vull, sll, aull);
+// CHECK: store <2 x i64> %{{[0-9]+}}, <2 x i64>* %{{[0-9]+}}, align 16
+// CHECK-LE: store <2 x i64> %{{[0-9]+}}, <2 x i64>* %{{[0-9]+}}, align 16
+
+vec_xst(vd, sll, ad);
+// CHECK: store <2 x double> %{{[0-9]+}}, <2 x double>* %{{[0-9]+}}, align 16
+// CHECK-LE: store <2 x double> %{{[0-9]+}}, <2 x double>* %{{[0-9]+}}, align 16
 }
Index: test/CodeGen/builtins-ppc-quadword.c
===
--- test/CodeGen/builtins-ppc-quadword.c
+++ test/CodeGen/builtins-ppc-quadword.c
@@ -15,6 +15,12 @@
 // CHECK-PPC: error: __int128 is not supported on this target
 vector unsigned __int128 vulll = { 1 };
 
+signed long long param_sll;
+// CHECK-PPC: error: __int128 is not supported on this target
+signed __int128 param_lll;
+// CHECK-PPC: error: __int128 is not supported on this target
+unsigned __int128 param_ulll;
+
 // CHECK-PPC: error: __int128 is not supported on this target
 vector signed __int128 res_vlll;
 // CHECK-PPC: error: __int128 is not supported on this target
@@ -165,4 +171,26 @@
 // CHECK-LE: xor <16 x i8>
 // CHECK-LE: call <4 x i32> @llvm.ppc.altivec.vperm(<4 x i32> {{%.+}}, <4 x i32> {{%.+}}, <16 x i8> {{%.+}})
 // CHECK_PPC: error: call to 'vec_revb' is ambiguous
+
+  /* vec_xl */
+  res_vlll = vec_xl(param_sll, ¶m_lll);
+  // CHECK: load <1 x i128>, <1 x i128>* %{{[0-9]+}}, align 16
+  // CHECK-LE: load <1 x i128>, <1 x i128>* %{{[0-9]+}}, align 16
+  // CHECK-PPC: error: call to 'vec_xl' is ambiguous
+
+  res_vulll = vec_xl(param_sll, ¶m_ulll);
+  // CHECK: load <1 x i128>, <1 x i128>* %{{[0-9]+}}, align 16
+  // CHECK-LE: load <1 x i128>, <1 x i128>* %{{[0-9]+}}, align 16
+  // CHECK-PPC: error: call to 'vec_xl' is ambiguous
+
+  /* vec_xst */
+   vec_xst(vlll, param_sll, ¶m_lll);
+  // CHECK: store <1 x i128> %{{[0-9]+}}, <1 x i128>* %{{[0-9]+}}, align 16
+  // CHECK-LE: store <1 x i128> %{{[0-9]+}}, <1 x i128>* %{{[0-9]+}}, align 16
+  // CHECK-PPC: error: call to 'vec_xst' is ambiguous
+
+   vec_xst(vulll, param_sll, ¶m_ulll);
+  // CHECK: store <1 x i128> %{{[0-9]+}}, <1 x i128>* %{{[0-9]+}}, align 16
+  // CHECK-LE: store <1 x i128> %{{[0-9]+}}, <1 x i128>* %{{[0-9]+}}, align 16
+  // CHECK-PPC: error: call to 'vec_xst' is ambiguous
 }
Index: test/CodeGen/builtins-ppc-altivec.c
===
--- test/CodeGen/builtins-ppc-altivec.c
+++ test/CodeGen/builtins-ppc-altivec.c
@@ -45,6 +45,7 @@
 int param_i;
 unsigned int param_ui;
 float param_f;
+signed long long param_sll;
 
 int res_sc;
 int res_uc;
@@ -9200,3 +9201,69 @@
 // CHECK-LE: xor <16 x i8>
 // CHECK-LE: call <4 x i32> @llvm.ppc.altivec.vperm(<4 x i32> {{%.+}}, <4 x i32> {{%.+}}, <16 x i8> {{%.+}})
 }
+
+/* -- vec_xl  */
+void test9() {
+  // CHECK-LABEL: define void @test9
+  // CHECK-LE-LABEL: define void @test9
+  res_vsc = vec_xl(param_sll, ¶m_sc);
+  // CHECK: load <16 x i8>, <16 x i8>* %{{[0-9]+}}, align 16
+  // CHECK-LE: load <16 x i8>, <16 x i8>* %{{[0-9

[PATCH] D26138: [clang-tidy] Add modernize-use-equals-delete check

2016-11-05 Thread Malcolm Parsons via cfe-commits
malcolm.parsons planned changes to this revision.
malcolm.parsons added inline comments.



Comment at: clang-tidy/modernize/UseEqualsDeleteCheck.cpp:58
+  << FixItHint::CreateInsertion(StartLoc, "public: ")
+  << FixItHint::CreateInsertion(AfterLoc, " private:");
+}

aaron.ballman wrote:
> malcolm.parsons wrote:
> > aaron.ballman wrote:
> > > I am on the fence about this fixit. On the one hand, the fix is a 
> > > technical improvement because it means that implementations will 
> > > consistently find the declaration and bark about it being explicitly 
> > > deleted. On the other hand, the fixit suggests code that should never 
> > > pass a reasonable code review.
> > > 
> > > I'm wondering if it would make more sense to leave the access specifiers 
> > > alone and just put a FIXME in the code to point the situation out. I am 
> > > guessing that at some point we will have a refactoring tool that can help 
> > > without resorting to making declarations like `public: C() = delete; 
> > > private:`.
> > > 
> > > What do you think?
> > I'm wondering whether no fixit would be better than a not-good-enough fixit.
> Generally, no. Incremental improvements are almost always fine. However, the 
> user is asking to have their code modernized, and the fixit results in code 
> that looks more foreign than modern (at least, to me).
> 
> I won't block the patch moving forward regardless of whether the fixit is in 
> or out, but I am curious if @alexfh has an opinion, or if you have a strong 
> preference one way or the other.
I'll change the check to warn about deleted methods that aren't public; the 
user can fix those manually until a better fixit is possible.


https://reviews.llvm.org/D26138



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


[PATCH] D26302: [OpenCL] Remove redundant test for OpenCL header file

2016-11-05 Thread Anastasia Stulova via cfe-commits
Anastasia added a comment.

LGTM! Thanks! Do you know the runtime of this test now?


https://reviews.llvm.org/D26302



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


[PATCH] D26310: Add a method to get the list of registered static analyzer checkers.

2016-11-05 Thread Anna Zaks via cfe-commits
zaks.anna added a comment.

Thanks for fixing this!




Comment at: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp:39
+if (IncludeExperimental ||
+(!CheckName.startswith("debug.") && !CheckName.startswith("alpha.")))
+  Result.push_back(CheckName);

Debug checkers are not user-facing; they are used to debug the analyzer. So we 
should either have another flag for them or always skip them.


https://reviews.llvm.org/D26310



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


[PATCH] D26311: Use AnalyzerOptions::getRegisteredCheckers() instead of clang/StaticAnalyzer/Checkers/Checkers.inc

2016-11-05 Thread Anna Zaks via cfe-commits
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.

LGTM!


https://reviews.llvm.org/D26311



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


Re: [PATCH] Warning for main returning a bool.

2016-11-05 Thread Manuel Klimek via cfe-commits
+richard

On Fri, Oct 14, 2016 at 10:18 AM Joshua Hurwitz via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> See attached.
>
> Returning a bool from main is a special case of return type mismatch. The
> common convention when returning a bool is that 'true' (== 1) indicates
> success and 'false' (== 0) failure. But since main expects a return value
> of 0 on success, returning a bool is usually unintended.
> ___
> 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


r286041 - clang-format: Better support for CUDA's triple brackets.

2016-11-05 Thread Daniel Jasper via cfe-commits
Author: djasper
Date: Sat Nov  5 12:43:16 2016
New Revision: 286041

URL: http://llvm.org/viewvc/llvm-project?rev=286041&view=rev
Log:
clang-format: Better support for CUDA's triple brackets.

Before:
  aaa<
  a, aa,
  aa><<>>();

After:
  aaa
  <<>>();

Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=286041&r1=286040&r2=286041&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Sat Nov  5 12:43:16 2016
@@ -2488,6 +2488,8 @@ bool TokenAnnotator::canBreakBefore(cons
 return true;
   if (Right.is(TT_RangeBasedForLoopColon))
 return false;
+  if (Left.is(TT_TemplateCloser) && Right.is(TT_TemplateOpener))
+return true;
   if (Left.isOneOf(TT_TemplateCloser, TT_UnaryOperator) ||
   Left.is(tok::kw_operator))
 return false;

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=286041&r1=286040&r2=286041&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Sat Nov  5 12:43:16 2016
@@ -11391,6 +11391,8 @@ TEST_F(FormatTest, TripleAngleBrackets)
   EXPECT_EQ("f<<<1, 1>>>();", format("f< param > <<< 1, 1 >>> ();"));
   
verifyFormat("aaa"
"aaa<<<\n1, 1>>>();");
+  verifyFormat("aaa\n"
+   "<<>>();");
 }
 
 TEST_F(FormatTest, MergeLessLessAtEnd) {


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


[PATCH] D26288: Deduplicate replacements by FileEntry instead of file names.

2016-11-05 Thread Manuel Klimek via cfe-commits
klimek added a comment.

In https://reviews.llvm.org/D26288#586932, @ioeric wrote:

> - Addressed comments: handle non-existing files.


We're not really handling them now though? We're just printing an error?

My point is that we might run the replacement generation on a distributed 
system, and then group/deduplicate/apply them somewhere where the files might 
not actually exist (think the reduce stage of a mapreduce). If possible, I'd 
like to not rely on the existence of the file when we deal with Replacements. 
I'd find it especially problematic if the existence or non-existence of files 
changes the semantics of those operations.


https://reviews.llvm.org/D26288



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


[PATCH] D26317: Fix use-of-temporary with StringRef in code coverage

2016-11-05 Thread Vedant Kumar via cfe-commits
vsk accepted this revision.
vsk added a comment.
This revision is now accepted and ready to land.

Thanks, lgtm. I've made a note to clean this up.


Repository:
  rL LLVM

https://reviews.llvm.org/D26317



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


[PATCH] D26288: Deduplicate replacements by FileEntry instead of file names.

2016-11-05 Thread Eric Liu via cfe-commits
ioeric added a comment.

In https://reviews.llvm.org/D26288#587513, @klimek wrote:

> In https://reviews.llvm.org/D26288#586932, @ioeric wrote:
>
> > - Addressed comments: handle non-existing files.
>
>
> We're not really handling them now though? We're just printing an error?
>
> My point is that we might run the replacement generation on a distributed 
> system, and then group/deduplicate/apply them somewhere where the files might 
> not actually exist (think the reduce stage of a mapreduce). If possible, I'd 
> like to not rely on the existence of the file when we deal with Replacements. 
> I'd find it especially problematic if the existence or non-existence of files 
> changes the semantics of those operations.


This function is only used in replacements application phase, and I think this 
is the most easiest if not the best way to ensure that two sets of replacements 
for the same file are applied without needing to handle dots and 
relative/absolute paths.

For distributed replacements generation, I think it makes more sense to 
duplications on the replacements application phase since this needs to be done 
when combining all replacements anyway.

It might make more sense to make this function local in 
tooling/Refactoring.cpp. But in that case, I can't unit test it :(


https://reviews.llvm.org/D26288



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


[PATCH] D26138: [clang-tidy] Add modernize-use-equals-delete check

2016-11-05 Thread Malcolm Parsons via cfe-commits
malcolm.parsons updated this revision to Diff 76971.
malcolm.parsons added a comment.

Check for non-public deleted methods.
Remove imperfect FixItHint.
Add FIXME comments.


https://reviews.llvm.org/D26138

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/UseEqualsDeleteCheck.cpp
  clang-tidy/modernize/UseEqualsDeleteCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-use-default.rst
  docs/clang-tidy/checks/modernize-use-equals-delete.rst
  test/clang-tidy/modernize-use-equals-delete.cpp

Index: test/clang-tidy/modernize-use-equals-delete.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-equals-delete.cpp
@@ -0,0 +1,134 @@
+// RUN: %check_clang_tidy %s modernize-use-equals-delete %t
+
+struct PositivePrivate {
+private:
+  PositivePrivate();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit calling of a special member function [modernize-use-equals-delete]
+  // CHECK-FIXES: PositivePrivate() = delete;
+  PositivePrivate(const PositivePrivate &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit calling of a special member function [modernize-use-equals-delete]
+  // CHECK-FIXES: PositivePrivate(const PositivePrivate &) = delete;
+  PositivePrivate &operator=(const PositivePrivate &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use '= delete' to prohibit calling of a special member function [modernize-use-equals-delete]
+  // CHECK-FIXES: PositivePrivate &operator=(const PositivePrivate &) = delete;
+  PositivePrivate(PositivePrivate &&);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit calling of a special member function [modernize-use-equals-delete]
+  // CHECK-FIXES: PositivePrivate(PositivePrivate &&) = delete;
+  PositivePrivate &operator=(PositivePrivate &&);
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use '= delete' to prohibit calling of a special member function [modernize-use-equals-delete]
+  // CHECK-FIXES: PositivePrivate &operator=(PositivePrivate &&) = delete;
+  ~PositivePrivate();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit calling of a special member function [modernize-use-equals-delete]
+  // CHECK-FIXES: ~PositivePrivate() = delete;
+};
+
+struct NegativePublic {
+  NegativePublic(const NegativePublic &);
+};
+
+struct NegativeProtected {
+protected:
+  NegativeProtected(const NegativeProtected &);
+};
+
+struct PositiveInlineMember {
+  int foo() { return 0; }
+
+private:
+  PositiveInlineMember(const PositiveInlineMember &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit calling of a special member function [modernize-use-equals-delete]
+  // CHECK-FIXES: PositiveInlineMember(const PositiveInlineMember &) = delete;
+};
+
+struct PositiveOutOfLineMember {
+  int foo();
+
+private:
+  PositiveOutOfLineMember(const PositiveOutOfLineMember &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit calling of a special member function [modernize-use-equals-delete]
+  // CHECK-FIXES: PositiveOutOfLineMember(const PositiveOutOfLineMember &) = delete;
+};
+
+int PositiveOutOfLineMember::foo() { return 0; }
+
+struct PositiveAbstractMember {
+  virtual int foo() = 0;
+
+private:
+  PositiveAbstractMember(const PositiveAbstractMember &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit calling of a special member function [modernize-use-equals-delete]
+  // CHECK-FIXES: PositiveAbstractMember(const PositiveAbstractMember &) = delete;
+};
+
+struct NegativeMemberNotImpl {
+  int foo();
+
+private:
+  NegativeMemberNotImpl(const NegativeMemberNotImpl &);
+};
+
+struct NegativeStaticMemberNotImpl {
+  static int foo();
+
+private:
+  NegativeStaticMemberNotImpl(const NegativeStaticMemberNotImpl &);
+};
+
+struct NegativeInline {
+private:
+  NegativeInline(const NegativeInline &) {}
+};
+
+struct NegativeOutOfLine {
+private:
+  NegativeOutOfLine(const NegativeOutOfLine &);
+};
+
+NegativeOutOfLine::NegativeOutOfLine(const NegativeOutOfLine &) {}
+
+struct NegativeConstructNotImpl {
+  NegativeConstructNotImpl();
+
+private:
+  NegativeConstructNotImpl(const NegativeConstructNotImpl &);
+};
+
+struct PositiveDefaultedConstruct {
+  PositiveDefaultedConstruct() = default;
+
+private:
+  PositiveDefaultedConstruct(const PositiveDefaultedConstruct &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit calling of a special member function [modernize-use-equals-delete]
+  // CHECK-FIXES: PositiveDefaultedConstruct(const PositiveDefaultedConstruct &) = delete;
+};
+
+struct PositiveDeletedConstruct {
+  PositiveDeletedConstruct() = delete;
+
+private:
+  PositiveDeletedConstruct(const PositiveDeletedConstruct &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit calling of a special member fun

[PATCH] D26288: Deduplicate replacements by FileEntry instead of file names.

2016-11-05 Thread Manuel Klimek via cfe-commits
klimek added a comment.

Ok, makes sense. Can you add a comment to the function that explains what 
happens with replacements for files that do not exist (we ignore them, unless 
I'm mistaken). Otherwise LG.


https://reviews.llvm.org/D26288



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


[PATCH] D26288: Deduplicate replacements by FileEntry instead of file names.

2016-11-05 Thread Eric Liu via cfe-commits
ioeric updated this revision to Diff 76975.
ioeric added a comment.

- addressed comments.


https://reviews.llvm.org/D26288

Files:
  include/clang/Tooling/Core/Replacement.h
  lib/Tooling/Core/Replacement.cpp
  lib/Tooling/Refactoring.cpp
  unittests/Tooling/RefactoringTest.cpp

Index: unittests/Tooling/RefactoringTest.cpp
===
--- unittests/Tooling/RefactoringTest.cpp
+++ unittests/Tooling/RefactoringTest.cpp
@@ -19,6 +19,7 @@
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/VirtualFileSystem.h"
 #include "clang/Format/Format.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
@@ -972,40 +973,64 @@
   toReplacements({{"", 0, 3, "cc"}, {"", 3, 3, "dd"}}));
 }
 
-TEST(DeduplicateByFileTest, LeaveLeadingDotDot) {
+TEST(DeduplicateByFileTest, PathsWithDots) {
   std::map FileToReplaces;
+  llvm::IntrusiveRefCntPtr VFS(
+  new vfs::InMemoryFileSystem());
+  FileManager *FileMgr = new FileManager(FileSystemOptions(), VFS);
 #if !defined(LLVM_ON_WIN32)
-  FileToReplaces["../../a/b/.././c.h"] = Replacements();
-  FileToReplaces["../../a/c.h"] = Replacements();
+  StringRef Path1 = "a/b/.././c.h";
+  StringRef Path2 = "a/c.h";
 #else
-  FileToReplaces["..\\..\\a\\b\\..\\.\\c.h"] = Replacements();
-  FileToReplaces["..\\..\\a\\c.h"] = Replacements();
+  StringRef Path1 = "a\\b\\..\\.\\c.h";
+  StringRef Path2 = "a\\c.h";
 #endif
-  FileToReplaces = groupReplacementsByFile(FileToReplaces);
+  EXPECT_TRUE(VFS->addFile(Path1, 0, llvm::MemoryBuffer::getMemBuffer("")));
+  EXPECT_TRUE(VFS->addFile(Path2, 0, llvm::MemoryBuffer::getMemBuffer("")));
+  FileToReplaces[Path1] = Replacements();
+  FileToReplaces[Path2] = Replacements();
+  FileToReplaces = groupReplacementsByFile(*FileMgr, FileToReplaces);
   EXPECT_EQ(1u, FileToReplaces.size());
-#if !defined(LLVM_ON_WIN32)
-  EXPECT_EQ("../../a/c.h", FileToReplaces.begin()->first);
-#else
-  EXPECT_EQ("..\\..\\a\\c.h", FileToReplaces.begin()->first);
-#endif
+  EXPECT_EQ(Path1, FileToReplaces.begin()->first);
 }
 
-TEST(DeduplicateByFileTest, RemoveDotSlash) {
+TEST(DeduplicateByFileTest, PathWithDotSlash) {
   std::map FileToReplaces;
+  llvm::IntrusiveRefCntPtr VFS(
+  new vfs::InMemoryFileSystem());
+  FileManager *FileMgr = new FileManager(FileSystemOptions(), VFS);
 #if !defined(LLVM_ON_WIN32)
-  FileToReplaces["./a/b/.././c.h"] = Replacements();
-  FileToReplaces["a/c.h"] = Replacements();
+  StringRef Path1 = "./a/b/c.h";
+  StringRef Path2 = "a/b/c.h";
 #else
-  FileToReplaces[".\\a\\b\\..\\.\\c.h"] = Replacements();
-  FileToReplaces["a\\c.h"] = Replacements();
+  StringRef Path1 = ".\\a\\b\\c.h";
+  StringRef Path2 = "a\\b\\c.h";
 #endif
-  FileToReplaces = groupReplacementsByFile(FileToReplaces);
+  EXPECT_TRUE(VFS->addFile(Path1, 0, llvm::MemoryBuffer::getMemBuffer("")));
+  EXPECT_TRUE(VFS->addFile(Path2, 0, llvm::MemoryBuffer::getMemBuffer("")));
+  FileToReplaces[Path1] = Replacements();
+  FileToReplaces[Path2] = Replacements();
+  FileToReplaces = groupReplacementsByFile(*FileMgr, FileToReplaces);
   EXPECT_EQ(1u, FileToReplaces.size());
+  EXPECT_EQ(Path1, FileToReplaces.begin()->first);
+}
+
+TEST(DeduplicateByFileTest, NonExistingFilePath) {
+  std::map FileToReplaces;
+  llvm::IntrusiveRefCntPtr VFS(
+  new vfs::InMemoryFileSystem());
+  FileManager *FileMgr = new FileManager(FileSystemOptions(), VFS);
 #if !defined(LLVM_ON_WIN32)
-  EXPECT_EQ("a/c.h", FileToReplaces.begin()->first);
+  StringRef Path1 = "./a/b/c.h";
+  StringRef Path2 = "a/b/c.h";
 #else
-  EXPECT_EQ("a\\c.h", FileToReplaces.begin()->first);
+  StringRef Path1 = ".\\a\\b\\c.h";
+  StringRef Path2 = "a\\b\\c.h";
 #endif
+  FileToReplaces[Path1] = Replacements();
+  FileToReplaces[Path2] = Replacements();
+  FileToReplaces = groupReplacementsByFile(*FileMgr, FileToReplaces);
+  EXPECT_TRUE(FileToReplaces.empty());
 }
 
 } // end namespace tooling
Index: lib/Tooling/Refactoring.cpp
===
--- lib/Tooling/Refactoring.cpp
+++ lib/Tooling/Refactoring.cpp
@@ -57,7 +57,8 @@
 
 bool RefactoringTool::applyAllReplacements(Rewriter &Rewrite) {
   bool Result = true;
-  for (const auto &Entry : groupReplacementsByFile(FileToReplaces))
+  for (const auto &Entry : groupReplacementsByFile(
+   Rewrite.getSourceMgr().getFileManager(), FileToReplaces))
 Result = tooling::applyAllReplacements(Entry.second, Rewrite) && Result;
   return Result;
 }
@@ -73,7 +74,8 @@
   FileManager &Files = SM.getFileManager();
 
   bool Result = true;
-  for (const auto &FileAndReplaces : groupReplacementsByFile(FileToReplaces)) {
+  for (const auto &FileAndReplaces : groupReplacementsByFile(
+   Rewrite.getSourceMgr().getFileManager(), FileToReplaces)) {
 const std::string &FilePath = FileAndReplaces.first;
 auto &CurReplaces 

[PATCH] D6833: Clang-format: Braces Indent Style Whitesmith

2016-11-05 Thread Daniel Jasper via cfe-commits
djasper added a comment.

In case you still want to move forward with this, some more comments.

Why have you opted for doing this in getNewLineColumn instead of the 
UnwrappedLineFormatter::formatFirstToken() function I suggested? I think the 
latter would be way easier, but there might be cases I am not thinking of right 
now.




Comment at: docs/ClangFormatStyleOptions.rst:260
+  * ``BS_Whitesmiths`` (in configuration: ``Whitesmiths``)
+Like ``Allman``, with the braces intended too.
   * ``BS_GNU`` (in configuration: ``GNU``)

indented?



Comment at: lib/Format/ContinuationIndenter.h:270
+  bool BlockNoExtraIndent;
+
   bool operator<(const ParenState &Other) const {

No. I don't think it is necessary to add an extra value to the indent state for 
this and doing so would be costly in runtime.


https://reviews.llvm.org/D6833



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


[PATCH] D26139: Tests for strings conversions under libcpp-no-exceptions

2016-11-05 Thread Marshall Clow via cfe-commits
mclow.lists added a comment.

>   I think it might be better to add TEST_TRY and TEST_CATCH(...) macros 
> defined like

@rogfer01 said at the top that he didn't want to add "a magical TEST_TRY macro" 
- and I agree.  Someone tried that in another review, and I nixed it there.


https://reviews.llvm.org/D26139



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