date:20160721

Re: [PATCH] D22662: [cxx1z-constexpr-lambda] Make a lambda's closure type a literal type in C++1z.

2016-07-21 Thread Richard Smith via cfe-commits

rsmith added a comment.

Everything other than the diagnostics change LGTM; can we handle the 
diagnostics change as a central patch and put this fallback logic directly into 
the diagnostics formatting code, so it applies everywhere we try to print the 
name of an unnamed class?



Comment at: lib/Sema/SemaType.cpp:7168-7176
@@ -7167,3 +7167,11 @@
  !RD->hasTrivialDefaultConstructor()) {
-Diag(RD->getLocation(), diag::note_non_literal_no_constexpr_ctors) << RD;
+// If the class does not have a name (for e.g. a lambda's closure class) 
use
+// its type which we should know how to pretty-print, otherwise use the
+// class's name.
+auto &&DiagBuilder =
+Diag(RD->getLocation(), diag::note_non_literal_no_constexpr_ctors);
+if (!RD->getIdentifier())
+  DiagBuilder << Context.getRecordType(RD);
+else
+  DiagBuilder << RD;
   } else if (RD->hasNonLiteralTypeFieldsOrBases()) {

This seems like something that would be better handled centrally by the 
diagnostics machinery.


https://reviews.llvm.org/D22662



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

Re: [PATCH] D21814: clang-rename: split existing options into two new subcommands

2016-07-21 Thread Manuel Klimek via cfe-commits

klimek added inline comments.


Comment at: clang-rename/RenamingAction.cpp:48
@@ +47,3 @@
+for (unsigned I = 0; I < NewNameList.size(); ++I) {
+  HandleOneRename(Context, NewNameList[I], PrevNameList[I], USRList[I]);
+}

Question is whether if we go down that route we shouldn't do one search for all 
the names, instead of re-looping for each name.


https://reviews.llvm.org/D21814



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

Re: [PATCH] D21459: Implement http://wg21.link/P0254R1: "Integrating std::string_view and std::string"

2016-07-21 Thread Marshall Clow via cfe-commits

mclow.lists added inline comments.


Comment at: include/string_view:216
@@ +215,3 @@
+   basic_string_view(const _CharT* __s)
+   : __data(__s), __size(_Traits::length(__s)) {}
+

kimgr wrote:
> mclow.lists wrote:
> > mclow.lists wrote:
> > > kimgr wrote:
> > > > mclow.lists wrote:
> > > > > kimgr wrote:
> > > > > > I'm working from the paper at 
> > > > > > https://isocpp.org/files/papers/N3762.html, and I find it a little 
> > > > > > sketchy on the policy for nullptrs.
> > > > > > 
> > > > > > Since the ctor above accepts nullptr as long as the length is zero, 
> > > > > > would it make sense to do that here too? That is, only call 
> > > > > > _Traits::length for non-nullptr __s args?
> > > > > Reading from N4600: Requires: `[str, str + traits::length(str))` is a 
> > > > > valid range.
> > > > > 
> > > > > So, no - passing `nullptr` here is undefined.
> > > > > 
> > > > OK, that feels more principled to me, anyway.
> > > > 
> > > > But the `(const char*, size_t)` constructor has the same requirement 
> > > > and it *does* accept `nullptr`, provided the length is zero. I saw you 
> > > > had to get rid of the assertion, but I feel like the constructor should 
> > > > probably not silently accept `nullptr` for zero-sized strings. Or do 
> > > > you know if that's intentional? Many StringRef/StringPiece 
> > > > implementations seem to have the same behavior.
> > > It is absolutely intentional. `[nullptr, nullptr+0)` is a perfectly fine 
> > > half-open range. It contains no characters.  
> > > 
> > > However, the ctor that takes just a pointer has to calculate the length 
> > > .. by dereferencing the pointer.
> > > 
> > > I had to get rid of the assertion because one of the bots (gcc 4.9) has a 
> > > bug about constexpr ctors in c++11 mode.  Even though the assertion was 
> > > `#ifdef`ed on C++ > 11, the compiler complained about it.  I'll be 
> > > putting the assertion back as soon as I can figure out how to keep gcc 
> > > from complaining.
> > This was discussed (at length) in LEWG during the design of `string_view`.
> Ah, got it, thanks! It opens up for cases where `data()` for an empty 
> `string_view` can sometimes return `""` and sometimes `nullptr`, but I guess 
> that problem extends beyond `string_view`'s responsibilities.
> 
> Thanks for the thorough explanation.
I think you're laboring under a misapprehension here.

An empty `string_view` points to *no characters*, not an empty null-terminated 
string.

Treating the pointer that you get back from `string_view::data` as a 
null-terminated string will lead to all sorts of problems.  This is explicitly 
called out in [string.view.access]/19:

> Note: Unlike basic_string::data() and string literals, data() may return a 
> pointer to a buffer that is not null-terminated. Therefore it is typically a 
> mistake to pass data() to a routine that takes just a const charT* and 
> expects a null-terminated string.




https://reviews.llvm.org/D21459



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

Re: [PATCH] D22662: [cxx1z-constexpr-lambda] Make a lambda's closure type a literal type in C++1z.

2016-07-21 Thread Faisal Vali via cfe-commits

faisalv updated this revision to Diff 65026.
faisalv added a comment.

Factor out the diagnostic builder RAII creation (through Sema.Diag) and reduce 
repetition.


https://reviews.llvm.org/D22662

Files:
  include/clang/AST/DeclCXX.h
  lib/Sema/SemaType.cpp
  test/SemaCXX/cxx1z-constexpr-lambdas.cpp

Index: test/SemaCXX/cxx1z-constexpr-lambdas.cpp
===
--- test/SemaCXX/cxx1z-constexpr-lambdas.cpp
+++ test/SemaCXX/cxx1z-constexpr-lambdas.cpp
@@ -1,8 +1,8 @@
 // RUN: %clang_cc1 -std=c++1z -verify -fsyntax-only -fblocks %s
 // RUN: %clang_cc1 -std=c++1z -verify -fsyntax-only -fblocks 
-fdelayed-template-parsing %s 
-// RUN: %clang_cc1 -std=c++1z -verify -fsyntax-only -fblocks -fms-extensions 
%s 
-// RUN: %clang_cc1 -std=c++1z -verify -fsyntax-only -fblocks 
-fdelayed-template-parsing -fms-extensions %s 
+// RUN: %clang_cc1 -std=c++14 -verify -fsyntax-only -fblocks %s 
-DCPP14_AND_EARLIER
 
+#ifndef CPP14_AND_EARLIER
 namespace test_constexpr_checking {
 
 namespace ns1 {
@@ -33,4 +33,16 @@
   L(3); //expected-note{{non-constexpr function}}
 } 
 
-} // end ns test_constexpr_call
\ No newline at end of file
+} // end ns test_constexpr_call
+
+#endif
+
+namespace test_lambda_is_literal {
+#ifdef CPP14_AND_EARLIER
+//expected-error@+4{{not a literal type}}
+//expected-note@+2{{not an aggregate and has no constexpr constructors}}
+#endif
+auto L = [] { };
+constexpr int foo(decltype(L) l) { return 0; }
+
+}
\ No newline at end of file
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -7165,7 +7165,15 @@
   << I.getSourceRange();
   } else if (!RD->isAggregate() && !RD->hasConstexprNonCopyMoveConstructor() &&
  !RD->hasTrivialDefaultConstructor()) {
-Diag(RD->getLocation(), diag::note_non_literal_no_constexpr_ctors) << RD;
+// If the class does not have a name (for e.g. a lambda's closure class) 
use
+// its type which we should know how to pretty-print, otherwise use the
+// class's name.
+auto &&DiagBuilder =
+Diag(RD->getLocation(), diag::note_non_literal_no_constexpr_ctors);
+if (!RD->getIdentifier())
+  DiagBuilder << Context.getRecordType(RD);
+else
+  DiagBuilder << RD;
   } else if (RD->hasNonLiteralTypeFieldsOrBases()) {
 for (const auto &I : RD->bases()) {
   if (!I.getType()->isLiteralType(Context)) {
Index: include/clang/AST/DeclCXX.h
===
--- include/clang/AST/DeclCXX.h
+++ include/clang/AST/DeclCXX.h
@@ -536,11 +536,10 @@
 MethodTyInfo(Info) {
   IsLambda = true;
 
-  // C++11 [expr.prim.lambda]p3:
-  //   This class type is neither an aggregate nor a literal type.
+  // C++1z [expr.prim.lambda]p4:
+  //   This class type is not an aggregate type.
   Aggregate = false;
   PlainOldData = false;
-  HasNonLiteralTypeFieldsOrBases = true;
 }
 
 /// \brief Whether this lambda is known to be dependent, even if its
@@ -1339,11 +1338,15 @@
   ///
   /// We resolve DR1361 by ignoring the second bullet. We resolve DR1452 by
   /// treating types with trivial default constructors as literal types.
+  ///
+  /// Only in C++1z and beyond, are lambdas literal types.
   bool isLiteral() const {
 return hasTrivialDestructor() &&
-   (isAggregate() || hasConstexprNonCopyMoveConstructor() ||
-hasTrivialDefaultConstructor()) &&
-   !hasNonLiteralTypeFieldsOrBases();
+   (!isLambda() || getASTContext().getLangOpts().CPlusPlus1z) &&
+   !hasNonLiteralTypeFieldsOrBases() &&
+   (isAggregate() || isLambda() ||
+hasConstexprNonCopyMoveConstructor() ||
+hasTrivialDefaultConstructor());
   }
 
   /// \brief If this record is an instantiation of a member class,


Index: test/SemaCXX/cxx1z-constexpr-lambdas.cpp
===
--- test/SemaCXX/cxx1z-constexpr-lambdas.cpp
+++ test/SemaCXX/cxx1z-constexpr-lambdas.cpp
@@ -1,8 +1,8 @@
 // RUN: %clang_cc1 -std=c++1z -verify -fsyntax-only -fblocks %s
 // RUN: %clang_cc1 -std=c++1z -verify -fsyntax-only -fblocks -fdelayed-template-parsing %s 
-// RUN: %clang_cc1 -std=c++1z -verify -fsyntax-only -fblocks -fms-extensions %s 
-// RUN: %clang_cc1 -std=c++1z -verify -fsyntax-only -fblocks -fdelayed-template-parsing -fms-extensions %s 
+// RUN: %clang_cc1 -std=c++14 -verify -fsyntax-only -fblocks %s -DCPP14_AND_EARLIER
 
+#ifndef CPP14_AND_EARLIER
 namespace test_constexpr_checking {
 
 namespace ns1 {
@@ -33,4 +33,16 @@
   L(3); //expected-note{{non-constexpr function}}
 } 
 
-} // end ns test_constexpr_call
\ No newline at end of file
+} // end ns test_constexpr_call
+
+#endif
+
+namespace test_lambda_is_literal {
+#ifdef CPP14_AND_EARLIER
+//expected-error@+4{{not a literal type}}
+//expected-note@+2{{not an

Re: [PATCH] D21992: [clang-tidy] new cppcoreguidelines-slicing

2016-07-21 Thread Piotr Padlewski via cfe-commits

Prazek added a comment.

Is it in upstream yet?



Comment at: clang-tidy/cppcoreguidelines/SlicingCheck.h:19
@@ +18,3 @@
+
+/// Flags slicing of member variables or vtable. See:
+///   -

courbet wrote:
> Prazek wrote:
> > some short description what does this check do?
> There is already a more detialed explanation provided in 
> extra/clang-tidy/checks/cppcoreguidelines-slicing.html, and I feel that the 
> links explain it better than anything short we could put in the comments 
> there. Or should I copy-paste what's in 
> extra/clang-tidy/checks/cppcoreguidelines-slicing.html ?
Well, it would be good to at least say what is slicing :)


https://reviews.llvm.org/D21992



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

Re: [PATCH] D22208: [clang-tidy] Fixes to modernize-use-emplace

2016-07-21 Thread Piotr Padlewski via cfe-commits

Prazek marked 6 inline comments as done.
Prazek added a comment.

Repository:
  rL LLVM

https://reviews.llvm.org/D22208



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

[PATCH] D22663: Support setting default value for -rtlib at build time

2016-07-21 Thread Lei Zhang via cfe-commits

zlei created this revision.
zlei added reviewers: cfe-commits, ddunbar, Hahnfeld.

This patch introduces a new cmake variable: CLANG_DEFAULT_RTLIB, thru
which we can specify a default value for -rtlib (libgcc or
compiler-rt) at build time, just like how we set the default C++
stdlib thru CLANG_DEFAULT_CXX_STDLIB.

With these two options, we can configure clang to build binaries on
Linux that have no runtime dependence on any gcc libs (libstdc++ or
libgcc_s).

https://reviews.llvm.org/D22663

Files:
  CMakeLists.txt
  include/clang/Config/config.h.cmake
  lib/Driver/ToolChain.cpp

Index: lib/Driver/ToolChain.cpp
===
--- lib/Driver/ToolChain.cpp
+++ lib/Driver/ToolChain.cpp
@@ -526,16 +526,16 @@
 
 ToolChain::RuntimeLibType ToolChain::GetRuntimeLibType(
 const ArgList &Args) const {
-  if (Arg *A = Args.getLastArg(options::OPT_rtlib_EQ)) {
-StringRef Value = A->getValue();
-if (Value == "compiler-rt")
-  return ToolChain::RLT_CompilerRT;
-if (Value == "libgcc")
-  return ToolChain::RLT_Libgcc;
-getDriver().Diag(diag::err_drv_invalid_rtlib_name)
-  << A->getAsString(Args);
-  }
+  const Arg* A = Args.getLastArg(options::OPT_rtlib_EQ);
+  StringRef LibName = A ? A->getValue() : CLANG_DEFAULT_RTLIB;
 
+  if (LibName == "compiler-rt")
+return ToolChain::RLT_CompilerRT;
+  if (LibName == "libgcc")
+return ToolChain::RLT_Libgcc;
+  if (A)
+getDriver().Diag(diag::err_drv_invalid_rtlib_name) << A->getAsString(Args);
+
   return GetDefaultRuntimeLibType();
 }
 
Index: include/clang/Config/config.h.cmake
===
--- include/clang/Config/config.h.cmake
+++ include/clang/Config/config.h.cmake
@@ -11,6 +11,9 @@
 /* Default C++ stdlib to use. */
 #define CLANG_DEFAULT_CXX_STDLIB "${CLANG_DEFAULT_CXX_STDLIB}"
 
+/* Default runtime library to use. */
+#define CLANG_DEFAULT_RTLIB "${CLANG_DEFAULT_RTLIB}"
+
 /* Default OpenMP runtime used by -fopenmp. */
 #define CLANG_DEFAULT_OPENMP_RUNTIME "${CLANG_DEFAULT_OPENMP_RUNTIME}"
 
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -201,6 +201,15 @@
   set(CLANG_DEFAULT_CXX_STDLIB "")
 endif()
 
+set(CLANG_DEFAULT_RTLIB "" CACHE STRING
+  "Default runtime library to use (libgcc or compiler-rt)")
+if (NOT(CLANG_DEFAULT_RTLIB STREQUAL "" OR
+CLANG_DEFAULT_RTLIB STREQUAL "libgcc" OR
+CLANG_DEFAULT_RTLIB STREQUAL "compiler-rt"))
+  message(WARNING "Resetting default rtlib to use platform default")
+  set(CLANG_DEFAULT_RTLIB "")
+endif()
+
 set(CLANG_DEFAULT_OPENMP_RUNTIME "libomp" CACHE STRING
   "Default OpenMP runtime used by -fopenmp.")
 


Index: lib/Driver/ToolChain.cpp
===
--- lib/Driver/ToolChain.cpp
+++ lib/Driver/ToolChain.cpp
@@ -526,16 +526,16 @@
 
 ToolChain::RuntimeLibType ToolChain::GetRuntimeLibType(
 const ArgList &Args) const {
-  if (Arg *A = Args.getLastArg(options::OPT_rtlib_EQ)) {
-StringRef Value = A->getValue();
-if (Value == "compiler-rt")
-  return ToolChain::RLT_CompilerRT;
-if (Value == "libgcc")
-  return ToolChain::RLT_Libgcc;
-getDriver().Diag(diag::err_drv_invalid_rtlib_name)
-  << A->getAsString(Args);
-  }
+  const Arg* A = Args.getLastArg(options::OPT_rtlib_EQ);
+  StringRef LibName = A ? A->getValue() : CLANG_DEFAULT_RTLIB;
 
+  if (LibName == "compiler-rt")
+return ToolChain::RLT_CompilerRT;
+  if (LibName == "libgcc")
+return ToolChain::RLT_Libgcc;
+  if (A)
+getDriver().Diag(diag::err_drv_invalid_rtlib_name) << A->getAsString(Args);
+
   return GetDefaultRuntimeLibType();
 }
 
Index: include/clang/Config/config.h.cmake
===
--- include/clang/Config/config.h.cmake
+++ include/clang/Config/config.h.cmake
@@ -11,6 +11,9 @@
 /* Default C++ stdlib to use. */
 #define CLANG_DEFAULT_CXX_STDLIB "${CLANG_DEFAULT_CXX_STDLIB}"
 
+/* Default runtime library to use. */
+#define CLANG_DEFAULT_RTLIB "${CLANG_DEFAULT_RTLIB}"
+
 /* Default OpenMP runtime used by -fopenmp. */
 #define CLANG_DEFAULT_OPENMP_RUNTIME "${CLANG_DEFAULT_OPENMP_RUNTIME}"
 
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -201,6 +201,15 @@
   set(CLANG_DEFAULT_CXX_STDLIB "")
 endif()
 
+set(CLANG_DEFAULT_RTLIB "" CACHE STRING
+  "Default runtime library to use (libgcc or compiler-rt)")
+if (NOT(CLANG_DEFAULT_RTLIB STREQUAL "" OR
+CLANG_DEFAULT_RTLIB STREQUAL "libgcc" OR
+CLANG_DEFAULT_RTLIB STREQUAL "compiler-rt"))
+  message(WARNING "Resetting default rtlib to use platform default")
+  set(CLANG_DEFAULT_RTLIB "")
+endif()
+
 set(CLANG_DEFAULT_OPENMP_RUNTIME "libomp" CACHE STRING
   "Default OpenMP runtime used by -fopenmp.")

Re: [PATCH] D22208: [clang-tidy] Fixes to modernize-use-emplace

2016-07-21 Thread Piotr Padlewski via cfe-commits

Prazek removed rL LLVM as the repository for this revision.
Prazek updated this revision to Diff 65023.
Prazek marked 4 inline comments as done.

https://reviews.llvm.org/D22208

Files:
  clang-tidy/modernize/UseEmplaceCheck.cpp
  clang-tidy/modernize/UseEmplaceCheck.h
  clang-tidy/utils/Matchers.h
  docs/clang-tidy/checks/modernize-use-emplace.rst
  test/clang-tidy/modernize-use-emplace.cpp

Index: test/clang-tidy/modernize-use-emplace.cpp
===
--- test/clang-tidy/modernize-use-emplace.cpp
+++ test/clang-tidy/modernize-use-emplace.cpp
@@ -1,4 +1,7 @@
-// RUN: %check_clang_tidy %s modernize-use-emplace %t
+// RUN: %check_clang_tidy %s modernize-use-emplace %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: modernize-use-emplace.ContainersWithPushBack, \
+// RUN:   value: '::std::vector; ::std::list; ::std::deque; llvm::LikeASmallVector'}]}" -- -std=c++11
 
 namespace std {
 template 
@@ -9,6 +12,7 @@
 
   template 
   void emplace_back(Args &&... args){};
+  ~vector();
 };
 template 
 class list {
@@ -18,6 +22,7 @@
 
   template 
   void emplace_back(Args &&... args){};
+  ~list();
 };
 
 template 
@@ -28,6 +33,7 @@
 
   template 
   void emplace_back(Args &&... args){};
+  ~deque();
 };
 
 template 
@@ -54,10 +60,24 @@
 template 
 class unique_ptr {
 public:
-  unique_ptr(T *) {}
+  explicit unique_ptr(T *) {}
+  ~unique_ptr();
 };
 } // namespace std
 
+namespace llvm {
+template 
+class LikeASmallVector {
+public:
+  void push_back(const T &) {}
+  void push_back(T &&) {}
+
+  template 
+  void emplace_back(Args &&... args){};
+};
+
+} // llvm
+
 void testInts() {
   std::vector v;
   v.push_back(42);
@@ -72,6 +92,7 @@
   Something(int a, int b = 41) {}
   Something() {}
   void push_back(Something);
+  int getInt() { return 42; }
 };
 
 struct Convertable {
@@ -103,6 +124,16 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
   // CHECK-FIXES: v.emplace_back();
 
+  Something Different;
+  v.push_back(Something(Different.getInt(), 42));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(Different.getInt(), 42);
+
+  v.push_back(Different.getInt());
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(Different.getInt());
+
+
   v.push_back(42);
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
   // CHECK-FIXES: v.emplace_back(42);
@@ -117,6 +148,23 @@
   v.push_back(s);
 }
 
+template 
+void dependOnElem() {
+  std::vector v;
+  v.push_back(ElemType(42));
+}
+
+template 
+void dependOnContainer() {
+  ContainerType v;
+  v.push_back(Something(42));
+}
+
+void callDependent() {
+  dependOnElem();
+  dependOnContainer>();
+}
+
 void test2() {
   std::vector v;
   v.push_back(Zoz(Something(21, 37)));
@@ -130,12 +178,20 @@
   v.push_back(getZoz(Something(1, 2)));
 }
 
+struct GetPair {
+  std::pair getPair();
+};
 void testPair() {
   std::vector> v;
   v.push_back(std::pair(1, 2));
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
   // CHECK-FIXES: v.emplace_back(1, 2);
 
+  GetPair g;
+  v.push_back(g.getPair());
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(g.getPair());
+
   std::vector> v2;
   v2.push_back(std::pair(Something(42, 42), Zoz(Something(21, 37;
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back
@@ -206,14 +262,14 @@
   v.push_back(new int(5));
 
   std::vector> v2;
-  v2.push_back(new int(42));
+  v2.push_back(std::unique_ptr(new int(42)));
   // This call can't be replaced with emplace_back.
   // If emplacement will fail (not enough memory to add to vector)
   // we will have leak of int because unique_ptr won't be constructed
   // (and destructed) as in push_back case.
 
   auto *ptr = new int;
-  v2.push_back(ptr);
+  v2.push_back(std::unique_ptr(ptr));
   // Same here
 }
 
@@ -240,6 +296,11 @@
   d.push_back(Something(42));
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
   // CHECK-FIXES: d.emplace_back(42);
+
+  llvm::LikeASmallVector ls;
+  ls.push_back(Something(42));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back
+  // CHECK-FIXES: ls.emplace_back(42);
 }
 
 class IntWrapper {
@@ -336,3 +397,29 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
   // CHECK-FIXES: v.emplace_back(42, var);
 }
+
+class PrivateCtor {
+  PrivateCtor(int z);
+
+public:
+  void doStuff() {
+std::vector v;
+// This should not change it because emplace back doesn't have permission.
+// Check currently doesn't support friend delcarations because pretty much
+// nobody would want to be friend with std::vector :(.
+v.push_back(PrivateCtor(42));
+  }
+};
+
+struct WithDtor {
+  WithDtor(int) {}
+  ~WithDtor();
+};
+
+void testWithDtor() {
+  std::vector v;
+
+  v.push_back(WithDtor(42));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use empl

Re: [PATCH] D22208: [clang-tidy] Fixes to modernize-use-emplace

2016-07-21 Thread Piotr Padlewski via cfe-commits

Prazek added a subscriber: klimek.
Prazek added a comment.

Can you look at my previous comment? I nedd an expert for AST.


Repository:
  rL LLVM

https://reviews.llvm.org/D22208



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

Re: [PATCH] D22208: [clang-tidy] Fixes to modernize-use-emplace

2016-07-21 Thread Piotr Padlewski via cfe-commits

Prazek added a comment.

There is one bug left:

In the ClangIncludeFixer.cpp:169 there is push back that looks like this

Symbols.push_back(find_all_symbols::SymbolInfo(

  Split.first.trim(),
  find_all_symbols::SymbolInfo::SymbolKind::Unknown,
  CommaSplits[I].trim(), 1, {}, /*NumOccurrences=*/E - I));

There is initializer list inside that should not allow this to be matched 
(because I check for initializer list as argument)

Unfortunatelly clang AST doesn't mention any initializer list.

http://wklejto.pl/258691

As you can see the "{}" looks like this in AST

  |-MaterializeTemporaryExpr 0x7f759d803900  'const 
std::vector':'const class std::vector 
>, class std::allocator 
> > >' lvalue
  |   |   |   | `-CXXBindTemporaryExpr 0x7f759d8038e0  'const std::vector':'const class std::vector >, class std::allocator 
> > >' (CXXTemporary 0x7f759d8038d8)
  |   |   |   |   `-CXXConstructExpr 0x7f759d8038a8  'const std::vector':'const class std::vector >, class std::allocator 
> > >' 'void (void)'
  |   |   |   

Good news are that this is the only 1 case from all push_backs that it found on 
LLVM+Clang+extra. This patch fixed the ExprWithCleanups by using 
ignoreImplicit, so I clang-tidy have many new push_backs.


Repository:
  rL LLVM

https://reviews.llvm.org/D22208



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

Re: [PATCH] D22208: [clang-tidy] Fixes to modernize-use-emplace

2016-07-21 Thread Piotr Padlewski via cfe-commits

Prazek added inline comments.


Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:112
@@ -95,1 +111,3 @@
+  auto CtorCallSourceRange = CharSourceRange::getTokenRange(
+  InnerCtorCall->getExprLoc(), CallParensRange.getBegin());
 

alexfh wrote:
> > There is a bug here that I forgot about. Should be 
> > InnerCtorCall->getStartLoc()
> 
> Is this still relevant?
The thing is, that I have no idea. So I was expecting this to happen when I 
will run on llvm code base, but it seems that it works I don't know why.

So firstly, I marked wrong line, because I think the problem might be in line 
94 (Call->getArg(0)->getExprLoc() should be ->getLocStart())

So what I was expecting that if The Call->getArg(0) will be a memberExpr (call 
to member function) then getExprLoc will point to dot instead of the beggining 
of argument, so
we would remove a object name like

v.push_back(obj.member());
into
v.emplace_back(member());

But I have no idea why I can't find it, and right now I don't know if it is a 
real bug or not, but it should be if I got a bug here
https://reviews.llvm.org/D21642

Do you have any thoughts/test case that would break it?


Repository:
  rL LLVM

https://reviews.llvm.org/D22208



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

Re: [PATCH] D22465: [clang-rename] introduce better symbol finding and add few more tests

2016-07-21 Thread Alexander Kornienko via cfe-commits

alexfh requested changes to this revision.
This revision now requires changes to proceed.


Comment at: clang-rename/USRFinder.cpp:81
@@ +80,3 @@
+const auto TypeEndLoc = Loc.getEndLoc(),
+   TypeBeginLoc = Lexer::GetBeginningOfToken(
+   TypeEndLoc, SourceMgr, Context->getLangOpts());

Declarations of multiple variables can be confusing. Please split this 
declaration.


Comment at: clang-rename/USRFinder.cpp:94
@@ -91,4 +93,3 @@
   const auto *Decl = NameLoc.getNestedNameSpecifier()->getAsNamespace();
-  if (Decl && !setResult(Decl, NameLoc.getLocalBeginLoc(),
- Decl->getNameAsString().length()))
-return false;
+  if (Decl) {
+setResult(Decl, NameLoc.getLocalBeginLoc(), NameLoc.getLocalEndLoc());

nit: No need for braces around single-line `if` bodies.


Comment at: clang-rename/USRFinder.h:49
@@ +48,3 @@
+// FIXME: This wouldn't be needed if
+// RecursiveASTVisitor::VisitNestedNameSpecifier would have been 
implemented.
+class NestedNameSpecifierLocFinder : public MatchFinder::MatchCallback {

We should implement it then ;)


Comment at: test/clang-rename/ComplicatedClassType.cpp:1
@@ -1,2 +1,2 @@
-// Unsupported test.
 // RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=220 -new-name=Bar %t.cpp -i --

I guess, this test will need `// REQUIRES: shell`. You can try to commit 
without it, but watch windows buildbots closely.


https://reviews.llvm.org/D22465



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

Re: [PATCH] D22208: [clang-tidy] Fixes to modernize-use-emplace

2016-07-21 Thread Alexander Kornienko via cfe-commits

alexfh added inline comments.


Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:45
@@ -33,3 +44,3 @@
   hasDeclaration(functionDecl(hasName("push_back"))),
-  on(hasType(cxxRecordDecl(hasAnyName("std::vector", "llvm::SmallVector",
-  "std::list", "std::deque");
+  on(hasType(cxxRecordDecl(hasAnyName(SmallVector(
+  ContainersWithPushBack.begin(), ContainersWithPushBack.end()));

Prazek wrote:
> alexfh wrote:
> > What's the reason for explicitly creating a `SmallVector`? 
> > `VariadicFunction::operator()` takes an `ArrayRef`, which `std::vector<>` 
> > should be implicitly convertible to. Am I missing something?
> It is not. I guess it is because it is ArrayRef and I have 
> std::vector
Fair enough.


Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:112
@@ -95,1 +111,3 @@
+  auto CtorCallSourceRange = CharSourceRange::getTokenRange(
+  InnerCtorCall->getExprLoc(), CallParensRange.getBegin());
 

> There is a bug here that I forgot about. Should be 
> InnerCtorCall->getStartLoc()

Is this still relevant?


Comment at: docs/clang-tidy/checks/modernize-use-emplace.rst:12
@@ +11,3 @@
+uses it. It also doesn't support ``insert`` functions for associative 
containers
+because chaning ``insert`` to ``emplace`` may result in
+`speed regression 
`_.

Prazek wrote:
> alexfh wrote:
> > alexfh wrote:
> > > s/chaning/changing/
> > How about the `insert` method of `std::vector<>`?
> insert -> emplace should be fine for the vector, but this patch is trying to 
> mainly fix the bugs (and I want it to land in clang-3.9).
> 
> I will add ``insert`` near the ``push_front``
SG


Repository:
  rL LLVM

https://reviews.llvm.org/D22208



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

Re: [PATCH] D22208: [clang-tidy] Fixes to modernize-use-emplace

2016-07-21 Thread Piotr Padlewski via cfe-commits

Prazek added inline comments.


Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:45
@@ -33,3 +44,3 @@
   hasDeclaration(functionDecl(hasName("push_back"))),
-  on(hasType(cxxRecordDecl(hasAnyName("std::vector", "llvm::SmallVector",
-  "std::list", "std::deque");
+  on(hasType(cxxRecordDecl(hasAnyName(SmallVector(
+  ContainersWithPushBack.begin(), ContainersWithPushBack.end()));

alexfh wrote:
> What's the reason for explicitly creating a `SmallVector`? 
> `VariadicFunction::operator()` takes an `ArrayRef`, which `std::vector<>` 
> should be implicitly convertible to. Am I missing something?
It is not. I guess it is because it is ArrayRef and I have 
std::vector


Comment at: docs/clang-tidy/checks/modernize-use-emplace.rst:12
@@ +11,3 @@
+uses it. It also doesn't support ``insert`` functions for associative 
containers
+because chaning ``insert`` to ``emplace`` may result in
+`speed regression 
`_.

alexfh wrote:
> alexfh wrote:
> > s/chaning/changing/
> How about the `insert` method of `std::vector<>`?
insert -> emplace should be fine for the vector, but this patch is trying to 
mainly fix the bugs (and I want it to land in clang-3.9).

I will add ``insert`` near the ``push_front``


Repository:
  rL LLVM

https://reviews.llvm.org/D22208



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

Re: [PATCH] D22208: [clang-tidy] Fixes to modernize-use-emplace

2016-07-21 Thread Alexander Kornienko via cfe-commits

alexfh requested changes to this revision.
This revision now requires changes to proceed.


Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:45
@@ -33,3 +44,3 @@
   hasDeclaration(functionDecl(hasName("push_back"))),
-  on(hasType(cxxRecordDecl(hasAnyName("std::vector", "llvm::SmallVector",
-  "std::list", "std::deque");
+  on(hasType(cxxRecordDecl(hasAnyName(SmallVector(
+  ContainersWithPushBack.begin(), ContainersWithPushBack.end()));

What's the reason for explicitly creating a `SmallVector`? 
`VariadicFunction::operator()` takes an `ArrayRef`, which `std::vector<>` 
should be implicitly convertible to. Am I missing something?


Comment at: docs/clang-tidy/checks/modernize-use-emplace.rst:12
@@ +11,3 @@
+uses it. It also doesn't support ``insert`` functions for associative 
containers
+because chaning ``insert`` to ``emplace`` may result in
+`speed regression 
`_.

s/chaning/changing/


Comment at: docs/clang-tidy/checks/modernize-use-emplace.rst:12
@@ +11,3 @@
+uses it. It also doesn't support ``insert`` functions for associative 
containers
+because chaning ``insert`` to ``emplace`` may result in
+`speed regression 
`_.

alexfh wrote:
> s/chaning/changing/
How about the `insert` method of `std::vector<>`?


Repository:
  rL LLVM

https://reviews.llvm.org/D22208



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

Re: [PATCH] D22656: [Clang-apply-replacements] Remove custom version printing; fix some Include What You Use warnings

2016-07-21 Thread Eugene Zelenko via cfe-commits

Eugene.Zelenko updated this revision to Diff 65004.
Eugene.Zelenko added a comment.

Full context diff.


Repository:
  rL LLVM

https://reviews.llvm.org/D22656

Files:
  clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp

Index: clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
===
--- clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
+++ clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
@@ -15,14 +15,29 @@
 
 #include "clang-apply-replacements/Tooling/ApplyReplacements.h"
 #include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
 #include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
-#include "clang/Basic/Version.h"
 #include "clang/Format/Format.h"
+#include "clang/Rewrite/Core/RewriteBuffer.h"
 #include "clang/Rewrite/Core/Rewriter.h"
-#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/StringSet.h"
+#include "llvm/ADT/ArrayRef.h"   // for make...
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
 
 using namespace llvm;
 using namespace clang;
@@ -43,7 +58,6 @@
  "merging/replacing."),
 cl::init(false), cl::cat(ReplacementCategory));
 
-
 static cl::opt DoFormat(
 "format",
 cl::desc("Enable formatting of code changed by applying replacements.\n"
@@ -85,10 +99,6 @@
 };
 } // namespace
 
-static void printVersion() {
-  outs() << "clang-apply-replacements version " CLANG_VERSION_STRING << "\n";
-}
-
 /// \brief Convenience function to get rewritten content for \c Filename from
 /// \c Rewrites.
 ///
@@ -200,8 +210,6 @@
 
 int main(int argc, char **argv) {
   cl::HideUnrelatedOptions(makeArrayRef(VisibleCategories));
-
-  cl::SetVersionPrinter(&printVersion);
   cl::ParseCommandLineOptions(argc, argv);
 
   IntrusiveRefCntPtr DiagOpts(new DiagnosticOptions());


Index: clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
===
--- clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
+++ clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
@@ -15,14 +15,29 @@
 
 #include "clang-apply-replacements/Tooling/ApplyReplacements.h"
 #include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
 #include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
-#include "clang/Basic/Version.h"
 #include "clang/Format/Format.h"
+#include "clang/Rewrite/Core/RewriteBuffer.h"
 #include "clang/Rewrite/Core/Rewriter.h"
-#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/StringSet.h"
+#include "llvm/ADT/ArrayRef.h"   // for make...
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
 
 using namespace llvm;
 using namespace clang;
@@ -43,7 +58,6 @@
  "merging/replacing."),
 cl::init(false), cl::cat(ReplacementCategory));
 
-
 static cl::opt DoFormat(
 "format",
 cl::desc("Enable formatting of code changed by applying replacements.\n"
@@ -85,10 +99,6 @@
 };
 } // namespace
 
-static void printVersion() {
-  outs() << "clang-apply-replacements version " CLANG_VERSION_STRING << "\n";
-}
-
 /// \brief Convenience function to get rewritten content for \c Filename from
 /// \c Rewrites.
 ///
@@ -200,8 +210,6 @@
 
 int main(int argc, char **argv) {
   cl::HideUnrelatedOptions(makeArrayRef(VisibleCategories));
-
-  cl::SetVersionPrinter(&printVersion);
   cl::ParseCommandLineOptions(argc, argv);
 
   IntrusiveRefCntPtr DiagOpts(new DiagnosticOptions());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22656: [Clang-apply-replacements] Remove custom version printing; fix some Include What You Use warnings

2016-07-21 Thread Alexander Kornienko via cfe-commits

alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.

Full context diffs, please. See 
http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface


Repository:
  rL LLVM

https://reviews.llvm.org/D22656



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

Re: [PATCH] D22605: [Clang-tool-extra] Restructure release notes

2016-07-21 Thread Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL276371: Restructure release notes. (authored by 
eugenezelenko).

Changed prior to commit:
  https://reviews.llvm.org/D22605?vs=64806&id=65003#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D22605

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

Index: clang-tools-extra/trunk/docs/ReleaseNotes.rst
===
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst
@@ -59,12 +59,12 @@
 Improvements to clang-tidy
 --
 
-...
+The improvements are...
 
-Clang-tidy changes from 3.9 to 4.0
-^^
+Improvements to include-fixer
+-
 
-...
+The improvements are...
 
 Improvements to modularize
 --


Index: clang-tools-extra/trunk/docs/ReleaseNotes.rst
===
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst
@@ -59,12 +59,12 @@
 Improvements to clang-tidy
 --
 
-...
+The improvements are...
 
-Clang-tidy changes from 3.9 to 4.0
-^^
+Improvements to include-fixer
+-
 
-...
+The improvements are...
 
 Improvements to modularize
 --
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r276371 - Restructure release notes.

2016-07-21 Thread Eugene Zelenko via cfe-commits

Author: eugenezelenko
Date: Thu Jul 21 19:34:42 2016
New Revision: 276371

URL: http://llvm.org/viewvc/llvm-project?rev=276371&view=rev
Log:
Restructure release notes.

Differential revision: https://reviews.llvm.org/D22605

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=276371&r1=276370&r2=276371&view=diff
==
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Thu Jul 21 19:34:42 2016
@@ -59,12 +59,12 @@ The improvements are...
 Improvements to clang-tidy
 --
 
-...
+The improvements are...
 
-Clang-tidy changes from 3.9 to 4.0
-^^
+Improvements to include-fixer
+-
 
-...
+The improvements are...
 
 Improvements to modularize
 --


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

Re: [PATCH] D22605: [Clang-tool-extra] Restructure release notes

2016-07-21 Thread Alexander Kornienko via cfe-commits

alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.

LG


Repository:
  rL LLVM

https://reviews.llvm.org/D22605



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

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

2016-07-21 Thread Alexander Kornienko via cfe-commits

alexfh requested changes to this revision.
This revision now requires changes to proceed.


Comment at: clang-tidy/mpi/MPITidyModule.cpp:30
@@ +29,3 @@
+static ClangTidyModuleRegistry::Add X("mpi-module",
+  "Adds MPI lint checks.");
+

nit: s/lint/clang-tidy/ ;)


Comment at: clang-tidy/mpi/TypeMismatchCheck.cpp:11
@@ +10,3 @@
+#include "TypeMismatchCheck.h"
+#include 
"clang/../../lib/StaticAnalyzer/Checkers/MPI-Checker/MPIFunctionClassifier.h"
+#include "clang/Lex/Lexer.h"

Since the header is used outside of its library, it should be moved to 
clang/include/StaticAnalyzer/


Comment at: clang-tidy/mpi/TypeMismatchCheck.cpp:50
@@ +49,3 @@
+static bool isStandardMPIDatatype(const std::string &MPIDatatype) {
+
+  static std::unordered_set AllTypes = {

nit: Please remove the empty line.


Comment at: clang-tidy/mpi/TypeMismatchCheck.cpp:185
@@ +184,3 @@
+
+  if (Template->getAsCXXRecordDecl()->getNameAsString() != "complex")
+return true;

I think, `getName` can be used here as well.


Comment at: clang-tidy/mpi/TypeMismatchCheck.cpp:256
@@ +255,3 @@
+
+  auto addPair = [&](const size_t BufferIdx, const size_t DatatypeIdx) {
+// Skip null pointer constants and in place 'operators'.

Explicit captures would make the code more transparent.


Comment at: clang-tidy/mpi/TypeMismatchCheck.cpp:256
@@ +255,3 @@
+
+  auto addPair = [&](const size_t BufferIdx, const size_t DatatypeIdx) {
+// Skip null pointer constants and in place 'operators'.

alexfh wrote:
> Explicit captures would make the code more transparent.
It's not clear what this lambda does. Please add a comment.


Comment at: clang-tidy/mpi/TypeMismatchCheck.h:52
@@ +51,3 @@
+  /// \param MPIDatatype MPI datatype name
+  void logError(const Expr *const ArgumentExpression, StringRef BufferTypeName,
+StringRef MPIDatatype);

With just one caller and two lines of implementation this method doesn't make 
the code any better. Please just inline it.


https://reviews.llvm.org/D21962



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

r276370 - [Coverage] Attempt to appease a Windows builder

2016-07-21 Thread Vedant Kumar via cfe-commits

Author: vedantk
Date: Thu Jul 21 19:25:09 2016
New Revision: 276370

URL: http://llvm.org/viewvc/llvm-project?rev=276370&view=rev
Log:
[Coverage] Attempt to appease a Windows builder

The builder prints out the following IR:

  \5CCoverageMapping\5COutput\5Ctest\5Cf1.c

The updated test in r276367 expects path separators to be either '/' or
'\\', so it chokes on the unexpected "5C" stuff. I'm not sure what that
is, but I included a kludge that should work around it.

Failing bot:

  
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/8718

Modified:
cfe/trunk/test/CoverageMapping/abspath.cpp

Modified: cfe/trunk/test/CoverageMapping/abspath.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CoverageMapping/abspath.cpp?rev=276370&r1=276369&r2=276370&view=diff
==
--- cfe/trunk/test/CoverageMapping/abspath.cpp (original)
+++ cfe/trunk/test/CoverageMapping/abspath.cpp Thu Jul 21 19:25:09 2016
@@ -9,7 +9,7 @@
 // RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -emit-llvm 
-main-file-name abspath.cpp ../test/f1.c -o - | FileCheck -check-prefix=RELPATH 
%s
 
 // RELPATH: @__llvm_coverage_mapping = {{.*}}"\01
-// RELPATH: {{[/\\]}}{{.*}}{{[/\\]}}test{{[/\\]}}f1.c
+// RELPATH: {{[/\\]}}{{.*}}{{[/\\][^/\\]*}}test{{[/\\][^/\\]*}}f1.c
 // RELPATH: "
 
 void f1() {}


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

Re: r276361 - Reverting r275115 which caused PR28634.

2016-07-21 Thread Mehdi Amini via cfe-commits


> On Jul 21, 2016, at 4:28 PM, Wolfgang Pieb via cfe-commits 
>  wrote:
> 
> Author: wolfgangp
> Date: Thu Jul 21 18:28:18 2016
> New Revision: 276361
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=276361&view=rev
> Log:
> Reverting r275115 which caused PR28634.
> When empty (forwarding) basic blocks that are referenced by user labels
> are removed, incorrect code may be generated.

Can you add a non-regression test case?
(It seems the test-suite didn’t catch this bug right?)

Thanks,

— 
Mehdi


> 
> 
> Removed:
>cfe/trunk/test/CodeGen/forwarding-blocks-if.c
> Modified:
>cfe/trunk/lib/CodeGen/CGStmt.cpp
> 
> Modified: cfe/trunk/lib/CodeGen/CGStmt.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmt.cpp?rev=276361&r1=276360&r2=276361&view=diff
> ==
> --- cfe/trunk/lib/CodeGen/CGStmt.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGStmt.cpp Thu Jul 21 18:28:18 2016
> @@ -623,14 +623,7 @@ void CodeGenFunction::EmitIfStmt(const I
> RunCleanupsScope ThenScope(*this);
> EmitStmt(S.getThen());
>   }
> -  {
> -auto CurBlock = Builder.GetInsertBlock();
> -EmitBranch(ContBlock);
> -// Eliminate any empty blocks that may have been created by nested
> -// control flow statements in the 'then' clause.
> -if (CurBlock)
> -  SimplifyForwardingBlocks(CurBlock); 
> -  }
> +  EmitBranch(ContBlock);
> 
>   // Emit the 'else' code if present.
>   if (const Stmt *Else = S.getElse()) {
> @@ -646,12 +639,7 @@ void CodeGenFunction::EmitIfStmt(const I
> {
>   // There is no need to emit line number for an unconditional branch.
>   auto NL = ApplyDebugLocation::CreateEmpty(*this);
> -  auto CurBlock = Builder.GetInsertBlock();
>   EmitBranch(ContBlock);
> -  // Eliminate any empty blocks that may have been created by nested
> -  // control flow statements emitted in the 'else' clause.
> -  if (CurBlock)
> -SimplifyForwardingBlocks(CurBlock); 
> }
>   }
> 
> 
> Removed: cfe/trunk/test/CodeGen/forwarding-blocks-if.c
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/forwarding-blocks-if.c?rev=276360&view=auto
> ==
> --- cfe/trunk/test/CodeGen/forwarding-blocks-if.c (original)
> +++ cfe/trunk/test/CodeGen/forwarding-blocks-if.c (removed)
> @@ -1,36 +0,0 @@
> -// RUN: %clang_cc1 %s -emit-llvm -o - | FileCheck %s
> -// Check that no empty blocks are generated for nested ifs.
> -
> -extern void func();
> -
> -int f0(int val) {
> -  if (val == 0) {
> -func();
> -  } else if (val == 1) {
> -func();
> -  }
> -  return 0;
> -}
> -
> -// CHECK-LABEL: define {{.*}}i32 @f0
> -// CHECK: call void {{.*}} @func
> -// CHECK: call void {{.*}} @func
> -// CHECK: br label %[[RETBLOCK1:[^ ]*]]
> -// CHECK: [[RETBLOCK1]]:
> -// CHECK-NOT: br label
> -// CHECK: ret i32
> -
> -int f1(int val, int g) {
> -  if (val == 0)
> -if (g == 1) {
> -  func();
> -}
> -  return 0;
> -}
> -
> -// CHECK-LABEL: define {{.*}}i32 @f1
> -// CHECK: call void {{.*}} @func
> -// CHECK: br label %[[RETBLOCK2:[^ ]*]]
> -// CHECK: [[RETBLOCK2]]:
> -// CHECK-NOT: br label
> -// CHECK: ret i32
> 
> 
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

Re: [PATCH] D19544: Pass for translating math intrinsics to math library calls.

2016-07-21 Thread Michael Zolotukhin via cfe-commits

mzolotukhin accepted this revision.
mzolotukhin added a comment.
This revision is now accepted and ready to land.

LGTM with a small nit: could you please run `opt -instnamer` on your test 
(it'll replace `%0`, `%1`,... with `%tmp0`, `%tmp1` etc, making it easier to 
modify test in future)?

Thanks,
Michael


https://reviews.llvm.org/D19544



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

Re: [PATCH] D19544: Pass for translating math intrinsics to math library calls.

2016-07-21 Thread Matt via cfe-commits

mmasten added a comment.

I think this is just saying that some of the weird types are not supported on 
all targets. For now, is it ok to proceed with checking this code in?

Thanks,

Matt


https://reviews.llvm.org/D19544



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

r276367 - [Coverage] Strengthen a test case

2016-07-21 Thread Vedant Kumar via cfe-commits

Author: vedantk
Date: Thu Jul 21 19:00:02 2016
New Revision: 276367

URL: http://llvm.org/viewvc/llvm-project?rev=276367&view=rev
Log:
[Coverage] Strengthen a test case

We should be able to use `mkdir` without turning on `REQUIRES: shell`.
Moreover, this test should check for a path separator which precedes the
relative filename to make sure that absolute paths are being used.

Modified:
cfe/trunk/test/CoverageMapping/abspath.cpp

Modified: cfe/trunk/test/CoverageMapping/abspath.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CoverageMapping/abspath.cpp?rev=276367&r1=276366&r2=276367&view=diff
==
--- cfe/trunk/test/CoverageMapping/abspath.cpp (original)
+++ cfe/trunk/test/CoverageMapping/abspath.cpp Thu Jul 21 19:00:02 2016
@@ -1,6 +1,3 @@
-// This test requires mkdir.
-// REQUIRES: shell
-//
 // RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -emit-llvm 
-main-file-name abspath.cpp %S/Inputs/../abspath.cpp -o - | FileCheck 
-check-prefix=RMDOTS %s
 
 // RMDOTS: @__llvm_coverage_mapping = {{.*}}"\01
@@ -12,7 +9,7 @@
 // RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -emit-llvm 
-main-file-name abspath.cpp ../test/f1.c -o - | FileCheck -check-prefix=RELPATH 
%s
 
 // RELPATH: @__llvm_coverage_mapping = {{.*}}"\01
-// RELPATH: test{{.*}}f1.c
+// RELPATH: {{[/\\]}}{{.*}}{{[/\\]}}test{{[/\\]}}f1.c
 // RELPATH: "
 
 void f1() {}


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

Re: [PATCH] D19311: [analyzer] Self Assignment Checker

2016-07-21 Thread Devin Coughlin via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL276365: [analyzer] Add checker modeling potential C++ 
self-assignment (authored by dcoughlin).

Changed prior to commit:
  https://reviews.llvm.org/D19311?vs=64864&id=64996#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D19311

Files:
  cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
  cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h
  cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  cfe/trunk/lib/StaticAnalyzer/Checkers/CXXSelfAssignmentChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  cfe/trunk/test/Analysis/self-assign.cpp

Index: cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -426,6 +426,13 @@
   //   Count naming convention errors more aggressively.
   if (isa(D))
 return false;
+  // We also want to reanalyze all C++ copy and move assignment operators to
+  // separately check the two cases where 'this' aliases with the parameter and
+  // where it may not. (cplusplus.SelfAssignmentChecker)
+  if (const auto *MD = dyn_cast(D)) {
+if (MD->isCopyAssignmentOperator() || MD->isMoveAssignmentOperator())
+  return false;
+  }
 
   // Otherwise, if we visited the function before, do not reanalyze it.
   return Visited.count(D);
@@ -437,9 +444,7 @@
   // We want to reanalyze all ObjC methods as top level to report Retain
   // Count naming convention errors more aggressively. But we should tune down
   // inlining when reanalyzing an already inlined function.
-  if (Visited.count(D)) {
-assert(isa(D) &&
-   "We are only reanalyzing ObjCMethods.");
+  if (Visited.count(D) && isa(D)) {
 const ObjCMethodDecl *ObjCM = cast(D);
 if (ObjCM->getMethodFamily() != OMF_init)
   return ExprEngine::Inline_Minimal;
Index: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1693,3 +1693,56 @@
   }
   return nullptr;
 }
+
+PathDiagnosticPiece *
+CXXSelfAssignmentBRVisitor::VisitNode(const ExplodedNode *Succ,
+  const ExplodedNode *Pred,
+  BugReporterContext &BRC, BugReport &BR) {
+  if (Satisfied)
+return nullptr;
+
+  auto Edge = Succ->getLocation().getAs();
+  if (!Edge.hasValue())
+return nullptr;
+
+  auto Tag = Edge->getTag();
+  if (!Tag)
+return nullptr;
+
+  if (Tag->getTagDescription() != "cplusplus.SelfAssignment")
+return nullptr;
+
+  Satisfied = true;
+
+  const auto *Met =
+  dyn_cast(Succ->getCodeDecl().getAsFunction());
+  assert(Met && "Not a C++ method.");
+  assert((Met->isCopyAssignmentOperator() || Met->isMoveAssignmentOperator()) &&
+ "Not a copy/move assignment operator.");
+
+  const auto *LCtx = Edge->getLocationContext();
+
+  const auto &State = Succ->getState();
+  auto &SVB = State->getStateManager().getSValBuilder();
+
+  const auto Param =
+  State->getSVal(State->getRegion(Met->getParamDecl(0), LCtx));
+  const auto This =
+  State->getSVal(SVB.getCXXThis(Met, LCtx->getCurrentStackFrame()));
+
+  auto L = PathDiagnosticLocation::create(Met, BRC.getSourceManager());
+
+  if (!L.isValid() || !L.asLocation().isValid())
+return nullptr;
+
+  SmallString<256> Buf;
+  llvm::raw_svector_ostream Out(Buf);
+
+  Out << "Assuming " << Met->getParamDecl(0)->getName() <<
+((Param == This) ? " == " : " != ") << "*this";
+
+  auto *Piece = new PathDiagnosticEventPiece(L, Out.str());
+  Piece->addRange(Met->getSourceRange());
+
+  return Piece;
+}
Index: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -3104,6 +3104,7 @@
 R->addVisitor(llvm::make_unique());
 R->addVisitor(llvm::make_unique());
 R->addVisitor(llvm::make_unique());
+R->addVisitor(llvm::make_unique());
 
 BugReport::VisitorList visitors;
 unsigned origReportConfigToken, finalReportConfigToken;
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -22,6 +22,7 @@
   CheckerDocumentation.cpp
   ChrootChecker.cpp
   ClangCheckers.cpp
+  CXXSelfAssignmentChecker.cpp
   DeadStoresChecker.cpp
   DebugCheckers.cpp
   DereferenceCheck

r276365 - [analyzer] Add checker modeling potential C++ self-assignment

2016-07-21 Thread Devin Coughlin via cfe-commits

Author: dcoughlin
Date: Thu Jul 21 18:42:31 2016
New Revision: 276365

URL: http://llvm.org/viewvc/llvm-project?rev=276365&view=rev
Log:
[analyzer] Add checker modeling potential C++ self-assignment

This checker checks copy and move assignment operators whether they are
protected against self-assignment. Since C++ core guidelines discourages
explicit checking for `&rhs==this` in general we take a different approach: in
top-frame analysis we branch the exploded graph for two cases, where &rhs==this
and &rhs!=this and let existing checkers (e.g. unix.Malloc) do the rest of the
work. It is important that we check all copy and move assignment operator in top
frame even if we checked them already since self-assignments may happen
undetected even in the same translation unit (e.g. using random indices for an
array what may or may not be the same).

This reapplies r275820 after fixing a string-lifetime issue discovered by the
bots.

A patch by Ádám Balogh!

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

Added:
cfe/trunk/lib/StaticAnalyzer/Checkers/CXXSelfAssignmentChecker.cpp
cfe/trunk/test/Analysis/self-assign.cpp
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h
cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td?rev=276365&r1=276364&r2=276365&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td Thu Jul 21 
18:42:31 2016
@@ -247,6 +247,10 @@ def NewDeleteLeaksChecker : Checker<"New
   HelpText<"Check for memory leaks. Traces memory managed by new/delete.">,
   DescFile<"MallocChecker.cpp">;
 
+def CXXSelfAssignmentChecker : Checker<"SelfAssignment">,
+  HelpText<"Checks C++ copy and move assignment operators for self 
assignment">,
+  DescFile<"CXXSelfAssignmentChecker.cpp">;
+
 } // end: "cplusplus"
 
 let ParentPackage = CplusplusAlpha in {

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h?rev=276365&r1=276364&r2=276365&view=diff
==
--- 
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h 
(original)
+++ 
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h 
Thu Jul 21 18:42:31 2016
@@ -331,6 +331,22 @@ public:
  BugReport &BR) override;
 };
 
+class CXXSelfAssignmentBRVisitor final
+  : public BugReporterVisitorImpl {
+  
+  bool Satisfied;
+
+public:
+  CXXSelfAssignmentBRVisitor() : Satisfied(false) {}
+
+  void Profile(llvm::FoldingSetNodeID &ID) const override {}
+
+  PathDiagnosticPiece *VisitNode(const ExplodedNode *Succ,
+ const ExplodedNode *Pred,
+ BugReporterContext &BRC,
+ BugReport &BR) override;
+};
+
 namespace bugreporter {
 
 /// Attempts to add visitors to trace a null or undefined value back to its

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt?rev=276365&r1=276364&r2=276365&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt Thu Jul 21 18:42:31 
2016
@@ -22,6 +22,7 @@ add_clang_library(clangStaticAnalyzerChe
   CheckerDocumentation.cpp
   ChrootChecker.cpp
   ClangCheckers.cpp
+  CXXSelfAssignmentChecker.cpp
   DeadStoresChecker.cpp
   DebugCheckers.cpp
   DereferenceChecker.cpp

Added: cfe/trunk/lib/StaticAnalyzer/Checkers/CXXSelfAssignmentChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CXXSelfAssignmentChecker.cpp?rev=276365&view=auto
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CXXSelfAssignmentChecker.cpp (added)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CXXSelfAssignmentChecker.cpp Thu Jul 
21 18:42:31 2016
@@ -0,0 +1,62 @@
+//=== CXXSelfAssignmentChecker.cpp -*- C++ 
-*--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License

r276361 - Reverting r275115 which caused PR28634.

2016-07-21 Thread Wolfgang Pieb via cfe-commits

Author: wolfgangp
Date: Thu Jul 21 18:28:18 2016
New Revision: 276361

URL: http://llvm.org/viewvc/llvm-project?rev=276361&view=rev
Log:
Reverting r275115 which caused PR28634.
When empty (forwarding) basic blocks that are referenced by user labels
are removed, incorrect code may be generated.


Removed:
cfe/trunk/test/CodeGen/forwarding-blocks-if.c
Modified:
cfe/trunk/lib/CodeGen/CGStmt.cpp

Modified: cfe/trunk/lib/CodeGen/CGStmt.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmt.cpp?rev=276361&r1=276360&r2=276361&view=diff
==
--- cfe/trunk/lib/CodeGen/CGStmt.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGStmt.cpp Thu Jul 21 18:28:18 2016
@@ -623,14 +623,7 @@ void CodeGenFunction::EmitIfStmt(const I
 RunCleanupsScope ThenScope(*this);
 EmitStmt(S.getThen());
   }
-  {
-auto CurBlock = Builder.GetInsertBlock();
-EmitBranch(ContBlock);
-// Eliminate any empty blocks that may have been created by nested
-// control flow statements in the 'then' clause.
-if (CurBlock)
-  SimplifyForwardingBlocks(CurBlock); 
-  }
+  EmitBranch(ContBlock);
 
   // Emit the 'else' code if present.
   if (const Stmt *Else = S.getElse()) {
@@ -646,12 +639,7 @@ void CodeGenFunction::EmitIfStmt(const I
 {
   // There is no need to emit line number for an unconditional branch.
   auto NL = ApplyDebugLocation::CreateEmpty(*this);
-  auto CurBlock = Builder.GetInsertBlock();
   EmitBranch(ContBlock);
-  // Eliminate any empty blocks that may have been created by nested
-  // control flow statements emitted in the 'else' clause.
-  if (CurBlock)
-SimplifyForwardingBlocks(CurBlock); 
 }
   }
 

Removed: cfe/trunk/test/CodeGen/forwarding-blocks-if.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/forwarding-blocks-if.c?rev=276360&view=auto
==
--- cfe/trunk/test/CodeGen/forwarding-blocks-if.c (original)
+++ cfe/trunk/test/CodeGen/forwarding-blocks-if.c (removed)
@@ -1,36 +0,0 @@
-// RUN: %clang_cc1 %s -emit-llvm -o - | FileCheck %s
-// Check that no empty blocks are generated for nested ifs.
-
-extern void func();
-
-int f0(int val) {
-  if (val == 0) {
-func();
-  } else if (val == 1) {
-func();
-  }
-  return 0;
-}
-
-// CHECK-LABEL: define {{.*}}i32 @f0
-// CHECK: call void {{.*}} @func
-// CHECK: call void {{.*}} @func
-// CHECK: br label %[[RETBLOCK1:[^ ]*]]
-// CHECK: [[RETBLOCK1]]:
-// CHECK-NOT: br label
-// CHECK: ret i32
-
-int f1(int val, int g) {
-  if (val == 0)
-if (g == 1) {
-  func();
-}
-  return 0;
-}
-
-// CHECK-LABEL: define {{.*}}i32 @f1
-// CHECK: call void {{.*}} @func
-// CHECK: br label %[[RETBLOCK2:[^ ]*]]
-// CHECK: [[RETBLOCK2]]:
-// CHECK-NOT: br label
-// CHECK: ret i32


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

r276356 - [profile] update test case with interface change.

2016-07-21 Thread Xinliang David Li via cfe-commits

Author: davidxl
Date: Thu Jul 21 18:19:39 2016
New Revision: 276356

URL: http://llvm.org/viewvc/llvm-project?rev=276356&view=rev
Log:
[profile] update test case with interface change.

See http://reviews.llvm.org/D22613, http://reviews.llvm.org/D22614

Modified:
cfe/trunk/test/Profile/c-generate.c
cfe/trunk/test/Profile/gcc-flag-compatibility.c

Modified: cfe/trunk/test/Profile/c-generate.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Profile/c-generate.c?rev=276356&r1=276355&r2=276356&view=diff
==
--- cfe/trunk/test/Profile/c-generate.c (original)
+++ cfe/trunk/test/Profile/c-generate.c Thu Jul 21 18:19:39 2016
@@ -3,9 +3,7 @@
 // RUN: %clang_cc1 %s -o - -emit-llvm -fprofile-instrument=none | FileCheck %s 
--check-prefix=PROF-INSTR-NONE
 // RUN: not %clang_cc1 %s -o - -emit-llvm -fprofile-instrument=garbage 2>&1 | 
FileCheck %s --check-prefix=PROF-INSTR-GARBAGE
 //
-// PROF-INSTR-PATH: private constant [24 x i8] c"c-generate-test.profraw\00"
-// PROF-INSTR-PATH: call void @__llvm_profile_override_default_filename(i8* 
getelementptr inbounds ([24 x i8], [24 x i8]* @0, i32 0, i32 0))
-// PROF-INSTR-PATH: declare void @__llvm_profile_override_default_filename(i8*)
+// PROF-INSTR-PATH: constant [24 x i8] c"c-generate-test.profraw\00"
 //
 // PROF-INSTR-NONE-NOT: @__profn_main
 // PROF-INSTR-GARBAGE: invalid PGO instrumentor in argument 
'-fprofile-instrument=garbage'

Modified: cfe/trunk/test/Profile/gcc-flag-compatibility.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Profile/gcc-flag-compatibility.c?rev=276356&r1=276355&r2=276356&view=diff
==
--- cfe/trunk/test/Profile/gcc-flag-compatibility.c (original)
+++ cfe/trunk/test/Profile/gcc-flag-compatibility.c Thu Jul 21 18:19:39 2016
@@ -7,17 +7,12 @@
 // -fprofile-use=Uses the profile file /default.profdata
 // -fprofile-use=/file   Uses the profile file /file
 
-// FIXME: IRPGO shouldn't use the override API when no profraw name is given.
-// Check that -fprofile-generate overrides the default profraw.
 // RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate | FileCheck 
-check-prefix=PROFILE-GEN %s
-// PROFILE-GEN: call void @__llvm_profile_override_default_filename
-// PROFILE-GEN: declare void @__llvm_profile_override_default_filename(i8*)
+// PROFILE-GEN: __llvm_profile_filename
 
 // Check that -fprofile-generate=/path/to generates /path/to/default.profraw
 // RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate=/path/to | 
FileCheck -check-prefix=PROFILE-GEN-EQ %s
-// PROFILE-GEN-EQ: private constant [25 x i8] 
c"/path/to{{/|\\5C}}default.profraw\00"
-// PROFILE-GEN-EQ: call void @__llvm_profile_override_default_filename(i8* 
getelementptr inbounds ([25 x i8], [25 x i8]* @0, i32 0, i32 0))
-// PROFILE-GEN-EQ: declare void @__llvm_profile_override_default_filename(i8*)
+// PROFILE-GEN-EQ: constant [25 x i8] c"/path/to{{/|\\5C}}default.profraw\00"
 
 // Check that -fprofile-use=some/path reads some/path/default.profdata
 // RUN: rm -rf %t.dir


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

r276352 - [Sema] Handle errors during rewriteBuiltinFunctionDecl

2016-07-21 Thread David Majnemer via cfe-commits

Author: majnemer
Date: Thu Jul 21 18:03:43 2016
New Revision: 276352

URL: http://llvm.org/viewvc/llvm-project?rev=276352&view=rev
Log:
[Sema] Handle errors during rewriteBuiltinFunctionDecl

rewriteBuiltinFunctionDecl can encounter errors when performing
DefaultFunctionArrayLvalueConversion.  These errors were not handled
which led to a null pointer dereference.

This fixes PR28651.

Modified:
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/Sema/builtins.cl

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=276352&r1=276351&r2=276352&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Jul 21 18:03:43 2016
@@ -5051,7 +5051,11 @@ static FunctionDecl *rewriteBuiltinFunct
   for (QualType ParamType : FT->param_types()) {
 
 // Convert array arguments to pointer to simplify type lookup.
-Expr *Arg = 
Sema->DefaultFunctionArrayLvalueConversion(ArgExprs[i++]).get();
+ExprResult ArgRes =
+Sema->DefaultFunctionArrayLvalueConversion(ArgExprs[i++]);
+if (ArgRes.isInvalid())
+  return nullptr;
+Expr *Arg = ArgRes.get();
 QualType ArgType = Arg->getType();
 if (!ParamType->isPointerType() ||
 ParamType.getQualifiers().hasAddressSpace() ||

Modified: cfe/trunk/test/Sema/builtins.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/builtins.cl?rev=276352&r1=276351&r2=276352&view=diff
==
--- cfe/trunk/test/Sema/builtins.cl (original)
+++ cfe/trunk/test/Sema/builtins.cl Thu Jul 21 18:03:43 2016
@@ -1,8 +1,11 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic
-// expected-no-diagnostics
 
 kernel void test(global float *out, global float *in, global int* in2) {
   out[0] = __builtin_nanf("");
   __builtin_memcpy(out, in, 32);
   out[0] = __builtin_frexpf(in[0], in2);
 }
+
+void pr28651() {
+  __builtin_alloca(value); // expected-error{{use of undeclared identifier}}
+}


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

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

2016-07-21 Thread Alexander Droste via cfe-commits

Alexander_Droste marked 5 inline comments as done.
Alexander_Droste added a comment.

https://reviews.llvm.org/D21962



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

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

2016-07-21 Thread Alexander Droste via cfe-commits

Alexander_Droste updated this revision to Diff 64985.
Alexander_Droste added a comment.

- use `llvm::StringMap` instead of `std::map`
- `getQualifiedNameAsString` -> `getName`
- remove redundant map lookup
- create MPIi module
- replace `misc` with  `mpi` within the check

I created an MPI module based on how `misc` is set up. Somehow the 
`mpi-type-mismatch` is not found when the tests are run or when I try to 
apply the check on a project but I am assuming you see at first glance what
ingredient missing.


https://reviews.llvm.org/D21962

Files:
  clang-tidy/CMakeLists.txt
  clang-tidy/mpi/CMakeLists.txt
  clang-tidy/mpi/MPITidyModule.cpp
  clang-tidy/mpi/TypeMismatchCheck.cpp
  clang-tidy/mpi/TypeMismatchCheck.h
  clang-tidy/plugin/CMakeLists.txt
  clang-tidy/tool/CMakeLists.txt
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/mpi-type-mismatch.rst
  test/clang-tidy/Inputs/mpi-type-mismatch/mpimock.h
  test/clang-tidy/mpi-type-mismatch.cpp

Index: test/clang-tidy/mpi-type-mismatch.cpp
===
--- /dev/null
+++ test/clang-tidy/mpi-type-mismatch.cpp
@@ -0,0 +1,254 @@
+// RUN: %check_clang_tidy %s mpi-type-mismatch %t -- -- -I %S/Inputs/mpi-type-mismatch
+
+#include "mpimock.h"
+
+void charNegativeTest() {
+  unsigned char buf;
+  MPI_Send(&buf, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'unsigned char' does not match the MPI datatype 'MPI_CHAR' [mpi-type-mismatch]
+  
+  int buf2;
+  MPI_Send(&buf2, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'int' does not match the MPI datatype 'MPI_CHAR' [mpi-type-mismatch]
+  
+  short buf3;
+  MPI_Send(&buf3, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'short' does not match the MPI datatype 'MPI_CHAR' [mpi-type-mismatch]
+
+  long buf4;
+  MPI_Send(&buf4, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'long' does not match the MPI datatype 'MPI_CHAR' [mpi-type-mismatch]
+  
+  int8_t buf5;
+  MPI_Send(&buf5, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'int8_t' does not match the MPI datatype 'MPI_CHAR' [mpi-type-mismatch]
+
+  uint16_t buf6;
+  MPI_Send(&buf6, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'uint16_t' does not match the MPI datatype 'MPI_CHAR' [mpi-type-mismatch]
+
+  long double _Complex buf7;
+  MPI_Send(&buf7, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'long double _Complex' does not match the MPI datatype 'MPI_CHAR' [mpi-type-mismatch]
+
+  std::complex buf8;
+  MPI_Send(&buf8, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'complex' does not match the MPI datatype 'MPI_CHAR' [mpi-type-mismatch]
+}
+
+void intNegativeTest() {
+  unsigned char buf;
+  MPI_Send(&buf, 1, MPI_INT, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'unsigned char' does not match the MPI datatype 'MPI_INT' [mpi-type-mismatch]
+  
+  unsigned buf2;
+  MPI_Send(&buf2, 1, MPI_INT, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'unsigned int' does not match the MPI datatype 'MPI_INT' [mpi-type-mismatch]
+  
+  short buf3;
+  MPI_Send(&buf3, 1, MPI_INT, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'short' does not match the MPI datatype 'MPI_INT' [mpi-type-mismatch]
+
+  long buf4;
+  MPI_Send(&buf4, 1, MPI_INT, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'long' does not match the MPI datatype 'MPI_INT' [mpi-type-mismatch]
+  
+  int8_t buf5;
+  MPI_Send(&buf5, 1, MPI_INT, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'int8_t' does not match the MPI datatype 'MPI_INT' [mpi-type-mismatch]
+
+  uint16_t buf6;
+  MPI_Send(&buf6, 1, MPI_INT, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'uint16_t' does not match the MPI datatype 'MPI_INT' [mpi-type-mismatch]
+
+  long double _Complex buf7;
+  MPI_Send(&buf7, 1, MPI_INT, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'long double _Complex' does not match the MPI datatype 'MPI_INT' [mpi-type-mismatch]
+
+  std::complex buf8;
+  MPI_Send(&buf8, 1, MPI_INT, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'complex' does not match the MPI datatype 'MPI_INT' [mpi-type-mismatch]
+}
+
+void longNegativeTest() {
+  char buf;
+  MPI_Send(&buf, 1, MPI_LONG, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'char' does not match the MPI datatype 'MPI_LONG' [mpi-type-mismatch]
+  
+  unsigned buf2;
+  MPI_Send(&buf2, 1, MPI_LONG, 0, 0, MPI_COMM_WORLD);
+

Re: [PATCH] D22514: CloneDetection now respects statement specific data when searching for clones.

2016-07-21 Thread Raphael Isemann via cfe-commits

teemperor added a comment.

Binary size increases from 33017160 Bytes to 33060464 Bytes (+40 KiB).

I'm not sure if that's too much for such a minor feature, so I've added two new 
revisions:

- One is the original patch with the other mentioned issues fixed (so, same +40 
KiB size increase here).

- The other uses only one collect function which reduces the binary size 
increase to +2 KiB (but uses chained `if`'s with `dyn_cast`).

The problem is that the LLVM coding standard isn't a big fan chained `if`s with 
`dyn_cast` and recommends doing it in the same way as the original patch for 
performance reasons.



Comment at: lib/Analysis/CloneDetection.cpp:145
@@ +144,3 @@
+  }
+  #include "clang/AST/StmtNodes.inc"
+

v.g.vassilev wrote:
> Why do we need this? Wouldn't `callAddDataStmt` properly dispatch to the 
> concrete method call?
`callAddDataFoo` would call all `addDataXXX` functions for `Foo` and all parent 
classes of `Foo`. So `callAddDataStmt` would only call `addDataStmt` because 
`Stmt` has no parent class. `callAddCompoundStmt` would call `addDataStmt` 
(through `callAddDataStmt`) and `addDataCompoundStmt` and so on.


Comment at: lib/Analysis/CloneDetection.cpp:306
@@ -112,1 +305,3 @@
 for (Stmt const *Child : Parent->children()) {
+  if (!Child)
+continue;

v.g.vassilev wrote:
> In which case this happens?
For example
```
if (a < b) return 1;
``` 
would have two nullptr children in the AST (the missing 'init' and 'else' 
Stmts):


```
`-IfStmt 0x2d595f8 
  |-<<>>
  |-BinaryOperator 0x2d59598  '_Bool' '>'
  | |-ImplicitCastExpr 0x2d59568  'int' 
  | | `-DeclRefExpr 0x2d59518  'int' lvalue Var 0x2d59418 'a' 'int'
  | `-ImplicitCastExpr 0x2d59580  'int' 
  |   `-DeclRefExpr 0x2d59540  'int' lvalue Var 0x2d59488 'b' 'int'
  |-ReturnStmt 0x2d595e0 
  | `-IntegerLiteral 0x2d595c0  'int' 1
  `-<<>>
```


https://reviews.llvm.org/D22514



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

[PATCH] D22656: [Clang-apply-replacements] Remove custom version printing; fix some Include What You Use warnings

2016-07-21 Thread Eugene Zelenko via cfe-commits

Eugene.Zelenko created this revision.
Eugene.Zelenko added a reviewer: alexfh.
Eugene.Zelenko added a subscriber: cfe-commits.
Eugene.Zelenko set the repository for this revision to rL LLVM.

I checked this patch on my own build on RHEL 6. Regressions were OK.

Repository:
  rL LLVM

https://reviews.llvm.org/D22656

Files:
  clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp

Index: clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
===
--- clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
+++ clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
@@ -15,14 +15,29 @@
 
 #include "clang-apply-replacements/Tooling/ApplyReplacements.h"
 #include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
 #include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
-#include "clang/Basic/Version.h"
 #include "clang/Format/Format.h"
+#include "clang/Rewrite/Core/RewriteBuffer.h"
 #include "clang/Rewrite/Core/Rewriter.h"
-#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/StringSet.h"
+#include "llvm/ADT/ArrayRef.h"   // for make...
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
 
 using namespace llvm;
 using namespace clang;
@@ -43,7 +58,6 @@
  "merging/replacing."),
 cl::init(false), cl::cat(ReplacementCategory));
 
-
 static cl::opt DoFormat(
 "format",
 cl::desc("Enable formatting of code changed by applying replacements.\n"
@@ -85,10 +99,6 @@
 };
 } // namespace
 
-static void printVersion() {
-  outs() << "clang-apply-replacements version " CLANG_VERSION_STRING << "\n";
-}
-
 /// \brief Convenience function to get rewritten content for \c Filename from
 /// \c Rewrites.
 ///
@@ -200,8 +210,6 @@
 
 int main(int argc, char **argv) {
   cl::HideUnrelatedOptions(makeArrayRef(VisibleCategories));
-
-  cl::SetVersionPrinter(&printVersion);
   cl::ParseCommandLineOptions(argc, argv);
 
   IntrusiveRefCntPtr DiagOpts(new DiagnosticOptions());


Index: clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
===
--- clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
+++ clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
@@ -15,14 +15,29 @@
 
 #include "clang-apply-replacements/Tooling/ApplyReplacements.h"
 #include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
 #include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
-#include "clang/Basic/Version.h"
 #include "clang/Format/Format.h"
+#include "clang/Rewrite/Core/RewriteBuffer.h"
 #include "clang/Rewrite/Core/Rewriter.h"
-#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/StringSet.h"
+#include "llvm/ADT/ArrayRef.h"   // for make...
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
 
 using namespace llvm;
 using namespace clang;
@@ -43,7 +58,6 @@
  "merging/replacing."),
 cl::init(false), cl::cat(ReplacementCategory));
 
-
 static cl::opt DoFormat(
 "format",
 cl::desc("Enable formatting of code changed by applying replacements.\n"
@@ -85,10 +99,6 @@
 };
 } // namespace
 
-static void printVersion() {
-  outs() << "clang-apply-replacements version " CLANG_VERSION_STRING << "\n";
-}
-
 /// \brief Convenience function to get rewritten content for \c Filename from
 /// \c Rewrites.
 ///
@@ -200,8 +210,6 @@
 
 int main(int argc, char **argv) {
   cl::HideUnrelatedOptions(makeArrayRef(VisibleCategories));
-
-  cl::SetVersionPrinter(&printVersion);
   cl::ParseCommandLineOptions(argc, argv);
 
   IntrusiveRefCntPtr DiagOpts(new DiagnosticOptions());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22514: CloneDetection now respects statement specific data when searching for clones.

2016-07-21 Thread Raphael Isemann via cfe-commits

teemperor updated this revision to Diff 64983.
teemperor added a comment.

- Replaced visitor-style utility functions with chained `if`s to reduce binary 
size.


https://reviews.llvm.org/D22514

Files:
  lib/Analysis/CloneDetection.cpp
  test/Analysis/copypaste/test-min-max.cpp

Index: test/Analysis/copypaste/test-min-max.cpp
===
--- test/Analysis/copypaste/test-min-max.cpp
+++ test/Analysis/copypaste/test-min-max.cpp
@@ -18,14 +18,6 @@
 
 // False positives below. These clones should not be reported.
 
-// FIXME: Detect different binary operator kinds.
-int min1(int a, int b) { // expected-note{{Related code clone is here.}}
-  log();
-  if (a < b)
-return a;
-  return b;
-}
-
 // FIXME: Detect different variable patterns.
 int min2(int a, int b) { // expected-note{{Related code clone is here.}}
   log();
@@ -36,6 +28,13 @@
 
 // Functions below are not clones and should not be reported.
 
+int min1(int a, int b) { // no-warning
+  log();
+  if (a < b)
+return a;
+  return b;
+}
+
 int foo(int a, int b) { // no-warning
   return a + b;
 }
Index: lib/Analysis/CloneDetection.cpp
===
--- lib/Analysis/CloneDetection.cpp
+++ lib/Analysis/CloneDetection.cpp
@@ -78,6 +78,157 @@
 SourceLocation StmtSequence::getEndLoc() const { return back()->getLocEnd(); }
 
 namespace {
+/// \brief Collects the data of a single Stmt.
+///
+/// The data of a statement that isn't collected by this class is considered to
+/// be unimportant when comparing two statements. This means that this class
+/// defines what a 'similar' clone is. For example, this class doesn't collect
+/// names of variables, which makes statements that only differ in the names of
+/// the referenced variables clones of each other.
+class StmtDataCollector {
+  ASTContext &Context;
+  llvm::FoldingSetNodeID &D;
+
+public:
+
+  /// \brief Collects data from the given Stmt and saves it into the given
+  ///FoldingSetNodeID.
+  StmtDataCollector(Stmt const *S, ASTContext &Context,
+llvm::FoldingSetNodeID &D) : Context(Context), D(D) {
+collectData(S);
+  }
+
+private:
+  // Below are utility methods for adding generic data to the FoldingSetNodeID.
+
+  void addData(unsigned Data) {
+D.AddInteger(Data);
+  }
+
+  void addData(llvm::StringRef const &String) {
+D.AddString(String);
+  }
+
+  void addData(std::string const &String) {
+D.AddString(String);
+  }
+
+  void addData(QualType QT) {
+addData(QT.getAsString());
+  }
+
+  // Utility macro for collecting and adding statement specific data.
+  #define DEF_ADD_DATA(CLASS, CODE)\
+  if (auto S = dyn_cast(GivenStmt)) { \
+CODE;  \
+  }
+
+  /// \brief Collects all node-specific data from the given Stmt.
+  void collectData(Stmt const *GivenStmt) {
+addData(GivenStmt->getStmtClass());
+
+DEF_ADD_DATA(Expr, { addData(S->getType()); })
+
+//--- Builtin functionality --//
+DEF_ADD_DATA(ArrayTypeTraitExpr, { addData(S->getTrait()); })
+DEF_ADD_DATA(AtomicExpr, {
+  addData(S->isVolatile());
+  addData(S->getOp());
+})
+DEF_ADD_DATA(ExpressionTraitExpr, { addData(S->getTrait()); })
+DEF_ADD_DATA(PredefinedExpr, { addData(S->getIdentType()); })
+DEF_ADD_DATA(TypeTraitExpr, { addData(S->getTrait()); })
+
+//--- Calls --//
+DEF_ADD_DATA(CXXOperatorCallExpr, { addData(S->getOperator()); })
+DEF_ADD_DATA(CallExpr, {
+  addData(S->getDirectCallee()->getQualifiedNameAsString());
+})
+
+//--- Exceptions -//
+DEF_ADD_DATA(CXXCatchStmt, { addData(S->getCaughtType()); })
+
+//--- C++ OOP Stmts --//
+DEF_ADD_DATA(CXXDeleteExpr, {
+  addData(S->isArrayFormAsWritten());
+  addData(S->isGlobalDelete());
+})
+
+//--- Casts --//
+DEF_ADD_DATA(ObjCBridgedCastExpr, { addData(S->getBridgeKind()); })
+
+//--- Miscellaneous Exprs //
+DEF_ADD_DATA(BinaryOperator, { addData(S->getOpcode()); })
+DEF_ADD_DATA(UnaryOperator, { addData(S->getOpcode()); })
+
+//--- Control flow ---//
+DEF_ADD_DATA(GotoStmt, { addData(S->getLabel()->getName()); })
+DEF_ADD_DATA(IndirectGotoStmt, {
+   if (S->getConstantTarget())
+ addData(S->getConstantTarget()->getName());
+ })
+DEF_ADD_DATA(LabelStmt, { addData(S->getDecl()->getName()); })
+DEF_ADD_DATA(MSDependentExistsStmt, { addData(S->isI

Re: [PATCH] D22514: CloneDetection now respects statement specific data when searching for clones.

2016-07-21 Thread Raphael Isemann via cfe-commits

teemperor updated this revision to Diff 64982.
teemperor marked 3 inline comments as done.
teemperor added a comment.

- Added `FIXME:` and removed duplicate "for example".


https://reviews.llvm.org/D22514

Files:
  lib/Analysis/CloneDetection.cpp
  test/Analysis/copypaste/test-min-max.cpp

Index: test/Analysis/copypaste/test-min-max.cpp
===
--- test/Analysis/copypaste/test-min-max.cpp
+++ test/Analysis/copypaste/test-min-max.cpp
@@ -18,14 +18,6 @@
 
 // False positives below. These clones should not be reported.
 
-// FIXME: Detect different binary operator kinds.
-int min1(int a, int b) { // expected-note{{Related code clone is here.}}
-  log();
-  if (a < b)
-return a;
-  return b;
-}
-
 // FIXME: Detect different variable patterns.
 int min2(int a, int b) { // expected-note{{Related code clone is here.}}
   log();
@@ -36,6 +28,13 @@
 
 // Functions below are not clones and should not be reported.
 
+int min1(int a, int b) { // no-warning
+  log();
+  if (a < b)
+return a;
+  return b;
+}
+
 int foo(int a, int b) { // no-warning
   return a + b;
 }
Index: lib/Analysis/CloneDetection.cpp
===
--- lib/Analysis/CloneDetection.cpp
+++ lib/Analysis/CloneDetection.cpp
@@ -78,6 +78,199 @@
 SourceLocation StmtSequence::getEndLoc() const { return back()->getLocEnd(); }
 
 namespace {
+/// \brief Defines empty addData methods for each Stmt class.
+///
+/// StmtDataCollector inherits from this class so that each Stmt has an empty
+/// fallback addData method. This prevents compilation failures from missing
+/// methods if a new Stmt subclass is added to clang. Also, many subclasses
+/// don't have any special data attached to them, so we don't need to manually
+/// define empty addData methods in StmtDataCollector if they are already
+/// defined here.
+class StmtDataCollectorDefiner {
+protected:
+  #define STMT(CLASS, PARENT)  \
+  void addData##CLASS(CLASS const *S) {\
+  }
+  #include "clang/AST/StmtNodes.inc"
+};
+} // end anonymous namespace
+
+namespace {
+/// \brief Collects the data of a single Stmt.
+///
+/// The data of a statement that isn't collected by this class is considered to
+/// be unimportant when comparing two statements. This means that this class
+/// defines what a 'similar' clone is. For example, this class doesn't collect
+/// names of variables, which makes statements that only differ in the names of
+/// the referenced variables clones of each other.
+class StmtDataCollector : StmtDataCollectorDefiner {
+  ASTContext &Context;
+  llvm::FoldingSetNodeID &D;
+
+public:
+
+  /// \brief Collects data from the given Stmt and saves it into the given
+  ///FoldingSetNodeID.
+  StmtDataCollector(Stmt const *S, ASTContext &Context,
+llvm::FoldingSetNodeID &D) : Context(Context), D(D) {
+collectData(S);
+  }
+
+protected:
+  // Below are utility methods for adding generic data to the FoldingSetNodeID.
+
+  void addData(unsigned Data) {
+D.AddInteger(Data);
+  }
+
+  void addData(llvm::StringRef const &String) {
+D.AddString(String);
+  }
+
+  void addData(std::string const &String) {
+D.AddString(String);
+  }
+
+  void addData(QualType QT) {
+addData(QT.getAsString());
+  }
+
+  // Utility functions for calling the addData method for each parent class
+  // of a given Stmt class.
+  #define STMT(CLASS, PARENT)  \
+  void callAddData##CLASS(CLASS const *S) {\
+callAddData##PARENT(S);\
+addData##CLASS(S); \
+  }
+  #include "clang/AST/StmtNodes.inc"
+
+  // Above code won't define callAddDataStmt, so we have to manually define it.
+  void callAddDataStmt(Stmt const *S) {
+addDataStmt(S);
+  }
+
+  /// \brief Calls the appropriate callAddData method for the actual class of
+  ///the given Stmt.
+  void collectData(Stmt const *S) {
+switch(S->getStmtClass()) {
+  case Stmt::NoStmtClass:
+break;
+#define ABSTRACT_STMT(STMT)
+#define STMT(CLASS, PARENT)\
+  case Stmt::CLASS##Class: \
+callAddData##CLASS(cast(S)); break;
+#include "clang/AST/StmtNodes.inc"
+}
+  }
+
+  // Below are functions that collect the specific data for each Stmt subclass.
+  #define DEF_ADD_DATA(CLASS, CODE)\
+  void addData##CLASS(CLASS const *S) {\
+CODE;  \
+  }
+
+  DEF_ADD_DATA(Stmt, { addData(S->getStmtClass()); })
+  DEF_ADD_DATA(Expr, { addData(S->getType

Re: [PATCH] D22542: [CodeGen] Fix a crash on valid when constant folding 'default:' statement in switch

2016-07-21 Thread Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL276350: [CodeGen] Fix a crash when constant folding switch 
statement (authored by epilk).

Changed prior to commit:
  https://reviews.llvm.org/D22542?vs=64569&id=64981#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D22542

Files:
  cfe/trunk/lib/CodeGen/CGStmt.cpp
  cfe/trunk/test/CodeGenCXX/switch-case-folding-2.cpp

Index: cfe/trunk/test/CodeGenCXX/switch-case-folding-2.cpp
===
--- cfe/trunk/test/CodeGenCXX/switch-case-folding-2.cpp
+++ cfe/trunk/test/CodeGenCXX/switch-case-folding-2.cpp
@@ -18,4 +18,13 @@
  return test(5);
 }
 
+void other_test() {
+  switch(0) {
+  case 0:
+do {
+default:;
+} while(0);
+  }
+}
+
 // CHECK: call i32 (i8*, ...) @_Z6printfPKcz
Index: cfe/trunk/lib/CodeGen/CGStmt.cpp
===
--- cfe/trunk/lib/CodeGen/CGStmt.cpp
+++ cfe/trunk/lib/CodeGen/CGStmt.cpp
@@ -1264,6 +1264,14 @@
 }
 
 void CodeGenFunction::EmitDefaultStmt(const DefaultStmt &S) {
+  // If there is no enclosing switch instance that we're aware of, then this
+  // default statement can be elided. This situation only happens when we've
+  // constant-folded the switch.
+  if (!SwitchInsn) {
+EmitStmt(S.getSubStmt());
+return;
+  }
+
   llvm::BasicBlock *DefaultBlock = SwitchInsn->getDefaultDest();
   assert(DefaultBlock->empty() &&
  "EmitDefaultStmt: Default block already defined?");


Index: cfe/trunk/test/CodeGenCXX/switch-case-folding-2.cpp
===
--- cfe/trunk/test/CodeGenCXX/switch-case-folding-2.cpp
+++ cfe/trunk/test/CodeGenCXX/switch-case-folding-2.cpp
@@ -18,4 +18,13 @@
  return test(5);
 }
 
+void other_test() {
+  switch(0) {
+  case 0:
+do {
+default:;
+} while(0);
+  }
+}
+
 // CHECK: call i32 (i8*, ...) @_Z6printfPKcz
Index: cfe/trunk/lib/CodeGen/CGStmt.cpp
===
--- cfe/trunk/lib/CodeGen/CGStmt.cpp
+++ cfe/trunk/lib/CodeGen/CGStmt.cpp
@@ -1264,6 +1264,14 @@
 }
 
 void CodeGenFunction::EmitDefaultStmt(const DefaultStmt &S) {
+  // If there is no enclosing switch instance that we're aware of, then this
+  // default statement can be elided. This situation only happens when we've
+  // constant-folded the switch.
+  if (!SwitchInsn) {
+EmitStmt(S.getSubStmt());
+return;
+  }
+
   llvm::BasicBlock *DefaultBlock = SwitchInsn->getDefaultDest();
   assert(DefaultBlock->empty() &&
  "EmitDefaultStmt: Default block already defined?");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r276350 - [CodeGen] Fix a crash when constant folding switch statement

2016-07-21 Thread Erik Pilkington via cfe-commits

Author: epilk
Date: Thu Jul 21 17:31:40 2016
New Revision: 276350

URL: http://llvm.org/viewvc/llvm-project?rev=276350&view=rev
Log:
[CodeGen] Fix a crash when constant folding switch statement

Differential revision: https://reviews.llvm.org/D22542

Modified:
cfe/trunk/lib/CodeGen/CGStmt.cpp
cfe/trunk/test/CodeGenCXX/switch-case-folding-2.cpp

Modified: cfe/trunk/lib/CodeGen/CGStmt.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmt.cpp?rev=276350&r1=276349&r2=276350&view=diff
==
--- cfe/trunk/lib/CodeGen/CGStmt.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGStmt.cpp Thu Jul 21 17:31:40 2016
@@ -1264,6 +1264,14 @@ void CodeGenFunction::EmitCaseStmt(const
 }
 
 void CodeGenFunction::EmitDefaultStmt(const DefaultStmt &S) {
+  // If there is no enclosing switch instance that we're aware of, then this
+  // default statement can be elided. This situation only happens when we've
+  // constant-folded the switch.
+  if (!SwitchInsn) {
+EmitStmt(S.getSubStmt());
+return;
+  }
+
   llvm::BasicBlock *DefaultBlock = SwitchInsn->getDefaultDest();
   assert(DefaultBlock->empty() &&
  "EmitDefaultStmt: Default block already defined?");

Modified: cfe/trunk/test/CodeGenCXX/switch-case-folding-2.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/switch-case-folding-2.cpp?rev=276350&r1=276349&r2=276350&view=diff
==
--- cfe/trunk/test/CodeGenCXX/switch-case-folding-2.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/switch-case-folding-2.cpp Thu Jul 21 17:31:40 2016
@@ -18,4 +18,13 @@ int main(void) {
  return test(5);
 }
 
+void other_test() {
+  switch(0) {
+  case 0:
+do {
+default:;
+} while(0);
+  }
+}
+
 // CHECK: call i32 (i8*, ...) @_Z6printfPKcz


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

Re: [PATCH] D22208: clang-tidy] Fixes to modernize-use-emplace

2016-07-21 Thread Piotr Padlewski via cfe-commits

Prazek added inline comments.


Comment at: clang-tidy/modernize/UseEmplaceCheck.h:36-37
@@ -32,1 +35,4 @@
+private:
+  std::vector ContainersWithPushBack;
+  std::vector SmartPointers;
 };

aaron.ballman wrote:
> Why not use a SmallVector for these instead of a std::vector? Then you don't 
> need to typecast in registerMatchers().
> 
> If it's because of parseStringList(), I think that you can work around it's 
> tight coupling using llvm::iterator_range(parseStringList()) to perform the 
> initialization in the ctor initializer list.
Unfortunatelly it doesn't want to work. I tried  
llvm::iterator_range and  
llvm::iterator_range::iterator> and it doesn't 
compile.


Repository:
  rL LLVM

https://reviews.llvm.org/D22208



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

RE: r276102 - [X86][SSE] Reimplement SSE fp2si conversion intrinsics instead of using generic IR

2016-07-21 Thread Robinson, Paul via cfe-commits



> -Original Message-
> From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of
> Simon Pilgrim via cfe-commits
> Sent: Wednesday, July 20, 2016 3:18 AM
> To: cfe-commits@lists.llvm.org
> Subject: r276102 - [X86][SSE] Reimplement SSE fp2si conversion intrinsics
> instead of using generic IR
> 
> Author: rksimon
> Date: Wed Jul 20 05:18:01 2016
> New Revision: 276102
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=276102&view=rev
> Log:
> [X86][SSE] Reimplement SSE fp2si conversion intrinsics instead of using
> generic IR
> 
> D20859 and D20860 attempted to replace the SSE (V)CVTTPS2DQ and VCVTTPD2DQ
> truncating conversions with generic IR instead.
> 
> It turns out that the behaviour of these intrinsics is different enough
> from generic IR that this will cause problems, INF/NAN/out of range values
> are guaranteed to result in a 0x8000 value - which plays havoc with
> constant folding which converts them to either zero or UNDEF. This is also
> an issue with the scalar implementations (which were already generic IR
> and what I was trying to match).

Are the problems enough that this should be merged to the 3.9 release branch?
--paulr

> 
> This patch changes both scalar and packed versions back to using x86-
> specific builtins.
> 
> It also deals with the other scalar conversion cases that are runtime
> rounding mode dependent and can have similar issues with constant folding.
> 
> Differential Revision: https://reviews.llvm.org/D22105
> 
> Modified:
> cfe/trunk/include/clang/Basic/BuiltinsX86.def
> cfe/trunk/lib/Headers/avxintrin.h
> cfe/trunk/lib/Headers/emmintrin.h
> cfe/trunk/lib/Headers/xmmintrin.h
> cfe/trunk/test/CodeGen/avx-builtins.c
> cfe/trunk/test/CodeGen/builtins-x86.c
> cfe/trunk/test/CodeGen/sse-builtins.c
> cfe/trunk/test/CodeGen/sse2-builtins.c
> 
> Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
> URL: http://llvm.org/viewvc/llvm-
> project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=276102&r1=276101
> &r2=276102&view=diff
> ==
> 
> --- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
> +++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Wed Jul 20 05:18:01 2016
> @@ -303,7 +303,9 @@ TARGET_BUILTIN(__builtin_ia32_pabsd128,
>  TARGET_BUILTIN(__builtin_ia32_ldmxcsr, "vUi", "", "sse")
>  TARGET_BUILTIN(__builtin_ia32_stmxcsr, "Ui", "", "sse")
>  TARGET_BUILTIN(__builtin_ia32_cvtss2si, "iV4f", "", "sse")
> +TARGET_BUILTIN(__builtin_ia32_cvttss2si, "iV4f", "", "sse")
>  TARGET_BUILTIN(__builtin_ia32_cvtss2si64, "LLiV4f", "", "sse")
> +TARGET_BUILTIN(__builtin_ia32_cvttss2si64, "LLiV4f", "", "sse")
>  TARGET_BUILTIN(__builtin_ia32_storehps, "vV2i*V4f", "", "sse")
>  TARGET_BUILTIN(__builtin_ia32_storelps, "vV2i*V4f", "", "sse")
>  TARGET_BUILTIN(__builtin_ia32_movmskps, "iV4f", "", "sse")
> @@ -328,8 +330,12 @@ TARGET_BUILTIN(__builtin_ia32_cvtpd2dq,
>  TARGET_BUILTIN(__builtin_ia32_cvtpd2ps, "V4fV2d", "", "sse2")
>  TARGET_BUILTIN(__builtin_ia32_cvttpd2dq, "V4iV2d", "", "sse2")
>  TARGET_BUILTIN(__builtin_ia32_cvtsd2si, "iV2d", "", "sse2")
> +TARGET_BUILTIN(__builtin_ia32_cvttsd2si, "iV2d", "", "sse2")
>  TARGET_BUILTIN(__builtin_ia32_cvtsd2si64, "LLiV2d", "", "sse2")
> +TARGET_BUILTIN(__builtin_ia32_cvttsd2si64, "LLiV2d", "", "sse2")
> +TARGET_BUILTIN(__builtin_ia32_cvtsd2ss, "V4fV4fV2d", "", "sse2")
>  TARGET_BUILTIN(__builtin_ia32_cvtps2dq, "V4iV4f", "", "sse2")
> +TARGET_BUILTIN(__builtin_ia32_cvttps2dq, "V4iV4f", "", "sse2")
>  TARGET_BUILTIN(__builtin_ia32_clflush, "vvC*", "", "sse2")
>  TARGET_BUILTIN(__builtin_ia32_lfence, "v", "", "sse2")
>  TARGET_BUILTIN(__builtin_ia32_mfence, "v", "", "sse2")
> @@ -455,7 +461,9 @@ TARGET_BUILTIN(__builtin_ia32_cmpss, "V4
>  TARGET_BUILTIN(__builtin_ia32_cvtdq2ps256, "V8fV8i", "", "avx")
>  TARGET_BUILTIN(__builtin_ia32_cvtpd2ps256, "V4fV4d", "", "avx")
>  TARGET_BUILTIN(__builtin_ia32_cvtps2dq256, "V8iV8f", "", "avx")
> +TARGET_BUILTIN(__builtin_ia32_cvttpd2dq256, "V4iV4d", "", "avx")
>  TARGET_BUILTIN(__builtin_ia32_cvtpd2dq256, "V4iV4d", "", "avx")
> +TARGET_BUILTIN(__builtin_ia32_cvttps2dq256, "V8iV8f", "", "avx")
>  TARGET_BUILTIN(__builtin_ia32_vperm2f128_pd256, "V4dV4dV4dIc", "", "avx")
>  TARGET_BUILTIN(__builtin_ia32_vperm2f128_ps256, "V8fV8fV8fIc", "", "avx")
>  TARGET_BUILTIN(__builtin_ia32_vperm2f128_si256, "V8iV8iV8iIc", "", "avx")
> 
> Modified: cfe/trunk/lib/Headers/avxintrin.h
> URL: http://llvm.org/viewvc/llvm-
> project/cfe/trunk/lib/Headers/avxintrin.h?rev=276102&r1=276101&r2=276102&v
> iew=diff
> ==
> 
> --- cfe/trunk/lib/Headers/avxintrin.h (original)
> +++ cfe/trunk/lib/Headers/avxintrin.h Wed Jul 20 05:18:01 2016
> @@ -2117,7 +2117,7 @@ _mm256_cvtps_pd(__m128 __a)
>  static __inline __m128i __DEFAULT_FN_ATTRS
>  _mm256_cvttpd_epi32(__m256d __a)
>  {
> -  return (__m128i)__builtin_convertvec

[PATCH] D22654: [Clang-rename] Remove custom version, fix extra space in error message

2016-07-21 Thread Eugene Zelenko via cfe-commits

Eugene.Zelenko created this revision.
Eugene.Zelenko added reviewers: omtcyfz, vmiklos, klimek.
Eugene.Zelenko added a subscriber: cfe-commits.
Eugene.Zelenko set the repository for this revision to rL LLVM.

I also fixed some Include What You Use Warnings.

Repository:
  rL LLVM

https://reviews.llvm.org/D22654

Files:
  clang-rename/tool/ClangRename.cpp
  test/clang-rename/InvalidNewName.cpp

Index: test/clang-rename/InvalidNewName.cpp
===
--- test/clang-rename/InvalidNewName.cpp
+++ test/clang-rename/InvalidNewName.cpp
@@ -1,2 +1,2 @@
 // RUN: not clang-rename -new-name=class -offset=133 %s 2>&1 | FileCheck %s
-// CHECK: ERROR: new name is not a valid identifier in  C++17.
+// CHECK: ERROR: new name is not a valid identifier in C++17.
Index: clang-rename/tool/ClangRename.cpp
===
--- clang-rename/tool/ClangRename.cpp
+++ clang-rename/tool/ClangRename.cpp
@@ -15,28 +15,28 @@
 
 #include "../USRFindingAction.h"
 #include "../RenamingAction.h"
-#include "clang/AST/ASTConsumer.h"
-#include "clang/AST/ASTContext.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/FileManager.h"
+#include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/LangOptions.h"
-#include "clang/Basic/TargetInfo.h"
-#include "clang/Basic/TargetOptions.h"
-#include "clang/Frontend/CommandLineSourceLoc.h"
-#include "clang/Frontend/CompilerInstance.h"
-#include "clang/Frontend/FrontendAction.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.h"
 #include "clang/Frontend/TextDiagnosticPrinter.h"
-#include "clang/Lex/Lexer.h"
-#include "clang/Lex/Preprocessor.h"
-#include "clang/Parse/ParseAST.h"
-#include "clang/Parse/Parser.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Refactoring.h"
 #include "clang/Tooling/ReplacementsYaml.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
-#include "llvm/Support/Host.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Support/YAMLTraits.h"
+#include 
 #include 
+#include 
 
 using namespace llvm;
 
@@ -79,12 +79,6 @@
 cl::value_desc("filename"),
 cl::cat(ClangRenameCategory));
 
-#define CLANG_RENAME_VERSION "0.0.1"
-
-static void PrintVersion() {
-  outs() << "clang-rename version " << CLANG_RENAME_VERSION << '\n';
-}
-
 using namespace clang;
 
 const char RenameUsage[] = "A tool to rename symbols in C/C++ code.\n\
@@ -93,7 +87,6 @@
 Otherwise, the results are written to stdout.\n";
 
 int main(int argc, const char **argv) {
-  cl::SetVersionPrinter(PrintVersion);
   tooling::CommonOptionsParser OP(argc, argv, ClangRenameCategory, 
RenameUsage);
 
   // Check the arguments for correctness.
@@ -110,7 +103,7 @@
   IdentifierTable Table(Options);
   auto NewNameTokKind = Table.get(NewName).getTokenID();
   if (!tok::isAnyIdentifier(NewNameTokKind)) {
-errs() << "ERROR: new name is not a valid identifier in  C++17.\n\n";
+errs() << "ERROR: new name is not a valid identifier in C++17.\n\n";
 exit(1);
   }
 


Index: test/clang-rename/InvalidNewName.cpp
===
--- test/clang-rename/InvalidNewName.cpp
+++ test/clang-rename/InvalidNewName.cpp
@@ -1,2 +1,2 @@
 // RUN: not clang-rename -new-name=class -offset=133 %s 2>&1 | FileCheck %s
-// CHECK: ERROR: new name is not a valid identifier in  C++17.
+// CHECK: ERROR: new name is not a valid identifier in C++17.
Index: clang-rename/tool/ClangRename.cpp
===
--- clang-rename/tool/ClangRename.cpp
+++ clang-rename/tool/ClangRename.cpp
@@ -15,28 +15,28 @@
 
 #include "../USRFindingAction.h"
 #include "../RenamingAction.h"
-#include "clang/AST/ASTConsumer.h"
-#include "clang/AST/ASTContext.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/FileManager.h"
+#include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/LangOptions.h"
-#include "clang/Basic/TargetInfo.h"
-#include "clang/Basic/TargetOptions.h"
-#include "clang/Frontend/CommandLineSourceLoc.h"
-#include "clang/Frontend/CompilerInstance.h"
-#include "clang/Frontend/FrontendAction.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.h"
 #include "clang/Frontend/TextDiagnosticPrinter.h"
-#include "clang/Lex/Lexer.h"
-#include "clang/Lex/Preprocessor.h"
-#include "clang/Parse/ParseAST.h"
-#include "clang/Parse/Parser.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Refactoring.h"
 #include "clang/Tooling/ReplacementsYaml.h"
 #include "clang/Tooling/To

Re: [PATCH] D22587: [ASTContext] Fix part of DR224 for nontype template arguments

2016-07-21 Thread Matthias Gehre via cfe-commits

mgehre added inline comments.


Comment at: lib/AST/ASTContext.cpp:4456
@@ +4455,3 @@
+  // the template parameter and not an expression involving the template 
parameter.
+  auto *E = Arg.getAsExpr()->IgnoreImpCasts();
+  while(auto *DeclRef = dyn_cast_or_null(E)) {

aaron.ballman wrote:
> `const auto *` (and propagate it to other declarations)?
Unfortunately, the TemplateArgument constructor in

```
return TemplateArgument(DeclRef);
```

takes a Expr* (and not a const Expr*).


https://reviews.llvm.org/D22587



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

r276335 - Disable a flaky test on Windows that uses "echo >>"

2016-07-21 Thread Reid Kleckner via cfe-commits

Author: rnk
Date: Thu Jul 21 16:07:47 2016
New Revision: 276335

URL: http://llvm.org/viewvc/llvm-project?rev=276335&view=rev
Log:
Disable a flaky test on Windows that uses "echo >>"

Modified:
cfe/trunk/test/Modules/embed-files.cpp

Modified: cfe/trunk/test/Modules/embed-files.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/embed-files.cpp?rev=276335&r1=276334&r2=276335&view=diff
==
--- cfe/trunk/test/Modules/embed-files.cpp (original)
+++ cfe/trunk/test/Modules/embed-files.cpp Thu Jul 21 16:07:47 2016
@@ -10,6 +10,10 @@
 //
 // RUN: %clang_cc1 -fmodules -I%t -fmodules-embed-all-files %t/modulemap 
-fmodule-name=a -x c++ -emit-module -o %t/a.pcm
 // RUN: %clang_cc1 -fmodules -I%t -fmodules-embed-all-files %t/modulemap 
-fmodule-name=b -x c++ -emit-module -o %t/b.pcm
+// FIXME: This test is flaky on Windows because attempting to delete a file
+// after writing it just doesn't seem to work well, at least not in the lit
+// shell.
+// REQUIRES: shell
 // RUN: rm %t/x.h
 // RUN: %clang_cc1 -fmodules -I%t -fmodule-map-file=%t/modulemap 
-fmodule-file=%t/a.pcm -fmodule-file=%t/b.pcm %s -verify
 #include "a.h"
@@ -19,3 +23,4 @@ char t; // expected-error {{different ty
 #include "b.h"
 char t; // expected-error {{different type}}
 // expected-note@t.h:1 {{here}}
+


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

Re: [PATCH] D21459: Implement http://wg21.link/P0254R1: "Integrating std::string_view and std::string"

2016-07-21 Thread Kim Gräsman via cfe-commits

kimgr added inline comments.


Comment at: include/string_view:216
@@ +215,3 @@
+   basic_string_view(const _CharT* __s)
+   : __data(__s), __size(_Traits::length(__s)) {}
+

mclow.lists wrote:
> mclow.lists wrote:
> > kimgr wrote:
> > > mclow.lists wrote:
> > > > kimgr wrote:
> > > > > I'm working from the paper at 
> > > > > https://isocpp.org/files/papers/N3762.html, and I find it a little 
> > > > > sketchy on the policy for nullptrs.
> > > > > 
> > > > > Since the ctor above accepts nullptr as long as the length is zero, 
> > > > > would it make sense to do that here too? That is, only call 
> > > > > _Traits::length for non-nullptr __s args?
> > > > Reading from N4600: Requires: `[str, str + traits::length(str))` is a 
> > > > valid range.
> > > > 
> > > > So, no - passing `nullptr` here is undefined.
> > > > 
> > > OK, that feels more principled to me, anyway.
> > > 
> > > But the `(const char*, size_t)` constructor has the same requirement and 
> > > it *does* accept `nullptr`, provided the length is zero. I saw you had to 
> > > get rid of the assertion, but I feel like the constructor should probably 
> > > not silently accept `nullptr` for zero-sized strings. Or do you know if 
> > > that's intentional? Many StringRef/StringPiece implementations seem to 
> > > have the same behavior.
> > It is absolutely intentional. `[nullptr, nullptr+0)` is a perfectly fine 
> > half-open range. It contains no characters.  
> > 
> > However, the ctor that takes just a pointer has to calculate the length .. 
> > by dereferencing the pointer.
> > 
> > I had to get rid of the assertion because one of the bots (gcc 4.9) has a 
> > bug about constexpr ctors in c++11 mode.  Even though the assertion was 
> > `#ifdef`ed on C++ > 11, the compiler complained about it.  I'll be putting 
> > the assertion back as soon as I can figure out how to keep gcc from 
> > complaining.
> This was discussed (at length) in LEWG during the design of `string_view`.
Ah, got it, thanks! It opens up for cases where `data()` for an empty 
`string_view` can sometimes return `""` and sometimes `nullptr`, but I guess 
that problem extends beyond `string_view`'s responsibilities.

Thanks for the thorough explanation.


https://reviews.llvm.org/D21459



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

Re: [PATCH] D22542: [CodeGen] Fix a crash on valid when constant folding 'default:' statement in switch

2016-07-21 Thread Manman Ren via cfe-commits

manmanren accepted this revision.
manmanren added a comment.
This revision is now accepted and ready to land.

LGTM.

Manman


https://reviews.llvm.org/D22542



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

Re: [libunwind] r276215 - [libunwind] Properly align _Unwind_Exception.

2016-07-21 Thread Hans Wennborg via cfe-commits

I'm not sure we have an owner for libunwind, but things sounded
positive on the code review, so go ahead and merge.

Thanks,
Hans

On Wed, Jul 20, 2016 at 8:06 PM, Eric Fiselier  wrote:
> @Hans This should be merged into 3.9.
>
> Can a code owner give this the thumbs up?
>
> /Eric
>
> On Wed, Jul 20, 2016 at 5:56 PM, Eric Fiselier via cfe-commits
>  wrote:
>>
>> Author: ericwf
>> Date: Wed Jul 20 18:56:42 2016
>> New Revision: 276215
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=276215&view=rev
>> Log:
>> [libunwind] Properly align _Unwind_Exception.
>>
>> Summary: _Unwind_Exception is required to be double word aligned.
>> Currently the struct is under aligned.
>>
>> Reviewers: mclow.lists, compnerd, kledzik, emaste
>>
>> Subscribers: emaste, cfe-commits
>>
>> Differential Revision: https://reviews.llvm.org/D22543
>>
>> Added:
>> libunwind/trunk/test/alignment.pass.cpp
>> Modified:
>> libunwind/trunk/include/unwind.h
>>
>> Modified: libunwind/trunk/include/unwind.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/libunwind/trunk/include/unwind.h?rev=276215&r1=276214&r2=276215&view=diff
>>
>> ==
>> --- libunwind/trunk/include/unwind.h (original)
>> +++ libunwind/trunk/include/unwind.h Wed Jul 20 18:56:42 2016
>> @@ -122,13 +122,16 @@ struct _Unwind_Exception {
>>uintptr_t private_1; // non-zero means forced unwind
>>uintptr_t private_2; // holds sp that phase1 found for phase2 to use
>>  #ifndef __LP64__
>> -  // The gcc implementation of _Unwind_Exception used attribute mode on
>> the
>> -  // above fields which had the side effect of causing this whole struct
>> to
>> +  // The implementation of _Unwind_Exception uses an attribute mode on
>> the
>> +  // above fields which has the side effect of causing this whole struct
>> to
>>// round up to 32 bytes in size. To be more explicit, we add pad fields
>>// added for binary compatibility.
>>uint32_t reserved[3];
>>  #endif
>> -};
>> +  // The Itanium ABI requires that _Unwind_Exception objects are
>> "double-word
>> +  // aligned".  GCC has interpreted this to mean "use the maximum useful
>> +  // alignment for the target"; so do we.
>> +} __attribute__((__aligned__));
>>
>>  typedef _Unwind_Reason_Code (*_Unwind_Stop_Fn)
>>  (int version,
>>
>> Added: libunwind/trunk/test/alignment.pass.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/libunwind/trunk/test/alignment.pass.cpp?rev=276215&view=auto
>>
>> ==
>> --- libunwind/trunk/test/alignment.pass.cpp (added)
>> +++ libunwind/trunk/test/alignment.pass.cpp Wed Jul 20 18:56:42 2016
>> @@ -0,0 +1,21 @@
>> +// -*- C++ -*-
>>
>> +//===--===//
>> +//
>> +// The LLVM Compiler Infrastructure
>> +//
>> +// This file is dual licensed under the MIT and the University of
>> Illinois Open
>> +// Source Licenses. See LICENSE.TXT for details.
>> +//
>>
>> +//===--===//
>> +
>> +// The Itanium ABI requires that _Unwind_Exception objects are
>> "double-word
>> +// aligned".
>> +
>> +#include 
>> +
>> +struct MaxAligned {} __attribute__((aligned));
>> +static_assert(alignof(_Unwind_Exception) == alignof(MaxAligned), "");
>> +
>> +int main()
>> +{
>> +}
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [libunwind] r276128 - libunwind: limit stack usage in unwind cursor

2016-07-21 Thread Hans Wennborg via cfe-commits

On Wed, Jul 20, 2016 at 9:00 PM, Ed Maste  wrote:
> On 20 July 2016 at 15:19, Ed Maste via cfe-commits
>  wrote:
>> Author: emaste
>> Date: Wed Jul 20 10:19:09 2016
>> New Revision: 276128
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=276128&view=rev
>> Log:
>> libunwind: limit stack usage in unwind cursor
>>
>> Obtained from FreeBSD SVN r302475
>>
>> Differential Revision:  https://reviews.llvm.org/D22570
>
> Hans, OK to merge this to 3.9?

Yes, go ahead.

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

Re: [PATCH] D21814: clang-rename: split existing options into two new subcommands

2016-07-21 Thread Miklos Vajna via cfe-commits

vmiklos updated this revision to Diff 64941.

https://reviews.llvm.org/D21814

Files:
  clang-rename/RenamingAction.cpp
  clang-rename/RenamingAction.h
  clang-rename/tool/ClangRename.cpp
  clang-rename/tool/clang-rename.py
  docs/clang-rename.rst
  test/clang-rename/ClassFindByName.cpp
  test/clang-rename/ClassReplacements.cpp
  test/clang-rename/ClassSimpleRenaming.cpp
  test/clang-rename/ClassTestMulti.cpp
  test/clang-rename/ClassTestMultiByName.cpp
  test/clang-rename/ConstCastExpr.cpp
  test/clang-rename/ConstructExpr.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/Field.cpp
  test/clang-rename/FunctionMacro.cpp
  test/clang-rename/InvalidNewName.cpp
  test/clang-rename/MemberExprMacro.cpp
  test/clang-rename/Namespace.cpp
  test/clang-rename/NoNewName.cpp
  test/clang-rename/ReinterpretCastExpr.cpp
  test/clang-rename/StaticCastExpr.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,5 +1,5 @@
 // RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=208 -new-name=Z %t.cpp -i --
+// RUN: clang-rename rename-at -offset=218 -new-name=Z %t.cpp -i --
 // RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
 
 #define Y X // CHECK: #define Y Z
Index: test/clang-rename/Variable.cpp
===
--- test/clang-rename/Variable.cpp
+++ test/clang-rename/Variable.cpp
@@ -1,5 +1,5 @@
 // RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=148 -new-name=Bar %t.cpp -i --
+// RUN: clang-rename rename-at -offset=158 -new-name=Bar %t.cpp -i --
 // RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
 
 namespace A {
Index: test/clang-rename/StaticCastExpr.cpp
===
--- test/clang-rename/StaticCastExpr.cpp
+++ test/clang-rename/StaticCastExpr.cpp
@@ -1,5 +1,5 @@
 // RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=152 -new-name=Bar %t.cpp -i --
+// RUN: clang-rename rename-at -offset=162 -new-name=Bar %t.cpp -i --
 // RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
 
 class Baz {
Index: test/clang-rename/ReinterpretCastExpr.cpp
===
--- test/clang-rename/ReinterpretCastExpr.cpp
+++ test/clang-rename/ReinterpretCastExpr.cpp
@@ -1,5 +1,5 @@
 // RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=133 -new-name=X %t.cpp -i --
+// RUN: clang-rename rename-at -offset=143 -new-name=X %t.cpp -i --
 // RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
 class Cla {
 public:
Index: test/clang-rename/NoNewName.cpp
===
--- test/clang-rename/NoNewName.cpp
+++ test/clang-rename/NoNewName.cpp
@@ -1,4 +1,4 @@
 // Check for an error while -new-name argument has not been passed to
 // clang-rename.
-// RUN: not clang-rename -offset=133 %s 2>&1 | FileCheck %s
-// CHECK: ERROR: no new name provided.
+// RUN: not clang-rename rename-at -offset=133 %s 2>&1 | FileCheck %s
+// CHECK: clang-rename rename-at: for the -new-name option: must be specified
Index: test/clang-rename/Namespace.cpp
===
--- test/clang-rename/Namespace.cpp
+++ test/clang-rename/Namespace.cpp
@@ -1,5 +1,5 @@
 // RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=143 -new-name=llvm %t.cpp -i --
+// RUN: clang-rename rename-at -offset=153 -new-name=llvm %t.cpp -i --
 // RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
 
 namespace foo { // CHECK: namespace llvm {
Index: test/clang-rename/MemberExprMacro.cpp
===
--- test/clang-rename/MemberExprMacro.cpp
+++ test/clang-rename/MemberExprMacro.cpp
@@ -1,5 +1,5 @@
 // RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=156 -new-name=Bar %t.cpp -i --
+// RUN: clang-rename rename-at -offset=166 -new-name=Bar %t.cpp -i --
 // RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
 
 class Baz {
Index: test/clang-rename/InvalidNewName.cpp
===
--- test/clang-rename/InvalidNewName.cpp
+++ test/clang-rename/InvalidNewName.cpp
@@ -1,2 +1,2 @@
-// RUN: not clang-rename -new-name=class -offset=133 %s 2>&1 | FileCheck %s
+// RUN: not clang-rename rename-at -new-name=class -offset=133 %s 2>&1 | FileCheck %s
 // CHECK: ERROR: new name is not a valid identifier in  C++17.
Index: test/clang-rename/FunctionMacro.cpp
===
--- test/clang-rename/FunctionMacro.cpp
+++ test/clang-rename/FunctionMacro.cpp
@@ -1,5 +1,5 @@
 // RUN: cat %s > %t.cp

Re: [PATCH] D21814: clang-rename: split existing options into two new subcommands

2016-07-21 Thread Miklos Vajna via cfe-commits

vmiklos added a comment.

Rebased on top of r276282 and resolved conflicts.


https://reviews.llvm.org/D21814



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

[PATCH] D22642: CodeGen: Clean up implementation of vtable initializer builder. NFC.

2016-07-21 Thread Peter Collingbourne via cfe-commits

pcc created this revision.
pcc added a reviewer: rsmith.
pcc added a subscriber: cfe-commits.

- Simplify signature of CreateVTableInitializer function.
- Move vtable component builder to a separate function.
- Remove unnecessary accessors from VTableLayout class.

This is in preparation for a future change that will alter the type of the
vtable initializer.

https://reviews.llvm.org/D22642

Files:
  include/clang/AST/VTableBuilder.h
  lib/CodeGen/CGVTables.cpp
  lib/CodeGen/CGVTables.h
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/CodeGen/MicrosoftCXXABI.cpp

Index: lib/CodeGen/MicrosoftCXXABI.cpp
===
--- lib/CodeGen/MicrosoftCXXABI.cpp
+++ lib/CodeGen/MicrosoftCXXABI.cpp
@@ -1565,10 +1565,7 @@
[](const VTableComponent &VTC) { return VTC.isRTTIKind(); }))
   RTTI = getMSCompleteObjectLocator(RD, Info);
 
-llvm::Constant *Init = CGVT.CreateVTableInitializer(
-RD, VTLayout.vtable_component_begin(),
-VTLayout.getNumVTableComponents(), VTLayout.vtable_thunk_begin(),
-VTLayout.getNumVTableThunks(), RTTI);
+llvm::Constant *Init = CGVT.CreateVTableInitializer(VTLayout, RTTI);
 
 VTable->setInitializer(Init);
 
@@ -1691,7 +1688,8 @@
 
   uint64_t NumVTableSlots =
   VTContext.getVFTableLayout(RD, VFPtr->FullOffsetInMDC)
-  .getNumVTableComponents();
+  .vtable_components()
+  .size();
   llvm::GlobalValue::LinkageTypes VTableLinkage =
   VTableAliasIsRequred ? llvm::GlobalValue::PrivateLinkage : VFTableLinkage;
 
Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -1458,9 +1458,7 @@
   CGM.GetAddrOfRTTIDescriptor(CGM.getContext().getTagDeclType(RD));
 
   // Create and set the initializer.
-  llvm::Constant *Init = CGVT.CreateVTableInitializer(
-  RD, VTLayout.vtable_component_begin(), VTLayout.getNumVTableComponents(),
-  VTLayout.vtable_thunk_begin(), VTLayout.getNumVTableThunks(), RTTI);
+  llvm::Constant *Init = CGVT.CreateVTableInitializer(VTLayout, RTTI);
   VTable->setInitializer(Init);
 
   // Set the correct linkage.
@@ -1571,7 +1569,7 @@
 
   ItaniumVTableContext &VTContext = CGM.getItaniumVTableContext();
   llvm::ArrayType *ArrayType = llvm::ArrayType::get(
-  CGM.Int8PtrTy, VTContext.getVTableLayout(RD).getNumVTableComponents());
+  CGM.Int8PtrTy, VTContext.getVTableLayout(RD).vtable_components().size());
 
   VTable = CGM.CreateOrReplaceCXXRuntimeVariable(
   Name, ArrayType, llvm::GlobalValue::ExternalLinkage);
Index: lib/CodeGen/CGVTables.h
===
--- lib/CodeGen/CGVTables.h
+++ lib/CodeGen/CGVTables.h
@@ -49,22 +49,27 @@
   /// indices.
   SecondaryVirtualPointerIndicesMapTy SecondaryVirtualPointerIndices;
 
+  llvm::Constant *PureVirtualFn = nullptr, *DeletedVirtualFn = nullptr;
+
   /// emitThunk - Emit a single thunk.
   void emitThunk(GlobalDecl GD, const ThunkInfo &Thunk, bool ForVTable);
 
   /// maybeEmitThunkForVTable - Emit the given thunk for the vtable if needed by
   /// the ABI.
   void maybeEmitThunkForVTable(GlobalDecl GD, const ThunkInfo &Thunk);
 
+  llvm::Constant *CreateVTableComponent(unsigned Idx,
+const VTableLayout &VTLayout,
+llvm::Constant *RTTI,
+unsigned &NextVTableThunkIndex);
+
 public:
   /// CreateVTableInitializer - Create a vtable initializer for the given record
   /// decl.
   /// \param Components - The vtable components; this is really an array of
   /// VTableComponents.
-  llvm::Constant *CreateVTableInitializer(
-  const CXXRecordDecl *RD, const VTableComponent *Components,
-  unsigned NumComponents, const VTableLayout::VTableThunkTy *VTableThunks,
-  unsigned NumVTableThunks, llvm::Constant *RTTI);
+  llvm::Constant *CreateVTableInitializer(const VTableLayout &VTLayout,
+  llvm::Constant *RTTI);
 
   CodeGenVTables(CodeGenModule &CGM);
 
Index: lib/CodeGen/CGVTables.cpp
===
--- lib/CodeGen/CGVTables.cpp
+++ lib/CodeGen/CGVTables.cpp
@@ -517,139 +517,124 @@
 emitThunk(GD, Thunk, /*ForVTable=*/false);
 }
 
-llvm::Constant *CodeGenVTables::CreateVTableInitializer(
-const CXXRecordDecl *RD, const VTableComponent *Components,
-unsigned NumComponents, const VTableLayout::VTableThunkTy *VTableThunks,
-unsigned NumVTableThunks, llvm::Constant *RTTI) {
-  SmallVector Inits;
-
+llvm::Constant *CodeGenVTables::CreateVTableComponent(
+unsigned Idx, const VTableLayout &VTLayout, llvm::Constant *RTTI,
+unsigned &NextVTableThunkIndex) {
+  VTableComponent Component = VTLayout.vtable_components()[Idx];
   llvm::Type *Int8PtrTy = CGM.Int8PtrTy;
-  
-  llvm::Type *PtrDiffT

Re: [PATCH] D22010: Add more gcc compatibility names to clang's cpuid.h

2016-07-21 Thread Dimitry Andric via cfe-commits

dim added inline comments.


Comment at: lib/Headers/cpuid.h:114
@@ -109,2 +113,3 @@
 #define bit_AVX 0x1000
+#define bit_F16C0x2000
 #define bit_RDRND   0x4000

bruno wrote:
> Isn't this one also meant for gcc compat?
Well, we didn't have any clang-specific define, so might as well choose the 
same name that gcc has used?


https://reviews.llvm.org/D22010



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

Re: [PATCH] D12512: [libcxxabi] Manually align pointers in __cxa_allocate_exception - Fixes PR24604

2016-07-21 Thread Ed Maste via cfe-commits

emaste added a comment.

Note that the addition of the referenceCount at the beginning of 
`__cxa_exception` for 64-bit builds causes the `_Unwind_Exception unwindHeader` 
to become misaligned. libstdc++ had this same issue 
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=38732, now fixed) and libcxxrt 
does as well (https://github.com/pathscale/libcxxrt/issues/38).


https://reviews.llvm.org/D12512



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

Re: [PATCH] D22636: Module: retry building modules that were just compiled by the same instance and are are out of date

2016-07-21 Thread Manman Ren via cfe-commits

manmanren added a comment.

In https://reviews.llvm.org/D22636#491679, @benlangmuir wrote:

> > B.pcm becomes out of date when we try to load module "A", because another 
> > instance overwrites "B.pcm"
>
>
> How can this happen?  We intentionally keep the file descriptors of modules 
> open.  If you just built A, then you will have B.pcm still open.  When you 
> read A, you will use the same B that you used when you built A even if the 
> file on disk has been replaced (and we use rename to replace the file, so the 
> existing one is never modified).

Can you point me to the source codes where we use rename to replace the file? I 
am curious on how this all works out.

What I described is a scenario I thought possible that can cause "out-of-date" 
error:
module "B" is out of date and needs to be rebuilt
note: imported by module "A"

The only invocation of ReadAST that reads a module file and can't handle 
out-of-date modules, is the path where we just built module "A" and tries to 
load module "A" (here ModuleLoadCapabilities will be ARR_Missing).

I am still working with the project owner to collect more debugging messages.

If this is not a possible scenario, do you have any suggestion on what can 
cause this error?

Cheers,
Manman

https://reviews.llvm.org/D22636

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

Re: [PATCH] D21459: Implement http://wg21.link/P0254R1: "Integrating std::string_view and std::string"

2016-07-21 Thread Marshall Clow via cfe-commits

mclow.lists added inline comments.


Comment at: include/string_view:216
@@ +215,3 @@
+   basic_string_view(const _CharT* __s)
+   : __data(__s), __size(_Traits::length(__s)) {}
+

mclow.lists wrote:
> kimgr wrote:
> > mclow.lists wrote:
> > > kimgr wrote:
> > > > I'm working from the paper at 
> > > > https://isocpp.org/files/papers/N3762.html, and I find it a little 
> > > > sketchy on the policy for nullptrs.
> > > > 
> > > > Since the ctor above accepts nullptr as long as the length is zero, 
> > > > would it make sense to do that here too? That is, only call 
> > > > _Traits::length for non-nullptr __s args?
> > > Reading from N4600: Requires: `[str, str + traits::length(str))` is a 
> > > valid range.
> > > 
> > > So, no - passing `nullptr` here is undefined.
> > > 
> > OK, that feels more principled to me, anyway.
> > 
> > But the `(const char*, size_t)` constructor has the same requirement and it 
> > *does* accept `nullptr`, provided the length is zero. I saw you had to get 
> > rid of the assertion, but I feel like the constructor should probably not 
> > silently accept `nullptr` for zero-sized strings. Or do you know if that's 
> > intentional? Many StringRef/StringPiece implementations seem to have the 
> > same behavior.
> It is absolutely intentional. `[nullptr, nullptr+0)` is a perfectly fine 
> half-open range. It contains no characters.  
> 
> However, the ctor that takes just a pointer has to calculate the length .. by 
> dereferencing the pointer.
> 
> I had to get rid of the assertion because one of the bots (gcc 4.9) has a bug 
> about constexpr ctors in c++11 mode.  Even though the assertion was 
> `#ifdef`ed on C++ > 11, the compiler complained about it.  I'll be putting 
> the assertion back as soon as I can figure out how to keep gcc from 
> complaining.
This was discussed (at length) in LEWG during the design of `string_view`.


https://reviews.llvm.org/D21459



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

Re: [PATCH] D21459: Implement http://wg21.link/P0254R1: "Integrating std::string_view and std::string"

2016-07-21 Thread Marshall Clow via cfe-commits

mclow.lists added inline comments.


Comment at: include/string_view:216
@@ +215,3 @@
+   basic_string_view(const _CharT* __s)
+   : __data(__s), __size(_Traits::length(__s)) {}
+

kimgr wrote:
> mclow.lists wrote:
> > kimgr wrote:
> > > I'm working from the paper at https://isocpp.org/files/papers/N3762.html, 
> > > and I find it a little sketchy on the policy for nullptrs.
> > > 
> > > Since the ctor above accepts nullptr as long as the length is zero, would 
> > > it make sense to do that here too? That is, only call _Traits::length for 
> > > non-nullptr __s args?
> > Reading from N4600: Requires: `[str, str + traits::length(str))` is a valid 
> > range.
> > 
> > So, no - passing `nullptr` here is undefined.
> > 
> OK, that feels more principled to me, anyway.
> 
> But the `(const char*, size_t)` constructor has the same requirement and it 
> *does* accept `nullptr`, provided the length is zero. I saw you had to get 
> rid of the assertion, but I feel like the constructor should probably not 
> silently accept `nullptr` for zero-sized strings. Or do you know if that's 
> intentional? Many StringRef/StringPiece implementations seem to have the same 
> behavior.
It is absolutely intentional. `[nullptr, nullptr+0)` is a perfectly fine 
half-open range. It contains no characters.  

However, the ctor that takes just a pointer has to calculate the length .. by 
dereferencing the pointer.

I had to get rid of the assertion because one of the bots (gcc 4.9) has a bug 
about constexpr ctors in c++11 mode.  Even though the assertion was `#ifdef`ed 
on C++ > 11, the compiler complained about it.  I'll be putting the assertion 
back as soon as I can figure out how to keep gcc from complaining.


https://reviews.llvm.org/D21459



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

r276317 - Reroll "Include unreferenced nested types in member list only for CodeView"

2016-07-21 Thread Adrian McCarthy via cfe-commits

Author: amccarth
Date: Thu Jul 21 13:43:20 2016
New Revision: 276317

URL: http://llvm.org/viewvc/llvm-project?rev=276317&view=rev
Log:
Reroll "Include unreferenced nested types in member list only for CodeView"

Another attempt at r276271, hopefully without breaking ModuleDebugInfo test.

Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.h
cfe/trunk/test/CodeGenCXX/debug-info-dup-fwd-decl.cpp
cfe/trunk/test/CodeGenCXX/debug-info-ms-abi.cpp
cfe/trunk/test/Modules/ModuleDebugInfo.cpp

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=276317&r1=276316&r2=276317&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Thu Jul 21 13:43:20 2016
@@ -1090,6 +1090,14 @@ void CGDebugInfo::CollectRecordNormalFie
   elements.push_back(FieldType);
 }
 
+void CGDebugInfo::CollectRecordNestedRecord(
+const RecordDecl *RD, SmallVectorImpl &elements) {
+  QualType Ty = CGM.getContext().getTypeDeclType(RD);
+  SourceLocation Loc = RD->getLocation();
+  llvm::DIType *nestedType = getOrCreateType(Ty, getOrCreateFile(Loc));
+  elements.push_back(nestedType);
+}
+
 void CGDebugInfo::CollectRecordFields(
 const RecordDecl *record, llvm::DIFile *tunit,
 SmallVectorImpl &elements,
@@ -1101,6 +1109,10 @@ void CGDebugInfo::CollectRecordFields(
   else {
 const ASTRecordLayout &layout = 
CGM.getContext().getASTRecordLayout(record);
 
+// Debug info for nested records is included in the member list only for
+// CodeView.
+bool IncludeNestedRecords = CGM.getCodeGenOpts().EmitCodeView;
+
 // Field number for non-static fields.
 unsigned fieldNo = 0;
 
@@ -1126,7 +1138,10 @@ void CGDebugInfo::CollectRecordFields(
 
 // Bump field number for next field.
 ++fieldNo;
-  }
+  } else if (const auto *nestedRec = dyn_cast(I))
+if (IncludeNestedRecords && !nestedRec->isImplicit() &&
+nestedRec->getDeclContext() == record)
+  CollectRecordNestedRecord(nestedRec, elements);
   }
 }
 
@@ -3620,8 +3635,8 @@ void CGDebugInfo::EmitUsingDirective(con
   if (CGM.getCodeGenOpts().getDebugInfo() < codegenoptions::LimitedDebugInfo)
 return;
   const NamespaceDecl *NSDecl = UD.getNominatedNamespace();
-  if (!NSDecl->isAnonymousNamespace() || 
-  CGM.getCodeGenOpts().DebugExplicitImport) { 
+  if (!NSDecl->isAnonymousNamespace() ||
+  CGM.getCodeGenOpts().DebugExplicitImport) {
 DBuilder.createImportedModule(
 getCurrentContextDescriptor(cast(UD.getDeclContext())),
 getOrCreateNameSpace(NSDecl),

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.h?rev=276317&r1=276316&r2=276317&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.h (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.h Thu Jul 21 13:43:20 2016
@@ -254,6 +254,8 @@ class CGDebugInfo {
 llvm::DIFile *F,
 SmallVectorImpl &E,
 llvm::DIType *RecordTy, const RecordDecl *RD);
+  void CollectRecordNestedRecord(const RecordDecl *RD,
+ SmallVectorImpl &E);
   void CollectRecordFields(const RecordDecl *Decl, llvm::DIFile *F,
SmallVectorImpl &E,
llvm::DICompositeType *RecordTy);

Modified: cfe/trunk/test/CodeGenCXX/debug-info-dup-fwd-decl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-dup-fwd-decl.cpp?rev=276317&r1=276316&r2=276317&view=diff
==
--- cfe/trunk/test/CodeGenCXX/debug-info-dup-fwd-decl.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/debug-info-dup-fwd-decl.cpp Thu Jul 21 13:43:20 
2016
@@ -19,6 +19,6 @@ protected:
 
 Test t;
 
-// CHECK: !DIDerivedType(tag: DW_TAG_pointer_type
 // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "data"
+// CHECK: !DIDerivedType(tag: DW_TAG_pointer_type
 // CHECK-NOT: !DICompositeType(tag: DW_TAG_structure_type, name: "data"

Modified: cfe/trunk/test/CodeGenCXX/debug-info-ms-abi.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-ms-abi.cpp?rev=276317&r1=276316&r2=276317&view=diff
==
--- cfe/trunk/test/CodeGenCXX/debug-info-ms-abi.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/debug-info-ms-abi.cpp Thu Jul 21 13:43:20 2016
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -triple=i686-pc-windows-msvc -debug-info-kind=limited 
-emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -triple=i686-pc-windows-msvc -debug-info-kind=limited 
-gcodeview -emit-llvm -o

Re: [PATCH] D22637: [OpenCL] Add extension cl_khr_mipmap_image to clang

2016-07-21 Thread Yaxun Liu via cfe-commits

yaxunl added inline comments.


Comment at: include/clang/Basic/OpenCLExtensions.def:73
@@ -72,2 +72,3 @@
 OPENCLEXT_INTERNAL(cl_khr_terminate_context, 200, ~0U)
+OPENCLEXT_INTERNAL(cl_khr_mipmap_image, 200, ~0U)
 

Better to follow the alphabetical order.


Repository:
  rL LLVM

https://reviews.llvm.org/D22637



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

Re: [PATCH] D22637: [OpenCL] Add extension cl_khr_mipmap_image to clang

2016-07-21 Thread Yaxun Liu via cfe-commits

yaxunl added a comment.

Need to add a test to Misc/amdgcn.languageOptsOpenCL.cl and 
SemaOpenCL/extension-version.cl


Repository:
  rL LLVM

https://reviews.llvm.org/D22637



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

Re: [PATCH] D22636: Module: retry building modules that were just compiled by the same instance and are are out of date

2016-07-21 Thread Ben Langmuir via cfe-commits

benlangmuir added a comment.

> B.pcm becomes out of date when we try to load module "A", because another 
> instance overwrites "B.pcm"


How can this happen?  We intentionally keep the file descriptors of modules 
open.  If you just built A, then you will have B.pcm still open.  When you read 
A, you will use the same B that you used when you built A even if the file on 
disk has been replaced (and we use rename to replace the file, so the existing 
one is never modified).


https://reviews.llvm.org/D22636



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

[PATCH] D22638: Module: add debug_type to dump debugging messages related to modules being out of date

2016-07-21 Thread Manman Ren via cfe-commits

manmanren created this revision.
manmanren added reviewers: benlangmuir, rsmith.
manmanren added a subscriber: cfe-commits.

This is a patch I applied internally to debug out-of-date issues. In general is 
this the right way to add debugging messages in clang frontend?


https://reviews.llvm.org/D22638

Files:
  lib/Frontend/CompilerInstance.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  lib/Serialization/ModuleManager.cpp

Index: lib/Serialization/ModuleManager.cpp
===
--- lib/Serialization/ModuleManager.cpp
+++ lib/Serialization/ModuleManager.cpp
@@ -16,6 +16,7 @@
 #include "clang/Lex/ModuleMap.h"
 #include "clang/Serialization/GlobalModuleIndex.h"
 #include "clang/Serialization/ModuleManager.h"
+#include "llvm/Support/Debug.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
@@ -25,6 +26,8 @@
 #include "llvm/Support/GraphWriter.h"
 #endif
 
+#define DEBUG_TYPE "module-manager"
+
 using namespace clang;
 using namespace serialization;
 
@@ -76,6 +79,8 @@
   }
   if (lookupModuleFile(FileName, ExpectedSize, ExpectedModTime, Entry)) {
 ErrorStr = "module file out of date";
+DEBUG(llvm::dbgs() << "In addModule: size or modtime mismatch "
+   << FileName << '\n';);
 return OutOfDate;
   }
 
@@ -169,6 +174,8 @@
   assert(ImportedBy);
 delete ModuleEntry;
   }
+  DEBUG(llvm::dbgs() << "In addModule: signature mismatch "
+ << FileName << '\n';);
   return OutOfDate;
 }
   }
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -51,6 +51,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Bitcode/BitstreamWriter.h"
 #include "llvm/Support/Compression.h"
+#include "llvm/Support/Debug.h"
 #include "llvm/Support/EndianStream.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -62,6 +63,8 @@
 #include 
 #include 
 
+#define DEBUG_TYPE "module-astwriter"
+
 using namespace clang;
 using namespace clang::serialization;
 
@@ -4113,6 +4116,7 @@
 uint64_t ASTWriter::WriteAST(Sema &SemaRef, const std::string &OutputFile,
  Module *WritingModule, StringRef isysroot,
  bool hasErrors) {
+  DEBUG(llvm::dbgs() << "In WriteAST: " << OutputFile << '\n';);
   WritingAST = true;
 
   ASTHasCompilerErrors = hasErrors;
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -49,6 +49,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Bitcode/BitstreamReader.h"
 #include "llvm/Support/Compression.h"
+#include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -60,6 +61,8 @@
 #include 
 #include 
 
+#define DEBUG_TYPE "module-astreader"
+
 using namespace clang;
 using namespace clang::serialization;
 using namespace clang::serialization::reader;
@@ -364,6 +367,8 @@
 if (Complain)
   Diags.Report(diag::err_pch_diagopt_mismatch) << "-Werror=" +
   Diags.getDiagnosticIDs()->getWarningOptionForDiag(DiagID).str();
+DEBUG(llvm::dbgs() << "In checkDiagnosticGroupMappings: DiagID "
+   << Diags.getDiagnosticIDs()->getWarningOptionForDiag(DiagID).str() << '\n';);
 return true;
   }
 }
@@ -390,27 +395,39 @@
 if (StoredDiags.getSuppressSystemWarnings()) {
   if (Complain)
 Diags.Report(diag::err_pch_diagopt_mismatch) << "-Wsystem-headers";
+  DEBUG(llvm::dbgs() << "In checkDiagnosticMappings: suppress system warnings"
+ << ": IsSystem "
+ << IsSystem << '\n';);
   return true;
 }
   }
 
   if (Diags.getWarningsAsErrors() && !StoredDiags.getWarningsAsErrors()) {
 if (Complain)
   Diags.Report(diag::err_pch_diagopt_mismatch) << "-Werror";
+DEBUG(llvm::dbgs() << "In checkDiagnosticMappings: warnings as errors"
+   << ": IsSystem "
+   << IsSystem << '\n';);
 return true;
   }
 
   if (Diags.getWarningsAsErrors() && Diags.getEnableAllWarnings() &&
   !StoredDiags.getEnableAllWarnings()) {
 if (Complain)
   Diags.Report(diag::err_pch_diagopt_mismatch) << "-Weverything -Werror";
+DEBUG(llvm::dbgs() << "In checkDiagnosticMappings: enable all warnings"
+   << ": IsSystem "
+   << IsSystem << '\n';);
 return true;
   }
 
   if (isExtHandlingFromDiagsError(Diags) &&
   !isExtHandlingFromDiagsError(StoredDiags)) {
 if (Complain)
   Diags.Report(diag::err_pch_diagopt_mismatch) << "-pedantic-errors";
+D

[PATCH] D22637: [OpenCL] Add extension cl_khr_mipmap_image to clang

2016-07-21 Thread Aaron En Ye Shi via cfe-commits

ashi1 created this revision.
ashi1 added reviewers: yaxunl, Anastasia.
ashi1 added a subscriber: cfe-commits.
ashi1 set the repository for this revision to rL LLVM.

Adding extension cl_khr_mipmap_image to clang's OpenCL Extensions and initiated 
inside AMDGPU Target.

Repository:
  rL LLVM

https://reviews.llvm.org/D22637

Files:
  include/clang/Basic/OpenCLExtensions.def
  lib/Basic/Targets.cpp

Index: lib/Basic/Targets.cpp
===
--- lib/Basic/Targets.cpp
+++ lib/Basic/Targets.cpp
@@ -2116,6 +2116,7 @@
   Opts.cl_khr_int64_base_atomics = 1;
   Opts.cl_khr_int64_extended_atomics = 1;
   Opts.cl_khr_3d_image_writes = 1;
+  Opts.cl_khr_mipmap_image = 1;
 }
   }
 
Index: include/clang/Basic/OpenCLExtensions.def
===
--- include/clang/Basic/OpenCLExtensions.def
+++ include/clang/Basic/OpenCLExtensions.def
@@ -70,6 +70,7 @@
 OPENCLEXT_INTERNAL(cl_khr_srgb_image_writes, 200, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_subgroups, 200, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_terminate_context, 200, ~0U)
+OPENCLEXT_INTERNAL(cl_khr_mipmap_image, 200, ~0U)
 
 // Clang Extensions.
 OPENCLEXT_INTERNAL(cl_clang_storage_class_specifiers, 100, ~0U)


Index: lib/Basic/Targets.cpp
===
--- lib/Basic/Targets.cpp
+++ lib/Basic/Targets.cpp
@@ -2116,6 +2116,7 @@
   Opts.cl_khr_int64_base_atomics = 1;
   Opts.cl_khr_int64_extended_atomics = 1;
   Opts.cl_khr_3d_image_writes = 1;
+  Opts.cl_khr_mipmap_image = 1;
 }
   }
 
Index: include/clang/Basic/OpenCLExtensions.def
===
--- include/clang/Basic/OpenCLExtensions.def
+++ include/clang/Basic/OpenCLExtensions.def
@@ -70,6 +70,7 @@
 OPENCLEXT_INTERNAL(cl_khr_srgb_image_writes, 200, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_subgroups, 200, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_terminate_context, 200, ~0U)
+OPENCLEXT_INTERNAL(cl_khr_mipmap_image, 200, ~0U)
 
 // Clang Extensions.
 OPENCLEXT_INTERNAL(cl_clang_storage_class_specifiers, 100, ~0U)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D22636: Module: retry building modules that were just compiled by the same instance and are are out of date

2016-07-21 Thread Manman Ren via cfe-commits

manmanren created this revision.
manmanren added reviewers: benlangmuir, rsmith.
manmanren added a subscriber: cfe-commits.

Even though this instance just built module "A", it is likely that "A" imports 
another module "B" and B.pcm becomes out of date when we try to load module 
"A", because another instance overwrites "B.pcm" due to changes in warning 
options. This patch tries to fix the error: "Module file ‘.pcm' 
is out of date and needs to be rebuilt" by simply trying again.

This seems to be the last case where we call ReadAST without the ability to 
handle out-of-date.

If there are other ways to fix the issue, please let me know.
Also I don't quite know how to add a testing case when two compiling instances 
are involved and running in parallel.

rdar://problem/26676111

https://reviews.llvm.org/D22636

Files:
  lib/Frontend/CompilerInstance.cpp

Index: lib/Frontend/CompilerInstance.cpp
===
--- lib/Frontend/CompilerInstance.cpp
+++ lib/Frontend/CompilerInstance.cpp
@@ -1096,6 +1096,7 @@
 diagnoseBuildFailure();
 return false;
   }
+  ModuleLoadCapabilities |= ASTReader::ARR_OutOfDate;
   break;
 
 case llvm::LockFileManager::LFS_Shared:
@@ -1124,10 +1125,14 @@
 ModuleLoadCapabilities);
 
 if (ReadResult == ASTReader::OutOfDate &&
-Locked == llvm::LockFileManager::LFS_Shared) {
+(Locked == llvm::LockFileManager::LFS_Shared ||
+ Locked == llvm::LockFileManager::LFS_Owned)) {
   // The module may be out of date in the presence of file system races,
   // or if one of its imports depends on header search paths that are not
   // consistent with this ImportingInstance.  Try again...
+
+  // The module may be out of date right after we rebuild it if a module
+  // it imports is overwritten by another process. Try again...
   continue;
 } else if (ReadResult == ASTReader::Missing) {
   diagnoseBuildFailure();


Index: lib/Frontend/CompilerInstance.cpp
===
--- lib/Frontend/CompilerInstance.cpp
+++ lib/Frontend/CompilerInstance.cpp
@@ -1096,6 +1096,7 @@
 diagnoseBuildFailure();
 return false;
   }
+  ModuleLoadCapabilities |= ASTReader::ARR_OutOfDate;
   break;
 
 case llvm::LockFileManager::LFS_Shared:
@@ -1124,10 +1125,14 @@
 ModuleLoadCapabilities);
 
 if (ReadResult == ASTReader::OutOfDate &&
-Locked == llvm::LockFileManager::LFS_Shared) {
+(Locked == llvm::LockFileManager::LFS_Shared ||
+ Locked == llvm::LockFileManager::LFS_Owned)) {
   // The module may be out of date in the presence of file system races,
   // or if one of its imports depends on header search paths that are not
   // consistent with this ImportingInstance.  Try again...
+
+  // The module may be out of date right after we rebuild it if a module
+  // it imports is overwritten by another process. Try again...
   continue;
 } else if (ReadResult == ASTReader::Missing) {
   diagnoseBuildFailure();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21567: [OpenCL] Generate struct type for sampler_t and function call for the initializer

2016-07-21 Thread Yaxun Liu via cfe-commits

yaxunl updated this revision to Diff 64927.
yaxunl marked 4 inline comments as done.
yaxunl added a comment.

Update the test to accommodate changes in branch tip.


https://reviews.llvm.org/D21567

Files:
  include/clang/AST/OperationKinds.def
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/AST/ASTContext.cpp
  lib/AST/Expr.cpp
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGExprAgg.cpp
  lib/CodeGen/CGExprComplex.cpp
  lib/CodeGen/CGExprConstant.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/CodeGen/CGOpenCLRuntime.cpp
  lib/CodeGen/CGOpenCLRuntime.h
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  lib/Edit/RewriteObjCFoundationAPI.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaInit.cpp
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  test/CodeGenOpenCL/opencl_types.cl
  test/CodeGenOpenCL/sampler.cl
  test/SemaOpenCL/sampler_t.cl

Index: test/SemaOpenCL/sampler_t.cl
===
--- test/SemaOpenCL/sampler_t.cl
+++ test/SemaOpenCL/sampler_t.cl
@@ -1,6 +1,25 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -DCHECK_SAMPLER_VALUE -Wspir-compat -triple amdgcn--amdhsa
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -DCHECK_SAMPLER_VALUE -triple spir-unknown-unknown
 
-constant sampler_t glb_smp = 5;
+#define CLK_ADDRESS_CLAMP_TO_EDGE   2
+#define CLK_NORMALIZED_COORDS_TRUE  1
+#define CLK_FILTER_NEAREST  0x10
+#define CLK_FILTER_LINEAR   0x20
+
+constant sampler_t glb_smp = CLK_ADDRESS_CLAMP_TO_EDGE | CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_LINEAR;
+constant sampler_t glb_smp2; // expected-error{{variable in constant address space must be initialized}}
+global sampler_t glb_smp3 = CLK_ADDRESS_CLAMP_TO_EDGE | CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_NEAREST; // expected-error{{sampler type cannot be used with the __local and __global address space qualifiers}}
+
+constant sampler_t glb_smp4 = 0;
+#ifdef CHECK_SAMPLER_VALUE
+// expected-warning@-2{{sampler initializer has invalid Filter Mode bits}}
+#endif
+
+constant sampler_t glb_smp5 = 0x1f;
+#ifdef CHECK_SAMPLER_VALUE
+// expected-warning@-2{{sampler initializer has invalid Addressing Mode bits}}
+#endif
 
 void foo(sampler_t);
 
@@ -10,22 +29,27 @@
 
 void kernel ker(sampler_t argsmp) {
   local sampler_t smp; // expected-error{{sampler type cannot be used with the __local and __global address space qualifiers}}
-  const sampler_t const_smp = 7;
+  const sampler_t const_smp = CLK_ADDRESS_CLAMP_TO_EDGE | CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_LINEAR;
+  const sampler_t const_smp2;
   foo(glb_smp);
+  foo(glb_smp2);
+  foo(glb_smp3);
   foo(const_smp);
+  foo(const_smp2);
+  foo(argsmp);
   foo(5); // expected-error{{sampler_t variable required - got 'int'}}
   sampler_t sa[] = {argsmp, const_smp}; // expected-error {{array of 'sampler_t' type is invalid in OpenCL}}
 }
 
 void bad(sampler_t*); // expected-error{{pointer to type 'sampler_t' is invalid in OpenCL}}
 
 void bar() {
-  sampler_t smp1 = 7;
-  sampler_t smp2 = 2;
+  sampler_t smp1 = CLK_ADDRESS_CLAMP_TO_EDGE | CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_LINEAR;
+  sampler_t smp2 = CLK_ADDRESS_CLAMP_TO_EDGE | CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_NEAREST;
   smp1=smp2; //expected-error{{invalid operands to binary expression ('sampler_t' and 'sampler_t')}}
   smp1+1; //expected-error{{invalid operands to binary expression ('sampler_t' and 'int')}}
   &smp1; //expected-error{{invalid argument type 'sampler_t' to unary expression}}
   *smp2; //expected-error{{invalid argument type 'sampler_t' to unary expression}}
 }
 
-sampler_t bad(); //expected-error{{declaring function return value of type 'sampler_t' is not allowed}}
+sampler_t bad(void); //expected-error{{declaring function return value of type 'sampler_t' is not allowed}}
Index: test/CodeGenOpenCL/sampler.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/sampler.cl
@@ -0,0 +1,57 @@
+// RUN: %clang_cc1 %s -emit-llvm -triple spir-unknown-unknown -o - -O0 | FileCheck %s
+//
+// This test covers 6 cases of sampler initialzation:
+//   1. function argument passing
+//  1a. argument is a file-scope variable
+//  1b. argument is a function-scope variable
+//  1c. argument is one of caller function's parameters
+//   2. variable initialization
+//  2a. initializing a file-scope variable
+//  2b. initializing a function-scope variable
+
+#define CLK_ADDRESS_CLAMP_TO_EDGE   2
+#define CLK_NORMALIZED_COORDS_TRUE  1
+#define CLK_FILTER_NEAREST  0x10
+#define CLK_FILTER_LINEAR   0x20
+
+// CHECK: %__opencl_sampler_t = type opaque
+
+// Case 2a
+constant sampler_t glb_smp = CLK_ADDRESS_CLAMP_TO_EDGE | CLK_NORMALIZED_COORDS_TRUE | CLK_FILTE

[libcxx] r276309 - Remove FIXME for feature test macro

2016-07-21 Thread JF Bastien via cfe-commits

Author: jfb
Date: Thu Jul 21 12:34:28 2016
New Revision: 276309

URL: http://llvm.org/viewvc/llvm-project?rev=276309&view=rev
Log:
Remove FIXME for feature test macro

The value I'd picked was correct, as per the recently published SG10 paper 
http://wg21.link/p0096r3

Modified:
libcxx/trunk/include/atomic

Modified: libcxx/trunk/include/atomic
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/atomic?rev=276309&r1=276308&r2=276309&view=diff
==
--- libcxx/trunk/include/atomic (original)
+++ libcxx/trunk/include/atomic Thu Jul 21 12:34:28 2016
@@ -557,7 +557,6 @@ void atomic_signal_fence(memory_order m)
 #endif
 
 #if _LIBCPP_STD_VER > 14
-// FIXME: use the right feature test macro value as chose by SG10.
 # define __cpp_lib_atomic_is_always_lock_free 201603L
 #endif
 


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

Re: [PATCH] D22557: [libcxx] Diagnose invalid memory order arguments in . Fixes PR21179.

2016-07-21 Thread JF Bastien via cfe-commits

jfb added a comment.

Two comments, then lgtm.



Comment at: include/atomic:581
@@ +580,3 @@
+   || __f == memory_order_acq_rel, "")))  \
+__attribute__ ((__unavailable__("memory order argument to atomic operation 
is invalid")))
+#endif

OK that's totally fine with me. Could you put this in a comment? Something like 
"not checking the 'stronger' requirement, see wg21.link/p0418" (it'll be 
published in the pre-Issaquah meeting, URL will work then).


Comment at: test/libcxx/atomics/diagnose_invalid_memory_order.fail.cpp:86
@@ +85,3 @@
+// does not generate any diagnostics.
+x.compare_exchange_weak(val1, val2, std::memory_order_release);
+}

Could you do the other 5 (same below).


https://reviews.llvm.org/D22557



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

Re: [PATCH] D20168: [CodeGen] Handle structs directly in AMDGPUABIInfo

2016-07-21 Thread Vedran Miletić via cfe-commits

rivanvx updated this revision to Diff 64920.
rivanvx marked 2 inline comments as done.

https://reviews.llvm.org/D20168

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGenOpenCL/amdgpu-abi-struct-coerce.cl

Index: test/CodeGenOpenCL/amdgpu-abi-struct-coerce.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/amdgpu-abi-struct-coerce.cl
@@ -0,0 +1,66 @@
+// REQUIRES: amdgpu-registered-target
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -S -emit-llvm -o - %s | FileCheck %s
+
+// CHECK-NOT: %struct.single_element_struct_arg = type { i32 }
+typedef struct single_element_struct_arg
+{
+int i;
+} single_element_struct_arg_t;
+
+// CHECK: %struct.struct_arg = type { i32, float, i32 }
+typedef struct struct_arg
+{
+int i1;
+float f;
+int i2;
+} struct_arg_t;
+
+// CHECK: %struct.struct_of_arrays_arg = type { [2 x i32], float, [4 x i32], [3 x float], i32 }
+typedef struct struct_of_arrays_arg
+{
+int i1[2];
+float f1;
+int i2[4];
+float f2[3];
+int i3;
+} struct_of_arrays_arg_t;
+
+// CHECK: %struct.struct_of_structs_arg = type { i32, float, %struct.struct_arg, i32 }
+typedef struct struct_of_structs_arg
+{
+int i1;
+float f1;
+struct_arg_t s1;
+int i2;
+} struct_of_structs_arg_t;
+
+// CHECK-LABEL: @test_single_element_struct_arg
+// CHECK: i32 %arg1.coerce
+__kernel void test_single_element_struct_arg(single_element_struct_arg_t arg1)
+{
+}
+
+// CHECK-LABEL: @test_struct_arg
+// CHECK: %struct.struct_arg %arg1.coerce
+__kernel void test_struct_arg(struct_arg_t arg1)
+{
+}
+
+// CHECK-LABEL: @test_struct_of_arrays_arg
+// CHECK: %struct.struct_of_arrays_arg %arg1.coerce
+__kernel void test_struct_of_arrays_arg(struct_of_arrays_arg_t arg1)
+{
+}
+
+// CHECK-LABEL: @test_struct_of_structs_arg
+// CHECK: %struct.struct_of_structs_arg %arg1.coerce
+__kernel void test_struct_of_structs_arg(struct_of_structs_arg_t arg1)
+{
+}
+
+// CHECK-LABEL: @test_non_kernel_struct_arg
+// CHECK-NOT: %struct.struct_arg %arg1.coerce
+// CHECK: %struct.struct_arg* byval
+void test_non_kernel_struct_arg(struct_arg_t arg1)
+{
+}
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -6825,10 +6825,50 @@
 
 namespace {
 
+class AMDGPUABIInfo final : public DefaultABIInfo {
+public:
+  explicit AMDGPUABIInfo(CodeGen::CodeGenTypes &CGT) : DefaultABIInfo(CGT) {}
+
+private:
+  ABIArgInfo classifyArgumentType(QualType Ty) const;
+
+  void computeInfo(CGFunctionInfo &FI) const override;
+};
+
+void AMDGPUABIInfo::computeInfo(CGFunctionInfo &FI) const {
+  if (!getCXXABI().classifyReturnType(FI))
+FI.getReturnInfo() = classifyReturnType(FI.getReturnType());
+
+  unsigned CC = FI.getCallingConvention();
+  for (auto &Arg : FI.arguments())
+if (CC == llvm::CallingConv::AMDGPU_KERNEL)
+  Arg.info = classifyArgumentType(Arg.type);
+else
+  Arg.info = DefaultABIInfo::classifyArgumentType(Arg.type);
+}
+
+/// \brief Classify argument of given type \p Ty.
+ABIArgInfo AMDGPUABIInfo::classifyArgumentType(QualType Ty) const {
+  llvm::StructType *StrTy = dyn_cast(CGT.ConvertType(Ty));
+  if (!StrTy) {
+return DefaultABIInfo::classifyArgumentType(Ty);
+  }
+
+  // Coerce single element structs to its element.
+  if (StrTy->getNumElements() == 1) {
+return ABIArgInfo::getDirect();
+  }
+
+  // If we set CanBeFlattened to true, CodeGen will expand the struct to its
+  // individual elements, which confuses the Clover OpenCL backend; therefore we
+  // have to set it to false here. Other args of getDirect() are just defaults.
+  return ABIArgInfo::getDirect(nullptr, 0, nullptr, false);
+}
+
 class AMDGPUTargetCodeGenInfo : public TargetCodeGenInfo {
 public:
   AMDGPUTargetCodeGenInfo(CodeGenTypes &CGT)
-: TargetCodeGenInfo(new DefaultABIInfo(CGT)) {}
+: TargetCodeGenInfo(new AMDGPUABIInfo(CGT)) {}
   void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
CodeGen::CodeGenModule &M) const override;
   unsigned getOpenCLKernelCallingConv() const override;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20168: [CodeGen] Handle structs directly in AMDGPUABIInfo

2016-07-21 Thread Vedran Miletić via cfe-commits

rivanvx added a comment.

Addressed both concerns.


https://reviews.llvm.org/D20168



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

Re: [PATCH] D22608: [Profile] Use a flag to enable PGO rather than the profraw filename.

2016-07-21 Thread David Li via cfe-commits

davidxl accepted this revision.
davidxl added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rL LLVM

https://reviews.llvm.org/D22608



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

Re: [PATCH] D22587: [ASTContext] Fix part of DR224 for nontype template arguments

2016-07-21 Thread Richard Smith via cfe-commits

On 20 Jul 2016 1:25 p.m., "Matthias Gehre"  wrote:
>
> mgehre created this revision.
> mgehre added reviewers: klimek, aaron.ballman, rsmith.
> mgehre added a subscriber: cfe-commits.
>
> Look through expressions to determine if a nontype template argument has
been given the value of the template parameter.
>
> https://reviews.llvm.org/D22587
>
> Files:
>   lib/AST/ASTContext.cpp
>   test/CXX/drs/dr2xx.cpp
>
> Index: test/CXX/drs/dr2xx.cpp
> ===
> --- test/CXX/drs/dr2xx.cpp
> +++ test/CXX/drs/dr2xx.cpp
> @@ -275,9 +275,9 @@
>static const int my_I = I;
>static const int my_I2 = I+0;
>static const int my_I3 = my_I;
> -  B::type b3; // FIXME: expected-error {{missing
'typename'}}
> +  B::type b3;
>B::type b4; // expected-error {{missing
'typename'}}
> -  B::type b5; // FIXME: expected-error {{missing
'typename'}}
> +  B::type b5;
>  };
>}
>
> Index: lib/AST/ASTContext.cpp
> ===
> --- lib/AST/ASTContext.cpp
> +++ lib/AST/ASTContext.cpp
> @@ -4448,8 +4448,26 @@
>  case TemplateArgument::Null:
>return Arg;
>
> -case TemplateArgument::Expression:
> +case TemplateArgument::Expression: {
> +  // Look through variable declarations that have been initialized
to a non-type template
> +  // parameter, see 14.6.2.1 [temp.dep.type]:
> +  // [...], the argument must have been given the value of
> +  // the template parameter and not an expression involving the
template parameter.
> +  auto *E = Arg.getAsExpr()->IgnoreImpCasts();

Are all implicit casts really OK here? If the parameter is narrowed, the
value could change. Perhaps we should only walk through lvalue-to-rvalue
conversions here. What about parentheses, should we skip those?

> +  while(auto *DeclRef = dyn_cast_or_null(E)) {
> +auto *D = DeclRef->getDecl();
> +if (isa(D))
> +  return TemplateArgument(DeclRef);
> +
> +auto *VD = dyn_cast(D);
> +if (!VD)
> +  break;
> +E = VD->getInit();
> +if (E)
> +  E = E->IgnoreImpCasts();
> +  }
>return Arg;
> +}
>
>  case TemplateArgument::Declaration: {
>ValueDecl *D =
cast(Arg.getAsDecl()->getCanonicalDecl());
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15075: No error for conflict between inputs\outputs and clobber list

2016-07-21 Thread Akira Hatanaka via cfe-commits

ahatanak added a comment.

In https://reviews.llvm.org/D15075#486986, @zizhar wrote:

> Akira,
>  You've mentioned a good point, this X86 logic should indeed be moved to 
> X86TargetInfo.
>  The current convertConstraint() implementation is not doing what I need – it 
> doesn’t handle all the input/output constraints I need, and it returns the 
> constraint in a different format than the one I need.
>  E.g.  convertConstraint() does not handle “r” constraints or special 
> characters like “=”, “+”, also, the function returns the registers as “{ax}”, 
> when I need the “ax”.
>  I think it is better to add a new function for my logic instead of trying to 
> adjust this function to handle both its old logic and my new logic.
>
> My new solution is going to be to create another virtual function in 
> TargetInfo that will do everything I need.
>  The code that is currently under GetConstraintRegister() and 
> ExtractRegisterName() will now be part of the X86TargetInfo implementation of 
> this virtual function.
>  For all the other architectures’ TargetInfos I’ll create a default 
> implementation that will return an empty string and they can implement it if 
> they want to (until they do, the existing behavior will be retained). 
>  Can I have your opinion on this? Do you see a better way to implement this?


I agree with you. You can define a virtual function in TargetInfo, which checks 
conflicts between clobbers and input/output, and override it in X86's 
TargetInfo.



Comment at: lib/Headers/Intrin.h:841-874
@@ -840,44 +840,36 @@
 #if defined(__i386__) || defined(__x86_64__)
 static __inline__ void __DEFAULT_FN_ATTRS
 __movsb(unsigned char *__dst, unsigned char const *__src, size_t __n) {
-  __asm__("rep movsb" : : "D"(__dst), "S"(__src), "c"(__n)
-: "%edi", "%esi", "%ecx");
+  __asm__("rep movsb" : : "D"(__dst), "S"(__src), "c"(__n) :);
 }
 static __inline__ void __DEFAULT_FN_ATTRS
 __movsd(unsigned long *__dst, unsigned long const *__src, size_t __n) {
-  __asm__("rep movsl" : : "D"(__dst), "S"(__src), "c"(__n)
-: "%edi", "%esi", "%ecx");
+  __asm__("rep movsl" : : "D"(__dst), "S"(__src), "c"(__n) :);
 }
 static __inline__ void __DEFAULT_FN_ATTRS
 __movsw(unsigned short *__dst, unsigned short const *__src, size_t __n) {
-  __asm__("rep movsw" : : "D"(__dst), "S"(__src), "c"(__n)
-: "%edi", "%esi", "%ecx");
+  __asm__("rep movsw" : : "D"(__dst), "S"(__src), "c"(__n) :);
 }
 static __inline__ void __DEFAULT_FN_ATTRS
 __stosb(unsigned char *__dst, unsigned char __x, size_t __n) {
-  __asm__("rep stosb" : : "D"(__dst), "a"(__x), "c"(__n)
-: "%edi", "%ecx");
+  __asm__("rep stosb" : : "D"(__dst), "a"(__x), "c"(__n) :);
 }
 static __inline__ void __DEFAULT_FN_ATTRS
 __stosd(unsigned long *__dst, unsigned long __x, size_t __n) {
-  __asm__("rep stosl" : : "D"(__dst), "a"(__x), "c"(__n)
-: "%edi", "%ecx");
+  __asm__("rep stosl" : : "D"(__dst), "a"(__x), "c"(__n) :);
 }
 static __inline__ void __DEFAULT_FN_ATTRS
 __stosw(unsigned short *__dst, unsigned short __x, size_t __n) {
-  __asm__("rep stosw" : : "D"(__dst), "a"(__x), "c"(__n)
-: "%edi", "%ecx");
+  __asm__("rep stosw" : : "D"(__dst), "a"(__x), "c"(__n) :);
 }
 #endif
 #ifdef __x86_64__
 static __inline__ void __DEFAULT_FN_ATTRS
 __movsq(unsigned long long *__dst, unsigned long long const *__src, size_t 
__n) {
-  __asm__("rep movsq" : : "D"(__dst), "S"(__src), "c"(__n)
-: "%edi", "%esi", "%ecx");
+  __asm__("rep movsq" : : "D"(__dst), "S"(__src), "c"(__n) :);
 }
 static __inline__ void __DEFAULT_FN_ATTRS
 __stosq(unsigned __int64 *__dst, unsigned __int64 __x, size_t __n) {
-  __asm__("rep stosq" : : "D"(__dst), "a"(__x), "c"(__n)
-: "%edi", "%ecx");
+  __asm__("rep stosq" : : "D"(__dst), "a"(__x), "c"(__n) :);
 }
 #endif

majnemer wrote:
> The clobber should be "memory", no?
> Er, and shouldn't the we list `__dst`, `__x` and `__n` as outputs because 
> they are modified?
Yes, I think we need "memory" here because it writes to the memory pointed to 
by dst.

Do we need to add x to the output too? I think dst and n are modfied by stosq 
and rep (which I think is why %edi and %ecx were added to the clobber list), 
but I don't see how x gets modified.


https://reviews.llvm.org/D15075



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

Re: [PATCH] D21459: Implement http://wg21.link/P0254R1: "Integrating std::string_view and std::string"

2016-07-21 Thread Kim Gräsman via cfe-commits

kimgr added inline comments.

Comment at: include/string_view:216
@@ +215,3 @@
+   basic_string_view(const _CharT* __s)
+   : __data(__s), __size(_Traits::length(__s)) {}
+

mclow.lists wrote:
> kimgr wrote:
> > I'm working from the paper at https://isocpp.org/files/papers/N3762.html, 
> > and I find it a little sketchy on the policy for nullptrs.
> > 
> > Since the ctor above accepts nullptr as long as the length is zero, would 
> > it make sense to do that here too? That is, only call _Traits::length for 
> > non-nullptr __s args?
> Reading from N4600: Requires: `[str, str + traits::length(str))` is a valid 
> range.
> 
> So, no - passing `nullptr` here is undefined.
> 
OK, that feels more principled to me, anyway.

But the `(const char*, size_t)` constructor has the same requirement and it 
*does* accept `nullptr`, provided the length is zero. I saw you had to get rid 
of the assertion, but I feel like the constructor should probably not silently 
accept `nullptr` for zero-sized strings. Or do you know if that's intentional? 
Many StringRef/StringPiece implementations seem to have the same behavior.

https://reviews.llvm.org/D21459

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

Re: [PATCH] D22587: [ASTContext] Fix part of DR224 for nontype template arguments

2016-07-21 Thread Aaron Ballman via cfe-commits

aaron.ballman added a comment.

Generally LGTM with a small nit.



Comment at: lib/AST/ASTContext.cpp:4456
@@ +4455,3 @@
+  // the template parameter and not an expression involving the template 
parameter.
+  auto *E = Arg.getAsExpr()->IgnoreImpCasts();
+  while(auto *DeclRef = dyn_cast_or_null(E)) {

`const auto *` (and propagate it to other declarations)?


https://reviews.llvm.org/D22587



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

Re: [PATCH] D22208: clang-tidy] Fixes to modernize-use-emplace

2016-07-21 Thread Aaron Ballman via cfe-commits

aaron.ballman added inline comments.


Comment at: clang-tidy/modernize/UseEmplaceCheck.h:36-37
@@ -32,1 +35,4 @@
+private:
+  std::vector ContainersWithPushBack;
+  std::vector SmartPointers;
 };

Why not use a SmallVector for these instead of a std::vector? Then you don't 
need to typecast in registerMatchers().

If it's because of parseStringList(), I think that you can work around it's 
tight coupling using llvm::iterator_range(parseStringList()) to perform the 
initialization in the ctor initializer list.


Repository:
  rL LLVM

https://reviews.llvm.org/D22208



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

Re: [PATCH] D21453: Add support for attribute "overallocated"

2016-07-21 Thread Aaron Ballman via cfe-commits

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM with one small testing nit, but you should wait for @rsmith to chime in 
since he had comments previously.



Comment at: test/SemaCXX/flexible-array-attr.cpp:52
@@ +51,3 @@
+  int a[4];
+  int b[4] __attribute__((flexible_array)); // expected-error 
{{'flexible_array' attribute only applies to the last member of a non-union 
class}}
+};

Would be good to change this to b[1] so that it looks like an otherwise 
well-formed construct. Should not change the diagnostic.


https://reviews.llvm.org/D21453



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

Re: [PATCH] D22507: Clang-tidy - Enum misuse check

2016-07-21 Thread Aaron Ballman via cfe-commits

aaron.ballman added a subscriber: aaron.ballman.


Comment at: clang-tidy/misc/EnumMisuseCheck.cpp:31
@@ +30,3 @@
+// Stores a min and a max value which describe an interval.
+struct ValueRange {
+  llvm::APSInt MinVal, MaxVal;

I think this class can be replaced by `std::minmax_element()` over the 
`EnumDec->enumerators()` and then grabbing the actual values out of the 
resulting `std::pair`.


Comment at: clang-tidy/misc/EnumMisuseCheck.cpp:59
@@ +58,3 @@
+
+bool isMaxValAllBitSet(const EnumDecl *EnumDec) {
+  for (auto I = EnumDec->enumerator_begin(), E = EnumDec->enumerator_end();

This function doesn't do what is described. It appears to be checking if the 
last value in the enumeration has all its bits set, not if the max value has 
all the bits set. e.g., it won't check:
```
enum E {
  MaxValue = 0x,
  First = 0,
  Second
};
```


Comment at: clang-tidy/misc/EnumMisuseCheck.h:24
@@ +23,3 @@
+class EnumMisuseCheck : public ClangTidyCheck {
+const bool IsStrict;
+

hokein wrote:
> Put it to private member.
It is a private member already. I think this usage is fine.


https://reviews.llvm.org/D22507



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

Re: [PATCH] D22507: Clang-tidy - Enum misuse check

2016-07-21 Thread Peter Szecsi via cfe-commits

szepet marked an inline comment as done.
szepet added a comment.

https://reviews.llvm.org/D22507



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

Re: [PATCH] D22507: Clang-tidy - Enum misuse check

2016-07-21 Thread Peter Szecsi via cfe-commits

szepet removed rL LLVM as the repository for this revision.
szepet updated this revision to Diff 64897.
szepet added a comment.

updating patch based on review comments


https://reviews.llvm.org/D22507

Files:
  clang-tidy/misc/EnumMisuseCheck.cpp
  clang-tidy/misc/EnumMisuseCheck.h
  docs/clang-tidy/checks/misc-enum-misuse.rst
  test/clang-tidy/misc-enum-misuse-weak.cpp
  test/clang-tidy/misc-enum-misuse.cpp

Index: test/clang-tidy/misc-enum-misuse.cpp
===
--- test/clang-tidy/misc-enum-misuse.cpp
+++ test/clang-tidy/misc-enum-misuse.cpp
@@ -1,6 +1,7 @@
 // RUN: %check_clang_tidy %s misc-enum-misuse %t -- -config="{CheckOptions: [{key: misc-enum-misuse.IsStrict, value: 0}]}" --
 
-enum Empty {};
+enum Empty {
+};
 
 enum A { A = 1,
  B = 2,
@@ -8,23 +9,27 @@
  D = 8,
  E = 16,
  F = 32,
- G = 63 };
+ G = 63
+};
 
 enum X { X = 8,
  Y = 16,
- Z = 4 };
+ Z = 4
+};
 
-enum {P = 2,
- Q = 3,
- R = 4,
- S = 8,
- T = 16 };
+enum { P = 2,
+   Q = 3,
+   R = 4,
+   S = 8,
+   T = 16
+};
 
 enum { H,
I,
J,
K,
-   L };
+   L
+};
 
 enum Days { Monday,
 Tuesday,
@@ -32,14 +37,14 @@
 Thursday,
 Friday,
 Saturday,
-Sunday };
+Sunday
+};
 
 Days bestDay() {
   return Friday;
 }
 
 int trigger() {
-
   if (bestDay() | A)
 return 1;
   // CHECK-MESSAGES: :[[@LINE-2]]:17: warning: enum values are from different enum types [misc-enum-misuse]
@@ -47,9 +52,8 @@
 return 1;
   // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: enum values are from different enum types [misc-enum-misuse]
   unsigned p;
-  p = Q | P; 
+  p = Q | P;
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: left hand side value is not power-of-2 unlike most other values in the enum type [misc-enum-misuse]
-
 }
 
 int dont_trigger() {
@@ -74,4 +78,4 @@
   if (H + I + L == 42)
 return 1;
   return 42;
-};
+}
Index: test/clang-tidy/misc-enum-misuse-weak.cpp
===
--- test/clang-tidy/misc-enum-misuse-weak.cpp
+++ test/clang-tidy/misc-enum-misuse-weak.cpp
@@ -6,24 +6,28 @@
  D = 8,
  E = 16,
  F = 32,
- G = 63 };
+ G = 63
+};
 
 enum X { X = 8,
  Y = 16,
- Z = 4 };
+ Z = 4
+};
 // CHECK-MESSAGES: :[[@LINE+1]]:1: warning: enum type seems like a bitfield but possibly contains misspelled number(s) [misc-enum-misuse]
 enum PP { P = 2,
   Q = 3,
   R = 4,
   S = 8,
   T = 16,
-  U = 31};
+  U = 31
+};
 
 enum { H,
I,
J,
K,
-   L };
+   L
+};
 
 enum Days { Monday,
 Tuesday,
@@ -31,7 +35,8 @@
 Thursday,
 Friday,
 Saturday,
-Sunday };
+Sunday
+};
 
 Days bestDay() {
   return Friday;
@@ -59,7 +64,7 @@
   // Line 60 triggers the LINE:18 warning
   p = A | G;
   //p = G | X;
-return 0;
+  return 0;
 }
 
 int dont_trigger() {
@@ -68,7 +73,7 @@
   int d = c | H, e = b * a;
   a = B | C;
   b = X | Z;
- 
+
   unsigned bitflag;
   enum A aa = B;
   bitflag = aa | C;
@@ -80,5 +85,4 @@
   if (H + I + L == 42)
 return 1;
   return 42;
-};
-
+}
Index: docs/clang-tidy/checks/misc-enum-misuse.rst
===
--- docs/clang-tidy/checks/misc-enum-misuse.rst
+++ docs/clang-tidy/checks/misc-enum-misuse.rst
@@ -23,3 +23,44 @@
  enum val. (only in "Strict")
   4. Check both side of | or + operator where the enum values are from the same
  enum type.
+
+Examples:
+
+.. code:: c++
+
+//1.
+Enum {A, B, C}
+Enum {D, E, F}
+Enum {G = 10, H = 11, I = 12};
+
+unsigned flag;
+flag = A | H; //OK, disjoint value intervalls in the enum types > probably good use
+flag = B | F; //warning, have common values so they are probably misused
+  
+
+
+//2.
+
+Enum Bitfield { A = 0;
+B = 1;
+C = 2;
+D = 4;
+E = 8;
+F = 16;
+G = 31; //OK, real bitfield
+}
+
+Enum AlmostBitfield { AA = 0;
+  BB = 1;
+  CC = 2;
+  DD = 4;
+  EE = 8;
+  FF = 16;
+  GG; //Problem, forgot to initialize
+}
+
+  unsigned flag = 0;
+  flag |= E; //ok
+  flag |= EE; //warning at the decl, and note that it was used here as a bitfield
+
+void f(const string&);  // Good: const is not top level.
Index: clang-tidy/misc/EnumMisuseCheck.h
===
--- clang-tidy/misc/EnumMisuseCheck.h
+++ clang-tidy/misc/EnumMisuseCheck.h
@@ -16,18 +16,20 @@
 namespace tidy {
 namespace misc {
 
-/// FIXME: Write a short description.
-///
+/// The checker det

Re: [PATCH] D22507: Clang-tidy - Enum misuse check

2016-07-21 Thread Peter Szecsi via cfe-commits

szepet marked 18 inline comments as done.
szepet added a comment.

https://reviews.llvm.org/D22507



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

r276292 - Move some IntrusiveRefCntPtrs instead of copying.

2016-07-21 Thread Benjamin Kramer via cfe-commits

Author: d0k
Date: Thu Jul 21 10:06:51 2016
New Revision: 276292

URL: http://llvm.org/viewvc/llvm-project?rev=276292&view=rev
Log:
Move some IntrusiveRefCntPtrs instead of copying.

No functionality change intended.

Modified:
cfe/trunk/include/clang/Basic/Diagnostic.h
cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp
cfe/trunk/lib/Basic/Diagnostic.cpp

Modified: cfe/trunk/include/clang/Basic/Diagnostic.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Diagnostic.h?rev=276292&r1=276291&r2=276292&view=diff
==
--- cfe/trunk/include/clang/Basic/Diagnostic.h (original)
+++ cfe/trunk/include/clang/Basic/Diagnostic.h Thu Jul 21 10:06:51 2016
@@ -344,11 +344,10 @@ private:
   std::string FlagValue;
 
 public:
-  explicit DiagnosticsEngine(
-  const IntrusiveRefCntPtr &Diags,
-  DiagnosticOptions *DiagOpts,
-  DiagnosticConsumer *client = nullptr,
-  bool ShouldOwnClient = true);
+  explicit DiagnosticsEngine(IntrusiveRefCntPtr Diags,
+ DiagnosticOptions *DiagOpts,
+ DiagnosticConsumer *client = nullptr,
+ bool ShouldOwnClient = true);
   ~DiagnosticsEngine();
 
   const IntrusiveRefCntPtr &getDiagnosticIDs() const {

Modified: cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp?rev=276292&r1=276291&r2=276292&view=diff
==
--- cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp (original)
+++ cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp Thu Jul 21 10:06:51 2016
@@ -72,10 +72,10 @@ private:
 };
 
 class IdDynMatcher : public DynMatcherInterface {
- public:
+public:
   IdDynMatcher(StringRef ID,
-   const IntrusiveRefCntPtr &InnerMatcher)
-  : ID(ID), InnerMatcher(InnerMatcher) {}
+   IntrusiveRefCntPtr InnerMatcher)
+  : ID(ID), InnerMatcher(std::move(InnerMatcher)) {}
 
   bool dynMatches(const ast_type_traits::DynTypedNode &DynNode,
   ASTMatchFinder *Finder,
@@ -85,7 +85,7 @@ class IdDynMatcher : public DynMatcherIn
 return Result;
   }
 
- private:
+private:
   const std::string ID;
   const IntrusiveRefCntPtr InnerMatcher;
 };
@@ -210,8 +210,9 @@ bool DynTypedMatcher::matchesNoKindCheck
 llvm::Optional DynTypedMatcher::tryBind(StringRef ID) const {
   if (!AllowBind) return llvm::None;
   auto Result = *this;
-  Result.Implementation = new IdDynMatcher(ID, Result.Implementation);
-  return Result;
+  Result.Implementation =
+  new IdDynMatcher(ID, std::move(Result.Implementation));
+  return std::move(Result);
 }
 
 bool DynTypedMatcher::canConvertTo(ast_type_traits::ASTNodeKind To) const {

Modified: cfe/trunk/lib/Basic/Diagnostic.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Diagnostic.cpp?rev=276292&r1=276291&r2=276292&view=diff
==
--- cfe/trunk/lib/Basic/Diagnostic.cpp (original)
+++ cfe/trunk/lib/Basic/Diagnostic.cpp Thu Jul 21 10:06:51 2016
@@ -55,10 +55,12 @@ static void DummyArgToStringFn(Diagnosti
   Output.append(Str.begin(), Str.end());
 }
 
-DiagnosticsEngine::DiagnosticsEngine(
-const IntrusiveRefCntPtr &diags, DiagnosticOptions 
*DiagOpts,
-DiagnosticConsumer *client, bool ShouldOwnClient)
-: Diags(diags), DiagOpts(DiagOpts), Client(nullptr), SourceMgr(nullptr) {
+DiagnosticsEngine::DiagnosticsEngine(IntrusiveRefCntPtr diags,
+ DiagnosticOptions *DiagOpts,
+ DiagnosticConsumer *client,
+ bool ShouldOwnClient)
+: Diags(std::move(diags)), DiagOpts(DiagOpts), Client(nullptr),
+  SourceMgr(nullptr) {
   setClient(client, ShouldOwnClient);
   ArgToStringFn = DummyArgToStringFn;
   ArgToStringCookie = nullptr;


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

Re: [PATCH] D22513: [clang-tidy] add check cppcoreguidelines-rule-of-five-and-zero

2016-07-21 Thread Aaron Ballman via cfe-commits

aaron.ballman added inline comments.


Comment at: clang-tidy/cppcoreguidelines/RuleOfFiveAndZeroCheck.cpp:63
@@ +62,3 @@
+  if (const auto *MatchedDecl = Result.Nodes.getNodeAs(Match)) {
+
+diag(MatchedDecl->getLocation(), "class %0 defines a %1 but does not "

No newline.


Comment at: clang-tidy/cppcoreguidelines/RuleOfFiveAndZeroCheck.cpp:72
@@ +71,3 @@
+void RuleOfFiveAndZeroCheck::check(const MatchFinder::MatchResult &Result) {
+
+  checkRuleOfFiveViolation(Result, "dtor", "destructor");

No newline.


Comment at: clang-tidy/cppcoreguidelines/RuleOfFiveAndZeroCheck.cpp:73-77
@@ +72,7 @@
+
+  checkRuleOfFiveViolation(Result, "dtor", "destructor");
+  checkRuleOfFiveViolation(Result, "copy-ctor", "copy constructor");
+  checkRuleOfFiveViolation(Result, "copy-assign", "copy assignment operator");
+  checkRuleOfFiveViolation(Result, "move-ctor", "move constructor");
+  checkRuleOfFiveViolation(Result, "move-assign", "move assignment operator");
+}

Prazek wrote:
> I think it would be much better to aggregate the diagnosics.
> E.g. if I declare 4 special functions then I will get 4 lines of warnings, 
> and I won't even know what function did I forgot to declare.
> 
> So it should be better to fire diag on the first, or just one of the special 
> function and say "class %0 defines {{list_here}} but doesn't define 
> {{other_list}}"
I tend to agree -- this will get very chatty if I define 4 out of the 5 special 
member functions, so it might be best if the diagnostic instead reads:

"class %0 defines %1, but does not define %2"

Where %1 is the list of things the class defines and %2 is the list of things 
the class fails to define.

Note, "define or delete" is a bit weird because a deleted function is a 
definition, just like an explicitly-defaulted function is a definition. It 
seems strange to mention delete but not default, so I just went with "define" 
by itself for brevity.


Comment at: clang-tidy/cppcoreguidelines/RuleOfFiveAndZeroCheck.h:19
@@ +18,3 @@
+
+/// Checks for classes where some, but not all, of the special member functions
+/// are defined.

Prazek wrote:
> no comma after all. But I might be wrong because I am not certified grammar 
> nazi
The comma is correct there. :-)


Comment at: test/clang-tidy/cppcoreguidelines-rule-of-five-and-zero.cpp:44
@@ +43,2 @@
+  ~DeletesEverything();
+};

I'd also like to see a test case with mixed =default and =delete (to ensure we 
do not trigger when explicitly defaulting or deleting the some special member 
functions).


Repository:
  rL LLVM

https://reviews.llvm.org/D22513



___
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-07-21 Thread Aaron Ballman via cfe-commits

aaron.ballman added a subscriber: aaron.ballman.
aaron.ballman added a reviewer: aaron.ballman.


Comment at: clang-tidy/misc/MoveForwardingReferenceCheck.cpp:20
@@ +19,3 @@
+
+static void ReplaceMoveWithForward(const UnresolvedLookupExpr *Callee,
+   const TemplateTypeParmType *TypeParmType,

Might as well pass `Context` by const reference rather than pointer.


Comment at: clang-tidy/misc/MoveForwardingReferenceCheck.cpp:43-53
@@ +42,13 @@
+// another namespace).
+const StringRef OriginalText =
+Lexer::getSourceText(CallRange, SM, LangOpts);
+if (OriginalText == "::std::move") {
+  Diag << FixItHint::CreateReplacement(CallRange, "::std::" + ForwardName);
+  // If the original text was simply "move", we conservatively still put a
+  // "std::" in front of the "forward". Rationale: Even if the code
+  // apparently had a "using std::move;", that doesn't tell us whether it
+  // also had a "using std::forward;".
+} else if (OriginalText == "std::move" || OriginalText == "move") {
+  Diag << FixItHint::CreateReplacement(CallRange, "std::" + ForwardName);
+}
+  }

I'm not certain I understand the benefit to this. We know it's a call to 
std::move() from the standard namespace already, so why do we care about the 
original text? I think it's reasonable to replace any call to move() from the 
standard namespace with `::std::forward()`, so we should be able to remove the 
if statements and not have to go read the original source text from the source 
manager (which could involve, for instance, a query for that text over a slow 
network).


Comment at: clang-tidy/misc/MoveForwardingReferenceCheck.cpp:92
@@ +91,3 @@
+  Finder->addMatcher(
+  callExpr(callee(unresolvedLookupExpr().bind("lookup")),
+   argumentCountIs(1),

It might be a bit more clear if you made `isStdMove()` into a local AST matcher 
and called it from here.


Comment at: clang-tidy/misc/MoveForwardingReferenceCheck.cpp:119
@@ +118,3 @@
+
+  auto Diag = diag(FileMoveRange.getBegin(),
+   "forwarding reference passed to std::move(); did you mean "

Is there a reason why you can't just use `CallMove->getExprLoc()` instead of 
toying with character ranges?


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] r276282 - [clang-tidy] Avoid duplicated DenseMap lookup.

2016-07-21 Thread Benjamin Kramer via cfe-commits

Author: d0k
Date: Thu Jul 21 09:13:45 2016
New Revision: 276282

URL: http://llvm.org/viewvc/llvm-project?rev=276282&view=rev
Log:
[clang-tidy] Avoid duplicated DenseMap lookup.

The std::string is still constructed on demand. No functionality change
intended.

Modified:
clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp

Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp?rev=276282&r1=276281&r2=276282&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp Thu Jul 
21 09:13:45 2016
@@ -176,8 +176,7 @@ DiagnosticBuilder ClangTidyContext::diag
   assert(Loc.isValid());
   unsigned ID = DiagEngine->getDiagnosticIDs()->getCustomDiagID(
   Level, (Description + " [" + CheckName + "]").str());
-  if (CheckNamesByDiagnosticID.count(ID) == 0)
-CheckNamesByDiagnosticID.insert(std::make_pair(ID, CheckName.str()));
+  CheckNamesByDiagnosticID.try_emplace(ID, CheckName);
   return DiagEngine->Report(Loc, ID);
 }
 


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

Re: [PATCH] D22557: [libcxx] Diagnose invalid memory order arguments in . Fixes PR21179.

2016-07-21 Thread Ben Craig via cfe-commits

bcraig added inline comments.


Comment at: include/atomic:569
@@ +568,3 @@
+__attribute__ ((__enable_if__(__m == memory_order_release \
+   || __m == memory_order_acq_rel, "")))  \
+__attribute__ ((__unavailable__("memory order argument to atomic operation 
is invalid")))

Gah, read the logic backwards... and even backwards my comments are incomplete.


https://reviews.llvm.org/D22557



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

Re: [PATCH] D18639: Use __builtin_isnan/isinf/isfinite in complex

2016-07-21 Thread Marshall Clow via cfe-commits

mclow.lists added a comment.

And is there any reason why `__libcpp_isinf` can't just return `false` for 
non-fp types?


https://reviews.llvm.org/D18639



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

Re: [PATCH] D18639: Use __builtin_isnan/isinf/isfinite in complex

2016-07-21 Thread Marshall Clow via cfe-commits

mclow.lists added a comment.

Maybe something like this? (untested)

  template 
  _LIBCPP_ALWAYS_INLINE
  typename std::enable_if::value, bool>::type
  __libcpp_isfinite(_A1 __lcpp_x) _NOEXCEPT
  {
  return isfinite(__lcpp_x);
  }
  
  template 
  _LIBCPP_ALWAYS_INLINE
  typename std::enable_if::value, bool>::type
  __libcpp_isfinite(_A1 __lcpp_x) _NOEXCEPT
  {
  #if __has_builtin(__builtin_isfinite)
  return __builtin_isfinite(__lcpp_x);
  #else
  return isfinite(__lcpp_x);
  #endif
  }


https://reviews.llvm.org/D18639



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

Re: [PATCH] D22567: [include-fixer] Add mising qualifiers to all instances of an unidentified symbol.

2016-07-21 Thread Haojian Wu via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL276280: [include-fixer] Add mising qualifiers to all 
instances of an unidentified… (authored by hokein).

Changed prior to commit:
  https://reviews.llvm.org/D22567?vs=64849&id=64877#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D22567

Files:
  clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
  clang-tools-extra/trunk/include-fixer/IncludeFixer.h
  clang-tools-extra/trunk/include-fixer/IncludeFixerContext.cpp
  clang-tools-extra/trunk/include-fixer/IncludeFixerContext.h
  clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
  clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.py
  clang-tools-extra/trunk/test/include-fixer/commandline_options.cpp
  clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp

Index: clang-tools-extra/trunk/test/include-fixer/commandline_options.cpp
===
--- clang-tools-extra/trunk/test/include-fixer/commandline_options.cpp
+++ clang-tools-extra/trunk/test/include-fixer/commandline_options.cpp
@@ -1,9 +1,9 @@
 // REQUIRES: shell
 // RUN: echo "foo f;" > %t.cpp
 // RUN: clang-include-fixer -db=fixed -input='foo= "foo.h","bar.h"' -output-headers %t.cpp -- | FileCheck %s
-// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{QuerySymbolInfo: {RawIdentifier: foo, Range: {Offset: 0, Length: 3}}, HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"}]}' %t.cpp | FileCheck %s -check-prefix=CHECK-CODE
-// RUN: cat %t.cpp | not clang-include-fixer -stdin -insert-header='{QuerySymbolInfo: {RawIdentifier: foo, Range: {Offset: 0, Length: 3}}, HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"},{Header: "\"foo2.h\"", QualifiedName: "foo"}]}' %t.cpp
-// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{QuerySymbolInfo: {RawIdentifier: foo, Range: {Offset: 0, Length: 3}}, HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "a:foo"},{Header: "\"foo.h\"", QualifiedName: "b:foo"}]}' %t.cpp
+// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"}]}' %t.cpp | FileCheck %s -check-prefix=CHECK-CODE
+// RUN: cat %t.cpp | not clang-include-fixer -stdin -insert-header='{QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"},{Header: "\"foo2.h\"", QualifiedName: "foo"}]}' %t.cpp
+// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "a:foo"},{Header: "\"foo.h\"", QualifiedName: "b:foo"}]}' %t.cpp
 //
 // CHECK: "HeaderInfos": [
 // CHECK-NEXT:  {"Header": "\"foo.h\"",
Index: clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp
===
--- clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp
+++ clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp
@@ -89,17 +89,14 @@
   runOnCode(&Factory, Code, FakeFileName, ExtraArgs);
   if (FixerContext.getHeaderInfos().empty())
 return Code;
-  auto Replaces = clang::include_fixer::createInsertHeaderReplacements(
-  Code, FakeFileName, FixerContext.getHeaderInfos().front().Header);
+  auto Replaces = clang::include_fixer::createIncludeFixerReplacements(
+  Code, FakeFileName, FixerContext);
   EXPECT_TRUE(static_cast(Replaces))
   << llvm::toString(Replaces.takeError()) << "\n";
   if (!Replaces)
 return "";
   clang::RewriterTestContext Context;
   clang::FileID ID = Context.createInMemoryFile(FakeFileName, Code);
-  Replaces->insert({FakeFileName, FixerContext.getSymbolRange().getOffset(),
-FixerContext.getSymbolRange().getLength(),
-FixerContext.getHeaderInfos().front().QualifiedName});
   clang::tooling::applyAllReplacements(*Replaces, Context.Rewrite);
   return Context.getRewrittenText(ID);
 }
@@ -211,12 +208,12 @@
   std::string Code = "#include \"a.h\"\n"
  "#include \"foo.h\"\n"
  "\n"
- "namespace a { b::bar b; }";
+ "namespace a {\nb::bar b;\n}\n";
   std::string Expected = "#include \"a.h\"\n"
  "#include \"bar.h\"\n"
  "#include \"foo.h\"\n"
  "\n"
- "namespace a { b::bar b; }";
+ "namespace a {\nb::bar b;\n}\n";
   EXPECT_EQ(Expected, runIncludeFixer(Code));
 }
 
@@ -275,6 +272,69 @@
 runIncludeFixer("namespace a {\n::a::b::bar b;\n}\n"));
 }
 
+TEST(IncludeFixer, FixNamespaceQualifiersForAllInstances) {
+  const char TestCode[] = R"(
+namespace a {
+bar b;
+int func1() {
+  bar a;
+

[clang-tools-extra] r276280 - [include-fixer] Add mising qualifiers to all instances of an unidentified symbol.

2016-07-21 Thread Haojian Wu via cfe-commits

Author: hokein
Date: Thu Jul 21 08:47:09 2016
New Revision: 276280

URL: http://llvm.org/viewvc/llvm-project?rev=276280&view=rev
Log:
[include-fixer] Add mising qualifiers to all instances of an unidentified 
symbol.

Reviewers: bkramer

Subscribers: ioeric, cfe-commits

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

Modified:
clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
clang-tools-extra/trunk/include-fixer/IncludeFixer.h
clang-tools-extra/trunk/include-fixer/IncludeFixerContext.cpp
clang-tools-extra/trunk/include-fixer/IncludeFixerContext.h
clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.py
clang-tools-extra/trunk/test/include-fixer/commandline_options.cpp
clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp

Modified: clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp?rev=276280&r1=276279&r2=276280&view=diff
==
--- clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp (original)
+++ clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp Thu Jul 21 08:47:09 
2016
@@ -227,17 +227,28 @@ public:
 Symbol.getContexts(),
 Symbol.getNumOccurrences());
 }
-return IncludeFixerContext(QuerySymbolInfo, SymbolCandidates);
+return IncludeFixerContext(QuerySymbolInfos, SymbolCandidates);
   }
 
 private:
   /// Query the database for a given identifier.
-  bool query(StringRef Query, StringRef ScopedQualifiers, tooling::Range 
Range) {
+  bool query(StringRef Query, StringRef ScopedQualifiers,
+ tooling::Range Range) {
 assert(!Query.empty() && "Empty query!");
 
-// Skip other identifiers once we have discovered an identfier 
successfully.
-if (!MatchedSymbols.empty())
+// Save all instances of an unidentified symbol.
+//
+// We use conservative behavior for detecting the same unidentified symbol
+// here. The symbols which have the same ScopedQualifier and RawIdentifier
+// are considered equal. So that include-fixer avoids false positives, and
+// always adds missing qualifiers to correct symbols.
+if (!QuerySymbolInfos.empty()) {
+  if (ScopedQualifiers == QuerySymbolInfos.front().ScopedQualifiers &&
+  Query == QuerySymbolInfos.front().RawIdentifier) {
+QuerySymbolInfos.push_back({Query.str(), ScopedQualifiers, Range});
+  }
   return false;
+}
 
 DEBUG(llvm::dbgs() << "Looking up '" << Query << "' at ");
 DEBUG(getCompilerInstance()
@@ -248,7 +259,7 @@ private:
   .print(llvm::dbgs(), getCompilerInstance().getSourceManager()));
 DEBUG(llvm::dbgs() << " ...");
 
-QuerySymbolInfo = {Query.str(), ScopedQualifiers, Range};
+QuerySymbolInfos.push_back({Query.str(), ScopedQualifiers, Range});
 
 // Query the symbol based on C++ name Lookup rules.
 // Firstly, lookup the identifier with scoped namespace contexts;
@@ -274,8 +285,8 @@ private:
   /// The client to use to find cross-references.
   SymbolIndexManager &SymbolIndexMgr;
 
-  /// The symbol information.
-  IncludeFixerContext::QuerySymbolInfo QuerySymbolInfo;
+  /// The information of the symbols being queried.
+  std::vector QuerySymbolInfos;
 
   /// All symbol candidates which match QuerySymbol. We only include the first
   /// discovered identifier to avoid getting caught in results from error
@@ -332,13 +343,13 @@ bool IncludeFixerActionFactory::runInvoc
   return !Compiler.getDiagnostics().hasFatalErrorOccurred();
 }
 
-llvm::Expected
-createInsertHeaderReplacements(StringRef Code, StringRef FilePath,
-   StringRef Header,
-   const clang::format::FormatStyle &Style) {
-  if (Header.empty())
+llvm::Expected createIncludeFixerReplacements(
+StringRef Code, StringRef FilePath, const IncludeFixerContext &Context,
+const clang::format::FormatStyle &Style, bool AddQualifiers) {
+  if (Context.getHeaderInfos().empty())
 return tooling::Replacements();
-  std::string IncludeName = "#include " + Header.str() + "\n";
+  std::string IncludeName =
+  "#include " + Context.getHeaderInfos().front().Header + "\n";
   // Create replacements for the new header.
   clang::tooling::Replacements Insertions = {
   tooling::Replacement(FilePath, UINT_MAX, 0, IncludeName)};
@@ -346,6 +357,14 @@ createInsertHeaderReplacements(StringRef
   auto CleanReplaces = cleanupAroundReplacements(Code, Insertions, Style);
   if (!CleanReplaces)
 return CleanReplaces;
+
+  if (AddQualifiers) {
+for (const auto &Info : Context.getQuerySymbolInfos()) {
+  CleanReplaces->insert({FilePath, Info.Range.getOffset(),
+ Info.Range.getLength(),
+

r276279 - Revert "Include unreferenced nested types in member list only for CodeView"

2016-07-21 Thread Adrian McCarthy via cfe-commits

Author: amccarth
Date: Thu Jul 21 08:41:25 2016
New Revision: 276279

URL: http://llvm.org/viewvc/llvm-project?rev=276279&view=rev
Log:
Revert "Include unreferenced nested types in member list only for CodeView"

Patch broke ModuleDebugInfo test on the build bots (but not locally).  Again.

svn revision:  r276271

This reverts commit 9da8a1b05362bc96f2855fb32b5588b89407685d.

Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.h
cfe/trunk/test/CodeGenCXX/debug-info-dup-fwd-decl.cpp
cfe/trunk/test/CodeGenCXX/debug-info-ms-abi.cpp
cfe/trunk/test/Modules/ModuleDebugInfo.cpp

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=276279&r1=276278&r2=276279&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Thu Jul 21 08:41:25 2016
@@ -1090,14 +1090,6 @@ void CGDebugInfo::CollectRecordNormalFie
   elements.push_back(FieldType);
 }
 
-void CGDebugInfo::CollectRecordNestedRecord(
-const RecordDecl *RD, SmallVectorImpl &elements) {
-  QualType Ty = CGM.getContext().getTypeDeclType(RD);
-  SourceLocation Loc = RD->getLocation();
-  llvm::DIType *nestedType = getOrCreateType(Ty, getOrCreateFile(Loc));
-  elements.push_back(nestedType);
-}
-
 void CGDebugInfo::CollectRecordFields(
 const RecordDecl *record, llvm::DIFile *tunit,
 SmallVectorImpl &elements,
@@ -1109,10 +1101,6 @@ void CGDebugInfo::CollectRecordFields(
   else {
 const ASTRecordLayout &layout = 
CGM.getContext().getASTRecordLayout(record);
 
-// Debug info for nested records is included in the member list only for
-// CodeView.
-bool IncludeNestedRecords = CGM.getCodeGenOpts().EmitCodeView;
-
 // Field number for non-static fields.
 unsigned fieldNo = 0;
 
@@ -1138,10 +1126,7 @@ void CGDebugInfo::CollectRecordFields(
 
 // Bump field number for next field.
 ++fieldNo;
-  } else if (const auto *nestedRec = dyn_cast(I))
-if (IncludeNestedRecords && !nestedRec->isImplicit() &&
-nestedRec->getDeclContext() == record)
-  CollectRecordNestedRecord(nestedRec, elements);
+  }
   }
 }
 
@@ -3635,8 +3620,8 @@ void CGDebugInfo::EmitUsingDirective(con
   if (CGM.getCodeGenOpts().getDebugInfo() < codegenoptions::LimitedDebugInfo)
 return;
   const NamespaceDecl *NSDecl = UD.getNominatedNamespace();
-  if (!NSDecl->isAnonymousNamespace() ||
-  CGM.getCodeGenOpts().DebugExplicitImport) {
+  if (!NSDecl->isAnonymousNamespace() || 
+  CGM.getCodeGenOpts().DebugExplicitImport) { 
 DBuilder.createImportedModule(
 getCurrentContextDescriptor(cast(UD.getDeclContext())),
 getOrCreateNameSpace(NSDecl),

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.h?rev=276279&r1=276278&r2=276279&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.h (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.h Thu Jul 21 08:41:25 2016
@@ -254,8 +254,6 @@ class CGDebugInfo {
 llvm::DIFile *F,
 SmallVectorImpl &E,
 llvm::DIType *RecordTy, const RecordDecl *RD);
-  void CollectRecordNestedRecord(const RecordDecl *RD,
- SmallVectorImpl &E);
   void CollectRecordFields(const RecordDecl *Decl, llvm::DIFile *F,
SmallVectorImpl &E,
llvm::DICompositeType *RecordTy);

Modified: cfe/trunk/test/CodeGenCXX/debug-info-dup-fwd-decl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-dup-fwd-decl.cpp?rev=276279&r1=276278&r2=276279&view=diff
==
--- cfe/trunk/test/CodeGenCXX/debug-info-dup-fwd-decl.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/debug-info-dup-fwd-decl.cpp Thu Jul 21 08:41:25 
2016
@@ -19,6 +19,6 @@ protected:
 
 Test t;
 
-// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "data"
 // CHECK: !DIDerivedType(tag: DW_TAG_pointer_type
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "data"
 // CHECK-NOT: !DICompositeType(tag: DW_TAG_structure_type, name: "data"

Modified: cfe/trunk/test/CodeGenCXX/debug-info-ms-abi.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-ms-abi.cpp?rev=276279&r1=276278&r2=276279&view=diff
==
--- cfe/trunk/test/CodeGenCXX/debug-info-ms-abi.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/debug-info-ms-abi.cpp Thu Jul 21 08:41:25 2016
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -triple=i686-pc-windows-msvc -debug-info-kind=limited 
-gcodeview -emit-llvm -o -

Re: [PATCH] D21459: Implement http://wg21.link/P0254R1: "Integrating std::string_view and std::string"

2016-07-21 Thread Marshall Clow via cfe-commits

mclow.lists added inline comments.


Comment at: include/string_view:216
@@ +215,3 @@
+   basic_string_view(const _CharT* __s)
+   : __data(__s), __size(_Traits::length(__s)) {}
+

kimgr wrote:
> I'm working from the paper at https://isocpp.org/files/papers/N3762.html, and 
> I find it a little sketchy on the policy for nullptrs.
> 
> Since the ctor above accepts nullptr as long as the length is zero, would it 
> make sense to do that here too? That is, only call _Traits::length for 
> non-nullptr __s args?
Reading from N4600: Requires: `[str, str + traits::length(str))` is a valid 
range.

So, no - passing `nullptr` here is undefined.



https://reviews.llvm.org/D21459



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

Re: [PATCH] D21070: Pass the ABI in the triple when appropriate (currently for MIPS) for 'clang -cc1' and 'clang -cc1as'

2016-07-21 Thread Daniel Sanders via cfe-commits

dsanders updated this revision to Diff 64870.
dsanders added a comment.

Refresh and ping


https://reviews.llvm.org/D21070

Files:
  lib/Basic/Targets.cpp
  lib/Driver/ToolChains.cpp
  lib/Driver/Tools.cpp
  lib/Frontend/CompilerInvocation.cpp
  tools/driver/cc1as_main.cpp

Index: tools/driver/cc1as_main.cpp
===
--- tools/driver/cc1as_main.cpp
+++ tools/driver/cc1as_main.cpp
@@ -69,6 +69,10 @@
   /// The name of the target triple to assemble for.
   std::string Triple;
 
+  /// The name of the ABI to assembler for or the empty string for the default
+  /// ABI.
+  std::string ABI;
+
   /// If given, the name of the target CPU to determine which instructions
   /// are legal.
   std::string CPU;
@@ -134,6 +138,7 @@
 public:
   AssemblerInvocation() {
 Triple = "";
+ABI = "";
 NoInitialTextSection = 0;
 InputFile = "-";
 OutputPath = "-";
@@ -185,13 +190,24 @@
 
   // Target Options
   Opts.Triple = llvm::Triple::normalize(Args.getLastArgValue(OPT_triple));
+  Opts.ABI = Args.getLastArgValue(OPT_target_abi);
   Opts.CPU = Args.getLastArgValue(OPT_target_cpu);
   Opts.Features = Args.getAllArgValues(OPT_target_feature);
 
   // Use the default target triple if unspecified.
   if (Opts.Triple.empty())
 Opts.Triple = llvm::sys::getDefaultTargetTriple();
 
+  // Modify the Triple and ABI according to the Triple and ABI.
+  llvm::Triple ABITriple;
+  StringRef ABIName;
+  std::tie(ABITriple, ABIName) =
+  llvm::Triple(Opts.Triple).getABIVariant(Opts.ABI);
+  if (ABITriple.getArch() == llvm::Triple::UnknownArch)
+Diags.Report(diag::err_target_unknown_abi) << Opts.ABI;
+  Opts.Triple = ABITriple.str();
+  Opts.ABI = ABIName;
+
   // Language Options
   Opts.IncludePaths = Args.getAllArgValues(OPT_I);
   Opts.NoInitialTextSection = Args.hasArg(OPT_n);
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2306,6 +2306,16 @@
   // Use the default target triple if unspecified.
   if (Opts.Triple.empty())
 Opts.Triple = llvm::sys::getDefaultTargetTriple();
+
+  // Modify the Triple and ABI according to the Triple and ABI.
+  llvm::Triple ABITriple;
+  StringRef ABIName;
+  std::tie(ABITriple, ABIName) =
+  llvm::Triple(Opts.Triple).getABIVariant(Opts.ABI);
+  if (ABITriple.getArch() == llvm::Triple::UnknownArch)
+Diags.Report(diag::err_target_unknown_abi) << Opts.ABI;
+  Opts.Triple = ABITriple.str();
+  Opts.ABI = ABIName;
 }
 
 bool CompilerInvocation::CreateFromArgs(CompilerInvocation &Res,
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -1249,7 +1249,7 @@
   // MIPS32r6 is the default for mips(el)?-img-linux-gnu and MIPS64r6 is the
   // default for mips64(el)?-img-linux-gnu.
   if (Triple.getVendor() == llvm::Triple::ImaginationTechnologies &&
-  Triple.getEnvironment() == llvm::Triple::GNU) {
+  Triple.isGNUEnvironment()) {
 DefMips32CPU = "mips32r6";
 DefMips64CPU = "mips64r6";
   }
Index: lib/Driver/ToolChains.cpp
===
--- lib/Driver/ToolChains.cpp
+++ lib/Driver/ToolChains.cpp
@@ -2404,7 +2404,7 @@
 
   if (TargetTriple.getVendor() == llvm::Triple::ImaginationTechnologies &&
   TargetTriple.getOS() == llvm::Triple::Linux &&
-  TargetTriple.getEnvironment() == llvm::Triple::GNU)
+  TargetTriple.isGNUEnvironment())
 return findMipsImgMultilibs(Flags, NonExistent, Result);
 
   if (findMipsCsMultilibs(Flags, NonExistent, Result))
Index: lib/Basic/Targets.cpp
===
--- lib/Basic/Targets.cpp
+++ lib/Basic/Targets.cpp
@@ -7145,10 +7145,22 @@
 BigEndian = getTriple().getArch() == llvm::Triple::mips ||
 getTriple().getArch() == llvm::Triple::mips64;
 
-setABI((getTriple().getArch() == llvm::Triple::mips ||
-getTriple().getArch() == llvm::Triple::mipsel)
-   ? "o32"
-   : "n64");
+if (getTriple().getEnvironment() == llvm::Triple::ABI32 ||
+getTriple().getEnvironment() == llvm::Triple::GNUABI32 ||
+getTriple().getEnvironment() == llvm::Triple::AndroidABI32)
+  setABI("o32");
+else if (getTriple().getEnvironment() == llvm::Triple::ABIN32 ||
+getTriple().getEnvironment() == llvm::Triple::GNUABIN32)
+  setABI("n32");
+else if (getTriple().getEnvironment() == llvm::Triple::ABI64 ||
+ getTriple().getEnvironment() == llvm::Triple::GNUABI64 ||
+ getTriple().getEnvironment() == llvm::Triple::AndroidABI64)
+  setABI("n64");
+else
+  setABI((getTriple().getArch() == llvm::Triple::mips ||
+  getTriple().getArch() == llvm::Triple::mipsel)
+ ? "o32"
+ : "

[libcxx] r276273 - Again, w/o the tabs

2016-07-21 Thread Marshall Clow via cfe-commits

Author: marshall
Date: Thu Jul 21 08:19:36 2016
New Revision: 276273

URL: http://llvm.org/viewvc/llvm-project?rev=276273&view=rev
Log:
Again, w/o the tabs

Modified:
libcxx/trunk/test/std/strings/string.view/string.view.ops/substr.pass.cpp

Modified: 
libcxx/trunk/test/std/strings/string.view/string.view.ops/substr.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/strings/string.view/string.view.ops/substr.pass.cpp?rev=276273&r1=276272&r2=276273&view=diff
==
--- libcxx/trunk/test/std/strings/string.view/string.view.ops/substr.pass.cpp 
(original)
+++ libcxx/trunk/test/std/strings/string.view/string.view.ops/substr.pass.cpp 
Thu Jul 21 08:19:36 2016
@@ -22,7 +22,7 @@
 
 template
 void test1(std::basic_string_view sv, size_t n, size_t pos) {
-   std::basic_string_view sv1;
+std::basic_string_view sv1;
 #ifdef TEST_HAS_NO_EXCEPTIONS
 if (pos > sv.size())
 return ;  // would throw if exceptions were enabled
@@ -37,10 +37,10 @@ void test1(std::basic_string_view
 return ;
 }
 #endif
-   const size_t rlen = std::min(n, sv.size() - pos);
-   assert (sv1.size() == rlen);
-   for (size_t i = 0; i <= rlen; ++i)
-   assert(sv[pos+i] == sv1[i]);
+const size_t rlen = std::min(n, sv.size() - pos);
+assert (sv1.size() == rlen);
+for (size_t i = 0; i <= rlen; ++i)
+assert(sv[pos+i] == sv1[i]);
 }
 
 


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

[libcxx] r276272 - Another fix to appease the no-exception bots.

2016-07-21 Thread Marshall Clow via cfe-commits

Author: marshall
Date: Thu Jul 21 08:18:50 2016
New Revision: 276272

URL: http://llvm.org/viewvc/llvm-project?rev=276272&view=rev
Log:
Another fix to appease the no-exception bots.

Modified:
libcxx/trunk/test/std/strings/string.view/string.view.ops/substr.pass.cpp

Modified: 
libcxx/trunk/test/std/strings/string.view/string.view.ops/substr.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/strings/string.view/string.view.ops/substr.pass.cpp?rev=276272&r1=276271&r2=276272&view=diff
==
--- libcxx/trunk/test/std/strings/string.view/string.view.ops/substr.pass.cpp 
(original)
+++ libcxx/trunk/test/std/strings/string.view/string.view.ops/substr.pass.cpp 
Thu Jul 21 08:18:50 2016
@@ -22,21 +22,25 @@
 
 template
 void test1(std::basic_string_view sv, size_t n, size_t pos) {
+   std::basic_string_view sv1;
 #ifdef TEST_HAS_NO_EXCEPTIONS
-if (pos <= sv.size())
-assert (sign( sv1.compare(pos1, n1, sv2)) == sign(expected));
+if (pos > sv.size())
+return ;  // would throw if exceptions were enabled
+sv1 = sv.substr(pos, n);
 #else
 try {
-std::basic_string_view sv1 = sv.substr(pos, n);
-const size_t rlen = std::min(n, sv.size() - pos);
-assert (sv1.size() == rlen);
-for (size_t i = 0; i <= rlen; ++i)
-assert(sv[pos+i] == sv1[i]);
+sv1 = sv.substr(pos, n);
+assert(pos <= sv.size());
 }
 catch (const std::out_of_range&) {
 assert(pos > sv.size());
+return ;
 }
 #endif
+   const size_t rlen = std::min(n, sv.size() - pos);
+   assert (sv1.size() == rlen);
+   for (size_t i = 0; i <= rlen; ++i)
+   assert(sv[pos+i] == sv1[i]);
 }
 
 


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

r276271 - Include unreferenced nested types in member list only for CodeView

2016-07-21 Thread Adrian McCarthy via cfe-commits

Author: amccarth
Date: Thu Jul 21 08:16:14 2016
New Revision: 276271

URL: http://llvm.org/viewvc/llvm-project?rev=276271&view=rev
Log:
Include unreferenced nested types in member list only for CodeView

Unreferenced nested structs and classes were omitted from the debug info.  In 
DWARF, this was intentional, to avoid bloat.  But for CodeView, we want this 
information to be consistent with what Microsoft tools would produce and expect.

Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.h
cfe/trunk/test/CodeGenCXX/debug-info-dup-fwd-decl.cpp
cfe/trunk/test/CodeGenCXX/debug-info-ms-abi.cpp
cfe/trunk/test/Modules/ModuleDebugInfo.cpp

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=276271&r1=276270&r2=276271&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Thu Jul 21 08:16:14 2016
@@ -1090,6 +1090,14 @@ void CGDebugInfo::CollectRecordNormalFie
   elements.push_back(FieldType);
 }
 
+void CGDebugInfo::CollectRecordNestedRecord(
+const RecordDecl *RD, SmallVectorImpl &elements) {
+  QualType Ty = CGM.getContext().getTypeDeclType(RD);
+  SourceLocation Loc = RD->getLocation();
+  llvm::DIType *nestedType = getOrCreateType(Ty, getOrCreateFile(Loc));
+  elements.push_back(nestedType);
+}
+
 void CGDebugInfo::CollectRecordFields(
 const RecordDecl *record, llvm::DIFile *tunit,
 SmallVectorImpl &elements,
@@ -1101,6 +1109,10 @@ void CGDebugInfo::CollectRecordFields(
   else {
 const ASTRecordLayout &layout = 
CGM.getContext().getASTRecordLayout(record);
 
+// Debug info for nested records is included in the member list only for
+// CodeView.
+bool IncludeNestedRecords = CGM.getCodeGenOpts().EmitCodeView;
+
 // Field number for non-static fields.
 unsigned fieldNo = 0;
 
@@ -1126,7 +1138,10 @@ void CGDebugInfo::CollectRecordFields(
 
 // Bump field number for next field.
 ++fieldNo;
-  }
+  } else if (const auto *nestedRec = dyn_cast(I))
+if (IncludeNestedRecords && !nestedRec->isImplicit() &&
+nestedRec->getDeclContext() == record)
+  CollectRecordNestedRecord(nestedRec, elements);
   }
 }
 
@@ -3620,8 +3635,8 @@ void CGDebugInfo::EmitUsingDirective(con
   if (CGM.getCodeGenOpts().getDebugInfo() < codegenoptions::LimitedDebugInfo)
 return;
   const NamespaceDecl *NSDecl = UD.getNominatedNamespace();
-  if (!NSDecl->isAnonymousNamespace() || 
-  CGM.getCodeGenOpts().DebugExplicitImport) { 
+  if (!NSDecl->isAnonymousNamespace() ||
+  CGM.getCodeGenOpts().DebugExplicitImport) {
 DBuilder.createImportedModule(
 getCurrentContextDescriptor(cast(UD.getDeclContext())),
 getOrCreateNameSpace(NSDecl),

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.h?rev=276271&r1=276270&r2=276271&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.h (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.h Thu Jul 21 08:16:14 2016
@@ -254,6 +254,8 @@ class CGDebugInfo {
 llvm::DIFile *F,
 SmallVectorImpl &E,
 llvm::DIType *RecordTy, const RecordDecl *RD);
+  void CollectRecordNestedRecord(const RecordDecl *RD,
+ SmallVectorImpl &E);
   void CollectRecordFields(const RecordDecl *Decl, llvm::DIFile *F,
SmallVectorImpl &E,
llvm::DICompositeType *RecordTy);

Modified: cfe/trunk/test/CodeGenCXX/debug-info-dup-fwd-decl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-dup-fwd-decl.cpp?rev=276271&r1=276270&r2=276271&view=diff
==
--- cfe/trunk/test/CodeGenCXX/debug-info-dup-fwd-decl.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/debug-info-dup-fwd-decl.cpp Thu Jul 21 08:16:14 
2016
@@ -19,6 +19,6 @@ protected:
 
 Test t;
 
-// CHECK: !DIDerivedType(tag: DW_TAG_pointer_type
 // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "data"
+// CHECK: !DIDerivedType(tag: DW_TAG_pointer_type
 // CHECK-NOT: !DICompositeType(tag: DW_TAG_structure_type, name: "data"

Modified: cfe/trunk/test/CodeGenCXX/debug-info-ms-abi.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-ms-abi.cpp?rev=276271&r1=276270&r2=276271&view=diff
==
--- cfe/trunk/test/CodeGenCXX/debug-info-ms-abi.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/debug-info-ms-abi.cpp Thu Jul 21 08:16:14 2016
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -triple=i686-pc-windows-msvc -debug-

Re: [PATCH] D19311: [analyzer] Self Assignment Checker

2016-07-21 Thread Balogh , Ádám via cfe-commits

baloghadamsoftware removed rL LLVM as the repository for this revision.
baloghadamsoftware updated this revision to Diff 64864.
baloghadamsoftware added a comment.

Bug path string fixed.


https://reviews.llvm.org/D19311

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/CXXSelfAssignmentChecker.cpp
  lib/StaticAnalyzer/Core/BugReporter.cpp
  lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  test/Analysis/self-assign.cpp

Index: test/Analysis/self-assign.cpp
===
--- /dev/null
+++ test/Analysis/self-assign.cpp
@@ -0,0 +1,89 @@
+// RUN: %clang_cc1 -std=c++11 -analyze -analyzer-checker=core,cplusplus,unix.Malloc,debug.ExprInspection %s -verify -analyzer-output=text
+
+extern "C" char *strdup(const char* s);
+extern "C" void free(void* ptr);
+
+namespace std {
+template struct remove_reference  { typedef T type; };
+template struct remove_reference  { typedef T type; };
+template struct remove_reference { typedef T type; };
+template typename remove_reference::type&& move(T&& t);
+}
+
+void clang_analyzer_eval(int);
+
+class StringUsed {
+public:
+  StringUsed(const char *s = "") : str(strdup(s)) {}
+  StringUsed(const StringUsed &rhs) : str(strdup(rhs.str)) {}
+  ~StringUsed();
+  StringUsed& operator=(const StringUsed &rhs);
+  StringUsed& operator=(StringUsed &&rhs);
+  operator const char*() const;
+private:
+  char *str;
+};
+
+StringUsed::~StringUsed() {
+  free(str);
+}
+
+StringUsed& StringUsed::operator=(const StringUsed &rhs) { // expected-note{{Assuming rhs == *this}} expected-note{{Assuming rhs == *this}} expected-note{{Assuming rhs != *this}}
+  clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}} expected-warning{{UNKNOWN}} expected-note{{TRUE}} expected-note{{UNKNOWN}}
+  free(str); // expected-note{{Memory is released}}
+  str = strdup(rhs.str); // expected-warning{{Use of memory after it is freed}}  expected-note{{Use of memory after it is freed}}
+  return *this;
+}
+
+StringUsed& StringUsed::operator=(StringUsed &&rhs) { // expected-note{{Assuming rhs == *this}} expected-note{{Assuming rhs != *this}}
+  clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}} expected-warning{{UNKNOWN}} expected-note{{TRUE}} expected-note{{UNKNOWN}}
+  str = rhs.str;
+  rhs.str = nullptr; // FIXME: An improved leak checker should warn here
+  return *this;
+}
+
+StringUsed::operator const char*() const {
+  return str;
+}
+
+class StringUnused {
+public:
+  StringUnused(const char *s = "") : str(strdup(s)) {}
+  StringUnused(const StringUnused &rhs) : str(strdup(rhs.str)) {}
+  ~StringUnused();
+  StringUnused& operator=(const StringUnused &rhs);
+  StringUnused& operator=(StringUnused &&rhs);
+  operator const char*() const;
+private:
+  char *str;
+};
+
+StringUnused::~StringUnused() {
+  free(str);
+}
+
+StringUnused& StringUnused::operator=(const StringUnused &rhs) { // expected-note{{Assuming rhs == *this}} expected-note{{Assuming rhs == *this}} expected-note{{Assuming rhs != *this}}
+  clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}} expected-warning{{UNKNOWN}} expected-note{{TRUE}} expected-note{{UNKNOWN}}
+  free(str); // expected-note{{Memory is released}}
+  str = strdup(rhs.str); // expected-warning{{Use of memory after it is freed}}  expected-note{{Use of memory after it is freed}}
+  return *this;
+}
+
+StringUnused& StringUnused::operator=(StringUnused &&rhs) { // expected-note{{Assuming rhs == *this}} expected-note{{Assuming rhs != *this}}
+  clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}} expected-warning{{UNKNOWN}} expected-note{{TRUE}} expected-note{{UNKNOWN}}
+  str = rhs.str;
+  rhs.str = nullptr; // FIXME: An improved leak checker should warn here
+  return *this;
+}
+
+StringUnused::operator const char*() const {
+  return str;
+}
+
+
+int main() {
+  StringUsed s1 ("test"), s2;
+  s2 = s1;
+  s2 = std::move(s1);
+  return 0;
+}
Index: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -426,6 +426,13 @@
   //   Count naming convention errors more aggressively.
   if (isa(D))
 return false;
+  // We also want to reanalyze all C++ copy and move assignment operators to
+  // separately check the two cases where 'this' aliases with the parameter and
+  // where it may not. (cplusplus.SelfAssignmentChecker)
+  if (const auto *MD = dyn_cast(D)) {
+if (MD->isCopyAssignmentOperator() || MD->isMoveAssignmentOperator())
+  return false;
+  }
 
   // Otherwise, if we visited the function before, do not reanalyze it.
   return Visited.count(D);
@@ -437,9 +444,7 @@
   // We want to reanalyze all ObjC met

Re: [PATCH] D21814: clang-rename: split existing options into two new subcommands

2016-07-21 Thread Miklos Vajna via cfe-commits

vmiklos marked an inline comment as done.
vmiklos added a comment.

https://reviews.llvm.org/D21814



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

Re: [PATCH] D21814: clang-rename: split existing options into two new subcommands

2016-07-21 Thread Miklos Vajna via cfe-commits

vmiklos added a comment.

> The patch looks fine to me (though I'm not sure if there are no new tests; if

>  they are interface changes should be applied).


`make check-clang-tools` + the patch at r276098 passes for me at least. But any
pending test should be trivial to adapt.

> P.S. it seems logical to me to support `-offset` option in `-rename-all`,

>  too.


OK, I've added that, with a testcase.

> And introducing `-rename-all` without actually supporting multiple

>  renaming actions "at once" seems weird to me, too.


OK, I've squashed the original diff into this one, with a testcase, which
addresses this.

A nice side-effect is that now the option parser takes care of checking if
-new-name is provided, no need to have explicit code for that in clang-rename.

> use std::function here?


Done, also changed the `typedef` to `using`.


https://reviews.llvm.org/D21814



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

Re: [PATCH] D21814: clang-rename: split existing options into two new subcommands

2016-07-21 Thread Miklos Vajna via cfe-commits

vmiklos updated this revision to Diff 64859.

https://reviews.llvm.org/D21814

Files:
  clang-rename/RenamingAction.cpp
  clang-rename/RenamingAction.h
  clang-rename/tool/ClangRename.cpp
  clang-rename/tool/clang-rename.py
  docs/clang-rename.rst
  test/clang-rename/ClassFindByName.cpp
  test/clang-rename/ClassReplacements.cpp
  test/clang-rename/ClassSimpleRenaming.cpp
  test/clang-rename/ClassTestMulti.cpp
  test/clang-rename/ClassTestMultiByName.cpp
  test/clang-rename/ConstCastExpr.cpp
  test/clang-rename/ConstructExpr.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/Field.cpp
  test/clang-rename/FunctionMacro.cpp
  test/clang-rename/MemberExprMacro.cpp
  test/clang-rename/Namespace.cpp
  test/clang-rename/NoNewName.cpp
  test/clang-rename/ReinterpretCastExpr.cpp
  test/clang-rename/StaticCastExpr.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,5 +1,5 @@
 // RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=208 -new-name=Z %t.cpp -i --
+// RUN: clang-rename rename-at -offset=218 -new-name=Z %t.cpp -i --
 // RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
 
 #define Y X // CHECK: #define Y Z
Index: test/clang-rename/Variable.cpp
===
--- test/clang-rename/Variable.cpp
+++ test/clang-rename/Variable.cpp
@@ -1,5 +1,5 @@
 // RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=148 -new-name=Bar %t.cpp -i --
+// RUN: clang-rename rename-at -offset=158 -new-name=Bar %t.cpp -i --
 // RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
 
 namespace A {
Index: test/clang-rename/StaticCastExpr.cpp
===
--- test/clang-rename/StaticCastExpr.cpp
+++ test/clang-rename/StaticCastExpr.cpp
@@ -1,5 +1,5 @@
 // RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=152 -new-name=Bar %t.cpp -i --
+// RUN: clang-rename rename-at -offset=162 -new-name=Bar %t.cpp -i --
 // RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
 
 class Baz {
Index: test/clang-rename/ReinterpretCastExpr.cpp
===
--- test/clang-rename/ReinterpretCastExpr.cpp
+++ test/clang-rename/ReinterpretCastExpr.cpp
@@ -1,5 +1,5 @@
 // RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=133 -new-name=X %t.cpp -i --
+// RUN: clang-rename rename-at -offset=143 -new-name=X %t.cpp -i --
 // RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
 class Cla {
 public:
Index: test/clang-rename/NoNewName.cpp
===
--- test/clang-rename/NoNewName.cpp
+++ test/clang-rename/NoNewName.cpp
@@ -1,4 +1,4 @@
 // Check for an error while -new-name argument has not been passed to
 // clang-rename.
-// RUN: not clang-rename -offset=133 %s 2>&1 | FileCheck %s
-// CHECK: clang-rename: no new name provided.
+// RUN: not clang-rename rename-at -offset=133 %s 2>&1 | FileCheck %s
+// CHECK: clang-rename rename-at: for the -new-name option: must be specified
Index: test/clang-rename/Namespace.cpp
===
--- test/clang-rename/Namespace.cpp
+++ test/clang-rename/Namespace.cpp
@@ -1,5 +1,5 @@
 // RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=143 -new-name=llvm %t.cpp -i --
+// RUN: clang-rename rename-at -offset=153 -new-name=llvm %t.cpp -i --
 // RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
 
 namespace foo { // CHECK: namespace llvm {
Index: test/clang-rename/MemberExprMacro.cpp
===
--- test/clang-rename/MemberExprMacro.cpp
+++ test/clang-rename/MemberExprMacro.cpp
@@ -1,5 +1,5 @@
 // RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=156 -new-name=Bar %t.cpp -i --
+// RUN: clang-rename rename-at -offset=166 -new-name=Bar %t.cpp -i --
 // RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
 
 class Baz {
Index: test/clang-rename/FunctionMacro.cpp
===
--- test/clang-rename/FunctionMacro.cpp
+++ test/clang-rename/FunctionMacro.cpp
@@ -1,5 +1,5 @@
 // RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=199 -new-name=macro_function %t.cpp -i --
+// RUN: clang-rename rename-at -offset=209 -new-name=macro_function %t.cpp -i --
 // RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
 
 #define moo foo // CHECK: #define moo macro_function
Index: test/clang-rename/Field.cpp
===
--- test/clang-rename/Field.cpp
+++ test/clang-rename/Field.cpp
@@ -1,5 +1,5 @@
 // RUN: cat %s > %t.cpp
-//

Re: [PATCH] D22292: [libunwind] Fix unw_getcontext for ARMv6-m

2016-07-21 Thread Renato Golin via cfe-commits

rengolin added inline comments.


Comment at: src/UnwindRegistersRestore.S:325
@@ -324,4 +324,3 @@
 
DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind13Registers_arm20restoreCoreAndJumpToEv)
-#if !defined(__ARM_ARCH_ISA_ARM)
-  ldr r2, [r0, #52]
-  ldr r3, [r0, #60]
+#if !defined(__ARM_ARCH_ISA_ARM) && __ARM_ARCH_ISA_THUMB == 1
+  @ r8-r12: ldr into r1-r5, then mov to r8-r12

bcraig wrote:
> weimingz wrote:
> > originally, r8-r12 were not restored. Was that some existing bug?
> I'm not sure why r12 is getting messed with here.  It is intended as a 
> GOT/PLT scratch register.  r8+ isn't ubiquitously available in Thumb state 
> according to the ATPCS.  The AAPCS says that r4-r8, r10, r11, and SP must be 
> preserved.  r9 can optionally be preserved.
So, you're replacing a Thumb2+ARM implementation with a Thumb1+ARM one. This is 
all fine and dandy, but a Thumb2 implementation would be much smaller and 
cleaner (thus also faster).

Maybe we should have three implementations? 


https://reviews.llvm.org/D22292



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

Re: [PATCH] D22622: [analyzer] Add more info to exploded graph dumps

2016-07-21 Thread Artem Dergachev via cfe-commits

NoQ added a comment.

Whoops forgot the screenshots:

F2187578: 2.png  F2187579: 1.png 


Also not sure how to write tests for these, because we can't choose the 
directory path for the dump, can we?


https://reviews.llvm.org/D22622



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

1 2 >

1 - 100 of 119 matches

Mail list logo