[PATCH] D30214: [Driver] Search for libc++ headers in ResourceDir

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

In https://reviews.llvm.org/D30214#690010, @jroelofs wrote:

> libc++ headers should not be installed in the resource dir.


They are currently not by default. But with https://reviews.llvm.org/D30015 for 
runtime libraries, this may become a place for headers packaged with the 
compiler.


https://reviews.llvm.org/D30214



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

[PATCH] D30517: [libc++abi] Add option to enable definitions for the new/delete overloads.

[Mehdi AMINI via Phabricator via cfe-commits]

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

LGTM.




Comment at: CMakeLists.txt:416
+set(LIBCXXABI_HAS_UNDEFINED_SYMBOLS ((NOT 
LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS)
+OR (LIBCXXABI_BUILD_EXTERNAL_THREAD_LIBRARY AND LIBCXXABI_ENABLE_SHARED)))
+

It is unrelated to this change, but I'm wondering about the 
`LIBCXXABI_BUILD_EXTERNAL_THREAD_LIBRARY` condition. I see undefined reference 
to libc symbols.
That said we're linking to libSystem which provided everything, so I'm not even 
sure why `dynamic_lookup` is needed at all?


https://reviews.llvm.org/D30517



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

[PATCH] D30516: [libc++] Add option to disable new/delete overloads when libc++abi provides them.

[Mehdi AMINI via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D30516



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

[PATCH] D30514: [libc++abi] Update new/delete definitions to match libc++

[Mehdi AMINI via Phabricator via cfe-commits]

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

LGTM.

(It seems that having libc++ and libc++abi in the same repo would help sharing 
code like this)


https://reviews.llvm.org/D30514



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

[libcxx] r296732 - [libc++] Make _LIBCPP_TYPE_VIS export members

[Shoaib Meenai via cfe-commits]

Author: smeenai
Date: Wed Mar  1 21:22:18 2017
New Revision: 296732

URL: http://llvm.org/viewvc/llvm-project?rev=296732&view=rev
Log:
[libc++] Make _LIBCPP_TYPE_VIS export members

Summary:
Most classes annotated with _LIBCPP_TYPE_VIS need to have at least some
of their members exported, otherwise we have a lot of link errors when
linking against a libc++ built with hidden visibility. This also makes
_LIBCPP_TYPE_VIS be consistent across platforms, since on Windows it
already exports members.

With this change made, any template methods of a class marked
_LIBCPP_TYPE_VIS will also get default visibility when instantiatied,
which is not desirable for clients of libc++ headers who wish to control
their visibility; this is the same issue as PR30642. Annotate all
problematic methods with an explicit visibility specifier to avoid this.

The problematic methods were found by running bad-visibility-finder [1]
against the libc++ headers after making the _LIBCPP_TYPE_VIS change. The
small methods were marked for inlining; the larger ones hidden.

[1] https://github.com/smeenai/bad-visibility-finder

Reviewers: mclow.lists, EricWF

Subscribers: cfe-commits

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

Modified:
libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst
libcxx/trunk/include/__config
libcxx/trunk/include/__locale
libcxx/trunk/include/__mutex_base
libcxx/trunk/include/condition_variable
libcxx/trunk/include/future
libcxx/trunk/include/mutex
libcxx/trunk/include/shared_mutex
libcxx/trunk/include/thread

Modified: libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst?rev=296732&r1=296731&r2=296732&view=diff
==
--- libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst (original)
+++ libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst Wed Mar  1 21:22:18 2017
@@ -47,18 +47,17 @@ Visibility Macros
   A synonym for `_LIBCPP_INLINE_VISIBILITY`
 
 **_LIBCPP_TYPE_VIS**
+  Mark a type's typeinfo, vtable and members as having default visibility.
+  This attribute cannot be used on class templates.
+
+**_LIBCPP_TEMPLATE_VIS**
   Mark a type's typeinfo and vtable as having default visibility.
-  `_LIBCPP_TYPE_VIS`. This macro has no effect on the visibility of the
-  type's member functions. This attribute cannot be used on class templates.
+  This macro has no effect on the visibility of the type's member functions.
 
   **GCC Behavior**: GCC does not support Clang's `type_visibility(...)`
   attribute. With GCC the `visibility(...)` attribute is used and member
   functions are affected.
 
-**_LIBCPP_TEMPLATE_VIS**
-  The same as `_LIBCPP_TYPE_VIS` except that it may be applied to class
-  templates.
-
   **Windows Behavior**: DLLs do not support dllimport/export on class 
templates.
   The macro has an empty definition on this platform.
 

Modified: libcxx/trunk/include/__config
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__config?rev=296732&r1=296731&r2=296732&view=diff
==
--- libcxx/trunk/include/__config (original)
+++ libcxx/trunk/include/__config Wed Mar  1 21:22:18 2017
@@ -628,18 +628,22 @@ namespace std {
 
 #ifndef _LIBCPP_TYPE_VIS
 #  if !defined(_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS)
-#if __has_attribute(__type_visibility__)
-#  define _LIBCPP_TYPE_VIS __attribute__ ((__type_visibility__("default")))
-#else
-#  define _LIBCPP_TYPE_VIS __attribute__ ((__visibility__("default")))
-#endif
+#define _LIBCPP_TYPE_VIS __attribute__ ((__visibility__("default")))
 #  else
 #define _LIBCPP_TYPE_VIS
 #  endif
 #endif
 
 #ifndef _LIBCPP_TEMPLATE_VIS
-# define _LIBCPP_TEMPLATE_VIS _LIBCPP_TYPE_VIS
+#  if !defined(_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS)
+#if __has_attribute(__type_visibility__)
+#  define _LIBCPP_TEMPLATE_VIS __attribute__ 
((__type_visibility__("default")))
+#else
+#  define _LIBCPP_TEMPLATE_VIS __attribute__ ((__visibility__("default")))
+#endif
+#  else
+#define _LIBCPP_TEMPLATE_VIS
+#  endif
 #endif
 
 #ifndef _LIBCPP_FUNC_VIS_ONLY

Modified: libcxx/trunk/include/__locale
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__locale?rev=296732&r1=296731&r2=296732&view=diff
==
--- libcxx/trunk/include/__locale (original)
+++ libcxx/trunk/include/__locale Wed Mar  1 21:22:18 2017
@@ -92,13 +92,16 @@ public:
 
 const locale& operator=(const locale&)  _NOEXCEPT;
 
-template  locale combine(const locale&) const;
+template 
+  _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
+  locale combine(const locale&) const;
 
 // locale operations:
 string name() const;
 bool operator==(const locale&) const;
 bool operator!=(const locale& __y) const 

[PATCH] D25208: [libc++] Make _LIBCPP_TYPE_VIS export members

[Shoaib Meenai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL296732: [libc++] Make _LIBCPP_TYPE_VIS export members 
(authored by smeenai).

Changed prior to commit:
  https://reviews.llvm.org/D25208?vs=90275&id=90276#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D25208

Files:
  libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst
  libcxx/trunk/include/__config
  libcxx/trunk/include/__locale
  libcxx/trunk/include/__mutex_base
  libcxx/trunk/include/condition_variable
  libcxx/trunk/include/future
  libcxx/trunk/include/mutex
  libcxx/trunk/include/shared_mutex
  libcxx/trunk/include/thread

Index: libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst
===
--- libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst
+++ libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst
@@ -47,18 +47,17 @@
   A synonym for `_LIBCPP_INLINE_VISIBILITY`
 
 **_LIBCPP_TYPE_VIS**
+  Mark a type's typeinfo, vtable and members as having default visibility.
+  This attribute cannot be used on class templates.
+
+**_LIBCPP_TEMPLATE_VIS**
   Mark a type's typeinfo and vtable as having default visibility.
-  `_LIBCPP_TYPE_VIS`. This macro has no effect on the visibility of the
-  type's member functions. This attribute cannot be used on class templates.
+  This macro has no effect on the visibility of the type's member functions.
 
   **GCC Behavior**: GCC does not support Clang's `type_visibility(...)`
   attribute. With GCC the `visibility(...)` attribute is used and member
   functions are affected.
 
-**_LIBCPP_TEMPLATE_VIS**
-  The same as `_LIBCPP_TYPE_VIS` except that it may be applied to class
-  templates.
-
   **Windows Behavior**: DLLs do not support dllimport/export on class templates.
   The macro has an empty definition on this platform.
 
Index: libcxx/trunk/include/__locale
===
--- libcxx/trunk/include/__locale
+++ libcxx/trunk/include/__locale
@@ -92,13 +92,16 @@
 
 const locale& operator=(const locale&)  _NOEXCEPT;
 
-template  locale combine(const locale&) const;
+template 
+  _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
+  locale combine(const locale&) const;
 
 // locale operations:
 string name() const;
 bool operator==(const locale&) const;
 bool operator!=(const locale& __y) const {return !(*this == __y);}
 template 
+  _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
   bool operator()(const basic_string<_CharT, _Traits, _Allocator>&,
   const basic_string<_CharT, _Traits, _Allocator>&) const;
 
Index: libcxx/trunk/include/future
===
--- libcxx/trunk/include/future
+++ libcxx/trunk/include/future
@@ -582,6 +582,7 @@
 _LIBCPP_INLINE_VISIBILITY
 wait_for(const chrono::duration<_Rep, _Period>& __rel_time) const;
 template 
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
 future_status
 wait_until(const chrono::time_point<_Clock, _Duration>& __abs_time) const;
 
@@ -1674,6 +1675,7 @@
 public:
 promise();
 template 
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
 promise(allocator_arg_t, const _Allocator& __a);
 #ifndef _LIBCPP_HAS_NO_RVALUE_REFERENCES
 _LIBCPP_INLINE_VISIBILITY
Index: libcxx/trunk/include/condition_variable
===
--- libcxx/trunk/include/condition_variable
+++ libcxx/trunk/include/condition_variable
@@ -133,12 +133,14 @@
 void notify_all() _NOEXCEPT;
 
 template 
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
 void wait(_Lock& __lock);
 template 
 _LIBCPP_INLINE_VISIBILITY
 void wait(_Lock& __lock, _Predicate __pred);
 
 template 
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
 cv_status
 wait_until(_Lock& __lock,
const chrono::time_point<_Clock, _Duration>& __t);
Index: libcxx/trunk/include/__mutex_base
===
--- libcxx/trunk/include/__mutex_base
+++ libcxx/trunk/include/__mutex_base
@@ -316,20 +316,24 @@
 
 void wait(unique_lock& __lk) _NOEXCEPT;
 template 
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
 void wait(unique_lock& __lk, _Predicate __pred);
 
 template 
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
 cv_status
 wait_until(unique_lock& __lk,
const chrono::time_point<_Clock, _Duration>& __t);
 
 template 
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
 bool
 wait_until(unique_lock& __lk,
const chrono::time_point<_Clock, _Duration>& __t,
_Predicate __pred);
 
 template 
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
 cv_s

[PATCH] D25208: [libc++] Make _LIBCPP_TYPE_VIS export members

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai updated this revision to Diff 90275.
smeenai added a comment.

Rebase


https://reviews.llvm.org/D25208

Files:
  docs/DesignDocs/VisibilityMacros.rst
  include/__config
  include/__locale
  include/__mutex_base
  include/condition_variable
  include/future
  include/mutex
  include/shared_mutex
  include/thread

Index: include/thread
===
--- include/thread
+++ include/thread
@@ -298,9 +298,12 @@
!is_same::type, thread>::value
   >::type
  >
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
 explicit thread(_Fp&& __f, _Args&&... __args);
 #else  // _LIBCPP_HAS_NO_VARIADICS
-template  explicit thread(_Fp __f);
+template 
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
+explicit thread(_Fp __f);
 #endif
 ~thread();
 
Index: include/shared_mutex
===
--- include/shared_mutex
+++ include/shared_mutex
@@ -220,6 +220,7 @@
 return try_lock_until(chrono::steady_clock::now() + __rel_time);
 }
 template 
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
 bool
 try_lock_until(const chrono::time_point<_Clock, _Duration>& __abs_time);
 void unlock();
@@ -235,6 +236,7 @@
 return try_lock_shared_until(chrono::steady_clock::now() + __rel_time);
 }
 template 
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
 bool
 try_lock_shared_until(const chrono::time_point<_Clock, _Duration>& __abs_time);
 void unlock_shared();
Index: include/mutex
===
--- include/mutex
+++ include/mutex
@@ -248,6 +248,7 @@
 bool try_lock_for(const chrono::duration<_Rep, _Period>& __d)
 {return try_lock_until(chrono::steady_clock::now() + __d);}
 template 
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
 bool try_lock_until(const chrono::time_point<_Clock, _Duration>& __t);
 void unlock() _NOEXCEPT;
 };
@@ -291,6 +292,7 @@
 bool try_lock_for(const chrono::duration<_Rep, _Period>& __d)
 {return try_lock_until(chrono::steady_clock::now() + __d);}
 template 
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
 bool try_lock_until(const chrono::time_point<_Clock, _Duration>& __t);
 void unlock() _NOEXCEPT;
 };
Index: include/future
===
--- include/future
+++ include/future
@@ -582,6 +582,7 @@
 _LIBCPP_INLINE_VISIBILITY
 wait_for(const chrono::duration<_Rep, _Period>& __rel_time) const;
 template 
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
 future_status
 wait_until(const chrono::time_point<_Clock, _Duration>& __abs_time) const;
 
@@ -1674,6 +1675,7 @@
 public:
 promise();
 template 
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
 promise(allocator_arg_t, const _Allocator& __a);
 #ifndef _LIBCPP_HAS_NO_RVALUE_REFERENCES
 _LIBCPP_INLINE_VISIBILITY
Index: include/condition_variable
===
--- include/condition_variable
+++ include/condition_variable
@@ -133,12 +133,14 @@
 void notify_all() _NOEXCEPT;
 
 template 
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
 void wait(_Lock& __lock);
 template 
 _LIBCPP_INLINE_VISIBILITY
 void wait(_Lock& __lock, _Predicate __pred);
 
 template 
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
 cv_status
 wait_until(_Lock& __lock,
const chrono::time_point<_Clock, _Duration>& __t);
Index: include/__mutex_base
===
--- include/__mutex_base
+++ include/__mutex_base
@@ -316,20 +316,24 @@
 
 void wait(unique_lock& __lk) _NOEXCEPT;
 template 
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
 void wait(unique_lock& __lk, _Predicate __pred);
 
 template 
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
 cv_status
 wait_until(unique_lock& __lk,
const chrono::time_point<_Clock, _Duration>& __t);
 
 template 
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
 bool
 wait_until(unique_lock& __lk,
const chrono::time_point<_Clock, _Duration>& __t,
_Predicate __pred);
 
 template 
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
 cv_status
 wait_for(unique_lock& __lk,
  const chrono::duration<_Rep, _Period>& __d);
Index: include/__locale
===
--- include/__locale
+++ include/__locale
@@ -92,13 +92,16 @@
 
 const locale& operator=(const loc

[PATCH] D29157: [libc++] Make _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS export members

[Shoaib Meenai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL296731: [libc++] Make _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS 
export members (authored by smeenai).

Changed prior to commit:
  https://reviews.llvm.org/D29157?vs=90272&id=90274#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D29157

Files:
  libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst
  libcxx/trunk/include/__config
  libcxx/trunk/include/locale
  libcxx/trunk/include/string
  libcxx/trunk/lib/abi/CHANGELOG.TXT

Index: libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst
===
--- libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst
+++ libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst
@@ -110,6 +110,35 @@
   the extern template declaration) as exported on Windows, as discussed above.
   On all other platforms, this macro has an empty definition.
 
+**_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS**
+  Mark a symbol as hidden so it will not be exported from shared libraries. This
+  is intended specifically for method templates of either classes marked with
+  `_LIBCPP_TYPE_VIS` or classes with an extern template instantiation
+  declaration marked with `_LIBCPP_EXTERN_TEMPLATE_TYPE_VIS`.
+
+  When building libc++ with hidden visibility, we want explicit template
+  instantiations to export members, which is consistent with existing Windows
+  behavior. We also want classes annotated with `_LIBCPP_TYPE_VIS` to export
+  their members, which is again consistent with existing Windows behavior.
+  Both these changes are necessary for clients to be able to link against a
+  libc++ DSO built with hidden visibility without encountering missing symbols.
+
+  An unfortunate side effect, however, is that method templates of classes
+  either marked `_LIBCPP_TYPE_VIS` or with extern template instantiation
+  declarations marked with `_LIBCPP_EXTERN_TEMPLATE_TYPE_VIS` also get default
+  visibility when instantiated. These methods are often implicitly instantiated
+  inside other libraries which use the libc++ headers, and will therefore end up
+  being exported from those libraries, since those implicit instantiations will
+  receive default visibility. This is not acceptable for libraries that wish to
+  control their visibility, and led to PR30642.
+
+  Consequently, all such problematic method templates are explicitly marked
+  either hidden (via this macro) or inline, so that they don't leak into client
+  libraries. The problematic methods were found by running
+  `bad-visibility-finder `_
+  against the libc++ headers after making `_LIBCPP_TYPE_VIS` and
+  `_LIBCPP_EXTERN_TEMPLATE_TYPE_VIS` expand to default visibility.
+
 **_LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY**
   Mark a member function of a class template as visible and always inline. This
   macro should only be applied to member functions of class templates that are
Index: libcxx/trunk/include/__config
===
--- libcxx/trunk/include/__config
+++ libcxx/trunk/include/__config
@@ -591,6 +591,7 @@
 #define _LIBCPP_EXTERN_VIS  _LIBCPP_DLL_VIS
 #define _LIBCPP_EXCEPTION_ABI   _LIBCPP_DLL_VIS
 #define _LIBCPP_HIDDEN
+#define _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
 #define _LIBCPP_TEMPLATE_VIS
 #define _LIBCPP_FUNC_VIS_ONLY
 #define _LIBCPP_ENUM_VIS
@@ -614,6 +615,9 @@
 #endif
 #endif
 
+// The inline should be removed once PR32114 is resolved
+#define _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS inline _LIBCPP_HIDDEN
+
 #ifndef _LIBCPP_FUNC_VIS
 #if !defined(_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS)
 #define _LIBCPP_FUNC_VIS __attribute__ ((__visibility__("default")))
@@ -668,7 +672,7 @@
 
 #ifndef _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS
 #  if !defined(_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS) && __has_attribute(__type_visibility__)
-#define _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS __attribute__ ((__type_visibility__("default")))
+#define _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS __attribute__ ((__visibility__("default")))
 #  else
 #define _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS
 #  endif
Index: libcxx/trunk/include/locale
===
--- libcxx/trunk/include/locale
+++ libcxx/trunk/include/locale
@@ -623,17 +623,20 @@
 ~num_get() {}
 
 template 
-inline iter_type __do_get_floating_point
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
+iter_type __do_get_floating_point
 (iter_type __b, iter_type __e, ios_base& __iob,
  ios_base::iostate& __err, _Fp& __v) const;
 
 template 
-inline iter_type __do_get_signed
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
+iter_type __do_get_signed
 (iter_type __b, iter_type __e, ios_base& __iob,
  ios_base::iostate& __err, _Signed& __v) con

[libcxx] r296731 - [libc++] Make _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS export members

[Shoaib Meenai via cfe-commits]

Author: smeenai
Date: Wed Mar  1 21:02:50 2017
New Revision: 296731

URL: http://llvm.org/viewvc/llvm-project?rev=296731&view=rev
Log:
[libc++] Make _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS export members

When building libc++ with hidden visibility, we want explicit template
instantiations to export members. This is consistent with existing
Windows behavior, and is necessary for clients to be able to link
against a hidden visibility built libc++ without running into lots of
missing symbols.

An unfortunate side effect, however, is that any template methods of a
class with an explicit instantiation will get default visibility when
instantiated, unless the methods are explicitly marked inline or hidden
visibility. This is not desirable for clients of libc++ headers who wish
to control their visibility, and led to PR30642.

Annotate all problematic methods with an explicit visibility specifier
to avoid this. The problematic methods were found by running
https://github.com/smeenai/bad-visibility-finder against the libc++
headers after making the _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS change. The
methods were marked with the new 
_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
macro, which was created for this purpose.

It should be noted that _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS was originally
intended to expand to default visibility, and was changed to expanding
to default type visibility to fix PR30642. The visibility macro
documentation was not updated accordingly, however, so this change makes
the macro consistent with its documentation again, while explicitly
fixing the methods which resulted in that PR.

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

Modified:
libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst
libcxx/trunk/include/__config
libcxx/trunk/include/locale
libcxx/trunk/include/string
libcxx/trunk/lib/abi/CHANGELOG.TXT

Modified: libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst?rev=296731&r1=296730&r2=296731&view=diff
==
--- libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst (original)
+++ libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst Wed Mar  1 21:02:50 2017
@@ -110,6 +110,35 @@ Visibility Macros
   the extern template declaration) as exported on Windows, as discussed above.
   On all other platforms, this macro has an empty definition.
 
+**_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS**
+  Mark a symbol as hidden so it will not be exported from shared libraries. 
This
+  is intended specifically for method templates of either classes marked with
+  `_LIBCPP_TYPE_VIS` or classes with an extern template instantiation
+  declaration marked with `_LIBCPP_EXTERN_TEMPLATE_TYPE_VIS`.
+
+  When building libc++ with hidden visibility, we want explicit template
+  instantiations to export members, which is consistent with existing Windows
+  behavior. We also want classes annotated with `_LIBCPP_TYPE_VIS` to export
+  their members, which is again consistent with existing Windows behavior.
+  Both these changes are necessary for clients to be able to link against a
+  libc++ DSO built with hidden visibility without encountering missing symbols.
+
+  An unfortunate side effect, however, is that method templates of classes
+  either marked `_LIBCPP_TYPE_VIS` or with extern template instantiation
+  declarations marked with `_LIBCPP_EXTERN_TEMPLATE_TYPE_VIS` also get default
+  visibility when instantiated. These methods are often implicitly instantiated
+  inside other libraries which use the libc++ headers, and will therefore end 
up
+  being exported from those libraries, since those implicit instantiations will
+  receive default visibility. This is not acceptable for libraries that wish to
+  control their visibility, and led to PR30642.
+
+  Consequently, all such problematic method templates are explicitly marked
+  either hidden (via this macro) or inline, so that they don't leak into client
+  libraries. The problematic methods were found by running
+  `bad-visibility-finder `_
+  against the libc++ headers after making `_LIBCPP_TYPE_VIS` and
+  `_LIBCPP_EXTERN_TEMPLATE_TYPE_VIS` expand to default visibility.
+
 **_LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY**
   Mark a member function of a class template as visible and always inline. This
   macro should only be applied to member functions of class templates that are

Modified: libcxx/trunk/include/__config
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__config?rev=296731&r1=296730&r2=296731&view=diff
==
--- libcxx/trunk/include/__config (original)
+++ libcxx/trunk/include/__config Wed Mar  1 21:02:50 2017
@@ -591,6 +591,7 @@ namespace std {
 #define _LIBCPP_EXTERN_VIS  _LIBCPP_DLL_VIS
 #define _LIBCPP_EXCEPTION

[PATCH] D29157: [libc++] Make _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS export members

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai updated this revision to Diff 90272.
smeenai added a comment.

Including inline in macro to work around PR32114


https://reviews.llvm.org/D29157

Files:
  docs/DesignDocs/VisibilityMacros.rst
  include/__config
  include/locale
  include/string

Index: include/string
===
--- include/string
+++ include/string
@@ -792,6 +792,7 @@
 basic_string(const basic_string& __str, size_type __pos,
  const _Allocator& __a = _Allocator());
 template
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
 basic_string(const _Tp& __t, size_type __pos, size_type __n,
  const allocator_type& __a = allocator_type(),
  typename enable_if<__can_be_converted_to_string_view<_CharT, _Traits, _Tp>::value, void>::type* = 0);
@@ -927,7 +928,8 @@
 basic_string& append(__self_view __sv) { return append(__sv.data(), __sv.size()); }
 basic_string& append(const basic_string& __str, size_type __pos, size_type __n=npos);
 template 
-inline typename enable_if
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
+typename enable_if
 <
 __can_be_converted_to_string_view<_CharT, _Traits, _Tp>::value,
 basic_string&
@@ -937,9 +939,11 @@
 basic_string& append(const value_type* __s);
 basic_string& append(size_type __n, value_type __c);
 template 
-inline basic_string& __append_forward_unsafe(_ForwardIterator, _ForwardIterator);
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
+basic_string& __append_forward_unsafe(_ForwardIterator, _ForwardIterator);
 template
-inline typename enable_if
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
+typename enable_if
 <
 __is_exactly_input_iterator<_InputIterator>::value
 || !__libcpp_string_gets_noexcept_iterator<_InputIterator>::value,
@@ -952,7 +956,8 @@
   return *this;
 }
 template
-inline typename enable_if
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
+typename enable_if
 <
 __is_forward_iterator<_ForwardIterator>::value
 && __libcpp_string_gets_noexcept_iterator<_ForwardIterator>::value,
@@ -988,7 +993,8 @@
 #endif
 basic_string& assign(const basic_string& __str, size_type __pos, size_type __n=npos);
 template 
-inline typename enable_if
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
+typename enable_if
 <
 __can_be_converted_to_string_view<_CharT, _Traits, _Tp>::value,
 basic_string&
@@ -998,15 +1004,17 @@
 basic_string& assign(const value_type* __s);
 basic_string& assign(size_type __n, value_type __c);
 template
-inline typename enable_if
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
+typename enable_if
 <
__is_exactly_input_iterator<_InputIterator>::value
 || !__libcpp_string_gets_noexcept_iterator<_InputIterator>::value,
 basic_string&
 >::type
 assign(_InputIterator __first, _InputIterator __last);
 template
-inline typename enable_if
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
+typename enable_if
 <
 __is_forward_iterator<_ForwardIterator>::value
  && __libcpp_string_gets_noexcept_iterator<_ForwardIterator>::value,
@@ -1023,7 +1031,8 @@
 _LIBCPP_INLINE_VISIBILITY
 basic_string& insert(size_type __pos1, __self_view __sv) { return insert(__pos1, __sv.data(), __sv.size()); }
 template 
-inline typename enable_if
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
+typename enable_if
 <
 __can_be_converted_to_string_view<_CharT, _Traits, _Tp>::value,
 basic_string&
@@ -1037,15 +1046,17 @@
 _LIBCPP_INLINE_VISIBILITY
 iterator  insert(const_iterator __pos, size_type __n, value_type __c);
 template
-inline typename enable_if
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
+typename enable_if
 <
__is_exactly_input_iterator<_InputIterator>::value
 || !__libcpp_string_gets_noexcept_iterator<_InputIterator>::value,
 iterator
 >::type
 insert(const_iterator __pos, _InputIterator __first, _InputIterator __last);
 template
-inline typename enable_if
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
+typename enable_if
 <
 __is_forward_iterator<_ForwardIterator>::value
  && __libcpp_string_gets_noexcept_iterator<_ForwardIterator>::value,
@@ -1070,7 +1081,8 @@
 basic_string& replace(size_type __pos1, size_type __n1, __self_view __sv) { return replace(__pos1, __n1, __sv.data(), __sv.size()); }
 basic_string& replace(size_type __pos1, size_type __n1, const basic_string& __str, size_type __pos2, size_type __n2=npos);
 template 
-inline typena

[PATCH] D30523: [libc++] Mark some std::num_get method templates inline

[Shoaib Meenai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL296729: [libc++] Mark some std::num_get method templates 
inline (authored by smeenai).

Changed prior to commit:
  https://reviews.llvm.org/D30523?vs=90270&id=90271#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30523

Files:
  libcxx/trunk/include/locale
  libcxx/trunk/lib/abi/CHANGELOG.TXT
  libcxx/trunk/lib/abi/x86_64-unknown-linux-gnu.abilist

Index: libcxx/trunk/lib/abi/x86_64-unknown-linux-gnu.abilist
===
--- libcxx/trunk/lib/abi/x86_64-unknown-linux-gnu.abilist
+++ libcxx/trunk/lib/abi/x86_64-unknown-linux-gnu.abilist
@@ -312,15 +312,6 @@
 {'is_defined': True, 'name': '_ZNKSt3__17collateIwE10do_compareEPKwS3_S3_S3_', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNKSt3__17collateIwE12do_transformEPKwS3_', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNKSt3__17collateIwE7do_hashEPKwS3_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE15__do_get_signedIlEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE15__do_get_signedIxEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE17__do_get_unsignedIjEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE17__do_get_unsignedImEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE17__do_get_unsignedItEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE17__do_get_unsignedIyEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE23__do_get_floating_pointIdEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE23__do_get_floating_pointIeEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE23__do_get_floating_pointIfEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE6do_getES4_S4_RNS_8ios_baseERjRPv', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE6do_getES4_S4_RNS_8ios_baseERjRb', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE6do_getES4_S4_RNS_8ios_baseERjRd', 'type': 'FUNC'}
@@ -332,15 +323,6 @@
 {'is_defined': True, 'name': '_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE6do_getES4_S4_RNS_8ios_baseERjRx', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE6do_getES4_S4_RNS_8ios_baseERjRy', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE6do_getES4_S4_RNS_8ios_baseERjS8_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIwNS_19istreambuf_iteratorIwNS_11char_traitsIwE15__do_get_signedIlEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIwNS_19istreambuf_iteratorIwNS_11char_traitsIwE15__do_get_signedIxEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIwNS_19istreambuf_iteratorIwNS_11char_traitsIwE17__do_get_unsignedIjEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIwNS_19istreambuf_iteratorIwNS_11char_traitsIwE17__do_get_unsignedImEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIwNS_19istreambuf_iteratorIwNS_11char_traitsIwE17__do_get_unsignedItEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIwNS_19istreambuf_iteratorIwNS_11char_traitsIwE17__do_get_unsignedIyEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIwNS_19istreambuf_iteratorIwNS_11char_traitsIwE23__do_get_floating_pointIdEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIwNS_19istreambuf_iteratorIwNS_11char_traitsIwE23__do_get_floating_pointIeEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIwNS_19istreambuf_iteratorIwNS_11char_traitsIwE23__do_get_floating_pointIfEES4_S4_S4_RNS_8ios_baseERjRT_', 'type':

[libcxx] r296729 - [libc++] Mark some std::num_get method templates inline

[Shoaib Meenai via cfe-commits]

Author: smeenai
Date: Wed Mar  1 20:51:27 2017
New Revision: 296729

URL: http://llvm.org/viewvc/llvm-project?rev=296729&view=rev
Log:
[libc++] Mark some std::num_get method templates inline

D29157 will make explicit template instantiations expand to default
visibility, at which point these method templates will need to be
explicitly marked hidden visibility to avoid leaking into other DSOs.
Unfortunately, because of clang PR32114, they must be marked inline (in
conjunction with `-fvisibility-inlines-hidden`) to actually hide them,
since clang doesn't respect the hidden visibility annotation.

Since this involves an ABI change, mark these methods inline in a
separate change, so that the ABI changes can be reviewed separately and
verified to be safe.

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

Modified:
libcxx/trunk/include/locale
libcxx/trunk/lib/abi/CHANGELOG.TXT
libcxx/trunk/lib/abi/x86_64-unknown-linux-gnu.abilist

Modified: libcxx/trunk/include/locale
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/locale?rev=296729&r1=296728&r2=296729&view=diff
==
--- libcxx/trunk/include/locale (original)
+++ libcxx/trunk/include/locale Wed Mar  1 20:51:27 2017
@@ -623,17 +623,17 @@ protected:
 ~num_get() {}
 
 template 
-iter_type __do_get_floating_point
+inline iter_type __do_get_floating_point
 (iter_type __b, iter_type __e, ios_base& __iob,
  ios_base::iostate& __err, _Fp& __v) const;
 
 template 
-iter_type __do_get_signed
+inline iter_type __do_get_signed
 (iter_type __b, iter_type __e, ios_base& __iob,
  ios_base::iostate& __err, _Signed& __v) const;
 
 template 
-iter_type __do_get_unsigned
+inline iter_type __do_get_unsigned
 (iter_type __b, iter_type __e, ios_base& __iob,
  ios_base::iostate& __err, _Unsigned& __v) const;
 

Modified: libcxx/trunk/lib/abi/CHANGELOG.TXT
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/lib/abi/CHANGELOG.TXT?rev=296729&r1=296728&r2=296729&view=diff
==
--- libcxx/trunk/lib/abi/CHANGELOG.TXT (original)
+++ libcxx/trunk/lib/abi/CHANGELOG.TXT Wed Mar  1 20:51:27 2017
@@ -13,6 +13,42 @@ Afterwards the ABI list should be update
 New entries should be added directly below the "Version" header.
 
 ---
+Version 5.0
+---
+
+* rTBD - Remove std::num_get template methods which should be inline
+
+  These functions should never have had visible definitions in the dylib but
+  since they were previously not specified with 'inline' they accidentally
+  got emitted. This change is non-ABI breaking because every "linkage unit"
+  must contain its own definition.
+
+  x86_64-linux-gnu
+  
+  SYMBOL REMOVED: 
_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE15__do_get_signedIlEES4_S4_S4_RNS_8ios_baseERjRT_
+  SYMBOL REMOVED: 
_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE15__do_get_signedIxEES4_S4_S4_RNS_8ios_baseERjRT_
+  SYMBOL REMOVED: 
_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE17__do_get_unsignedIjEES4_S4_S4_RNS_8ios_baseERjRT_
+  SYMBOL REMOVED: 
_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE17__do_get_unsignedImEES4_S4_S4_RNS_8ios_baseERjRT_
+  SYMBOL REMOVED: 
_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE17__do_get_unsignedItEES4_S4_S4_RNS_8ios_baseERjRT_
+  SYMBOL REMOVED: 
_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE17__do_get_unsignedIyEES4_S4_S4_RNS_8ios_baseERjRT_
+  SYMBOL REMOVED: 
_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE23__do_get_floating_pointIdEES4_S4_S4_RNS_8ios_baseERjRT_
+  SYMBOL REMOVED: 
_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE23__do_get_floating_pointIeEES4_S4_S4_RNS_8ios_baseERjRT_
+  SYMBOL REMOVED: 
_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE23__do_get_floating_pointIfEES4_S4_S4_RNS_8ios_baseERjRT_
+  SYMBOL REMOVED: 
_ZNKSt3__17num_getIwNS_19istreambuf_iteratorIwNS_11char_traitsIwE15__do_get_signedIlEES4_S4_S4_RNS_8ios_baseERjRT_
+  SYMBOL REMOVED: 
_ZNKSt3__17num_getIwNS_19istreambuf_iteratorIwNS_11char_traitsIwE15__do_get_signedIxEES4_S4_S4_RNS_8ios_baseERjRT_
+  SYMBOL REMOVED: 
_ZNKSt3__17num_getIwNS_19istreambuf_iteratorIwNS_11char_traitsIwE17__do_get_unsignedIjEES4_S4_S4_RNS_8ios_baseERjRT_
+  SYMBOL REMOVED: 
_ZNKSt3__17num_getIwNS_19istreambuf_iteratorIwNS_11char_traitsIwE17__do_get_unsignedImEES4_S4_S4_RNS_8ios_baseERjRT_
+  SYMBOL REMOVED: 
_ZNKSt3__17num_getIwNS_19istreambuf_iteratorIwNS_11char_traitsIwE17__do_get_unsignedItEES4_S4_S4_RNS_8ios_baseERjRT_
+  SYMBOL REMOVED: 
_ZNKSt3__1

[PATCH] D30523: [libc++] Mark some std::num_get method templates inline

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai created this revision.

https://reviews.llvm.org/D29157 will make explicit template instantiations 
expand to default
visibility, at which point these method templates will need to be
explicitly marked hidden visibility to avoid leaking into other DSOs.
Unfortunately, because of clang PR32114, they must be marked inline (in
conjunction with `-fvisibility-inlines-hidden`) to actually hide them,
since clang doesn't respect the hidden visibility annotation.

Since this involves an ABI change, mark these methods inline in a
separate change, so that the ABI changes can be reviewed separately and
verified to be safe.


https://reviews.llvm.org/D30523

Files:
  include/locale
  lib/abi/CHANGELOG.TXT
  lib/abi/x86_64-unknown-linux-gnu.abilist

Index: lib/abi/x86_64-unknown-linux-gnu.abilist
===
--- lib/abi/x86_64-unknown-linux-gnu.abilist
+++ lib/abi/x86_64-unknown-linux-gnu.abilist
@@ -312,15 +312,6 @@
 {'is_defined': True, 'name': '_ZNKSt3__17collateIwE10do_compareEPKwS3_S3_S3_', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNKSt3__17collateIwE12do_transformEPKwS3_', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNKSt3__17collateIwE7do_hashEPKwS3_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE15__do_get_signedIlEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE15__do_get_signedIxEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE17__do_get_unsignedIjEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE17__do_get_unsignedImEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE17__do_get_unsignedItEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE17__do_get_unsignedIyEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE23__do_get_floating_pointIdEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE23__do_get_floating_pointIeEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE23__do_get_floating_pointIfEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE6do_getES4_S4_RNS_8ios_baseERjRPv', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE6do_getES4_S4_RNS_8ios_baseERjRb', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE6do_getES4_S4_RNS_8ios_baseERjRd', 'type': 'FUNC'}
@@ -332,15 +323,6 @@
 {'is_defined': True, 'name': '_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE6do_getES4_S4_RNS_8ios_baseERjRx', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE6do_getES4_S4_RNS_8ios_baseERjRy', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNKSt3__17num_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcE6do_getES4_S4_RNS_8ios_baseERjS8_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIwNS_19istreambuf_iteratorIwNS_11char_traitsIwE15__do_get_signedIlEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIwNS_19istreambuf_iteratorIwNS_11char_traitsIwE15__do_get_signedIxEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIwNS_19istreambuf_iteratorIwNS_11char_traitsIwE17__do_get_unsignedIjEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIwNS_19istreambuf_iteratorIwNS_11char_traitsIwE17__do_get_unsignedImEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIwNS_19istreambuf_iteratorIwNS_11char_traitsIwE17__do_get_unsignedItEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIwNS_19istreambuf_iteratorIwNS_11char_traitsIwE17__do_get_unsignedIyEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIwNS_19istreambuf_iteratorIwNS_11char_traitsIwE23__do_get_floating_pointIdEES4_S4_S4_RNS_8ios_baseERjRT_', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNKSt3__17num_getIwNS_19istreambuf_iteratorIwNS

[PATCH] D30345: [CodeGen][Blocks] Refactor capture handling in code that generates block copy/destroy routines

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Thanks, LGTM.


Repository:
  rL LLVM

https://reviews.llvm.org/D30345



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

[libcxx] r296727 - Remove more usages of REQUIRES-ANY in the test-suite

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Wed Mar  1 20:04:49 2017
New Revision: 296727

URL: http://llvm.org/viewvc/llvm-project?rev=296727&view=rev
Log:
Remove more usages of REQUIRES-ANY in the test-suite

Modified:

libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.cons/assignment.pass.cpp

libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.cons/convert.pass.cpp

libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.cons/convert_assignment.pass.cpp

libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.cons/copy.pass.cpp

libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.cons/pointer.pass.cpp

libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.conv/assign_from_auto_ptr_ref.pass.cpp

libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.conv/convert_from_auto_ptr_ref.pass.cpp

libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.conv/convert_to_auto_ptr.pass.cpp

libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.conv/convert_to_auto_ptr_ref.pass.cpp

libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.members/arrow.pass.cpp

libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.members/deref.pass.cpp

libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.members/release.pass.cpp

libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.members/reset.pass.cpp
libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/element_type.pass.cpp

libcxx/trunk/test/std/utilities/memory/unique.ptr/unique.ptr.single/unique.ptr.single.ctor/auto_pointer.pass.cpp

libcxx/trunk/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.assign/auto_ptr_Y.pass.cpp

libcxx/trunk/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/auto_ptr.pass.cpp

Modified: 
libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.cons/assignment.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.cons/assignment.pass.cpp?rev=296727&r1=296726&r2=296727&view=diff
==
--- 
libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.cons/assignment.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.cons/assignment.pass.cpp
 Wed Mar  1 20:04:49 2017
@@ -13,7 +13,7 @@
 
 // auto_ptr& operator=(auto_ptr& a) throw();
 
-// REQUIRES-ANY: c++98, c++03, c++11, c++14
+// REQUIRES: c++98 || c++03 || c++11 || c++14
 
 #include 
 #include 

Modified: 
libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.cons/convert.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.cons/convert.pass.cpp?rev=296727&r1=296726&r2=296727&view=diff
==
--- 
libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.cons/convert.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.cons/convert.pass.cpp
 Wed Mar  1 20:04:49 2017
@@ -13,7 +13,7 @@
 
 // auto_ptr(auto_ptr& a) throw();
 
-// REQUIRES-ANY: c++98, c++03, c++11, c++14
+// REQUIRES: c++98 || c++03 || c++11 || c++14
 
 #include 
 #include 

Modified: 
libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.cons/convert_assignment.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.cons/convert_assignment.pass.cpp?rev=296727&r1=296726&r2=296727&view=diff
==
--- 
libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.cons/convert_assignment.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.cons/convert_assignment.pass.cpp
 Wed Mar  1 20:04:49 2017
@@ -13,7 +13,7 @@
 
 // template auto_ptr& operator=(auto_ptr& a) throw();
 
-// REQUIRES-ANY: c++98, c++03, c++11, c++14
+// REQUIRES: c++98 || c++03 || c++11 || c++14
 
 #include 
 #include 

Modified: 
libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.cons/copy.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.cons/copy.pass.cpp?rev=296727&r1=296726&r2=296727&view=diff
==
--- 
libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.cons/copy.pass.cpp 
(original)
+++ 
libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.cons/copy.pass.cpp 
Wed Mar  1 20:04:49 2017
@@ -13,7 +13,7 @@
 
 // auto_ptr(auto_ptr& a) throw();
 
-// REQUIRES-ANY: c++98, c++03, c++11, c++14
+// REQUIRES: c++98 || c++03 || c++11 || c++14
 
 #include 
 #include 

Modified: 
libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.cons/pointer.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/depr/depr.auto.ptr/auto.ptr/auto.ptr.cons/pointer.

[libcxx] r296725 - Recommit r296712: "Fix Apple-specific XFAIL directive in libc++ test"

[Mehdi Amini via cfe-commits]

Author: mehdi_amini
Date: Wed Mar  1 20:01:11 2017
New Revision: 296725

URL: http://llvm.org/viewvc/llvm-project?rev=296725&view=rev
Log:
Recommit r296712: "Fix Apple-specific XFAIL directive in libc++ test"

The test is passing with c++11 and c++14 but not c++1z on this
particular version of the compiler. Try to use lit boolean condition
to satisfy this constaint.

Modified:

libcxx/trunk/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.cons/default.pass.cpp

Modified: 
libcxx/trunk/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.cons/default.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.cons/default.pass.cpp?rev=296725&r1=296724&r2=296725&view=diff
==
--- 
libcxx/trunk/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.cons/default.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.cons/default.pass.cpp
 Wed Mar  1 20:01:11 2017
@@ -9,7 +9,7 @@
 
 // Usage of is_trivially_constructible is broken with these compilers.
 // See https://bugs.llvm.org/show_bug.cgi?id=31016
-// XFAIL: clang-3.7, apple-clang-7, apple-clang-7.0
+// XFAIL: clang-3.7, apple-clang-7 && c++1z
 
 // 
 


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

[libcxx] r296724 - Revert "Fix Apple-specific XFAIL directive in libc++ test"

[Mehdi Amini via cfe-commits]

Author: mehdi_amini
Date: Wed Mar  1 19:57:40 2017
New Revision: 296724

URL: http://llvm.org/viewvc/llvm-project?rev=296724&view=rev
Log:
Revert "Fix Apple-specific XFAIL directive in libc++ test"

This reverts commit r296712. It broke our bot.

It turns out that the test is passing with c++11 and c++14 but
not c++1z on this particular version of the compiler. Since one
job is defaulting to c++1z and the other is testing all config I'm
not sure how to fix this...

Modified:

libcxx/trunk/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.cons/default.pass.cpp

Modified: 
libcxx/trunk/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.cons/default.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.cons/default.pass.cpp?rev=296724&r1=296723&r2=296724&view=diff
==
--- 
libcxx/trunk/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.cons/default.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.cons/default.pass.cpp
 Wed Mar  1 19:57:40 2017
@@ -9,7 +9,7 @@
 
 // Usage of is_trivially_constructible is broken with these compilers.
 // See https://bugs.llvm.org/show_bug.cgi?id=31016
-// XFAIL: clang-3.7, apple-clang-7.0
+// XFAIL: clang-3.7, apple-clang-7, apple-clang-7.0
 
 // 
 


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

[PATCH] D29157: [libc++] Make _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS export members

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai updated this revision to Diff 90264.
smeenai added a comment.

Changing more instances to new macro, per IRC discussion with Eric


https://reviews.llvm.org/D29157

Files:
  docs/DesignDocs/VisibilityMacros.rst
  include/__config
  include/locale
  include/string

Index: include/string
===
--- include/string
+++ include/string
@@ -792,6 +792,7 @@
 basic_string(const basic_string& __str, size_type __pos,
  const _Allocator& __a = _Allocator());
 template
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
 basic_string(const _Tp& __t, size_type __pos, size_type __n,
  const allocator_type& __a = allocator_type(),
  typename enable_if<__can_be_converted_to_string_view<_CharT, _Traits, _Tp>::value, void>::type* = 0);
@@ -927,7 +928,8 @@
 basic_string& append(__self_view __sv) { return append(__sv.data(), __sv.size()); }
 basic_string& append(const basic_string& __str, size_type __pos, size_type __n=npos);
 template 
-inline typename enable_if
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
+typename enable_if
 <
 __can_be_converted_to_string_view<_CharT, _Traits, _Tp>::value,
 basic_string&
@@ -937,9 +939,11 @@
 basic_string& append(const value_type* __s);
 basic_string& append(size_type __n, value_type __c);
 template 
-inline basic_string& __append_forward_unsafe(_ForwardIterator, _ForwardIterator);
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
+basic_string& __append_forward_unsafe(_ForwardIterator, _ForwardIterator);
 template
-inline typename enable_if
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
+typename enable_if
 <
 __is_exactly_input_iterator<_InputIterator>::value
 || !__libcpp_string_gets_noexcept_iterator<_InputIterator>::value,
@@ -952,7 +956,8 @@
   return *this;
 }
 template
-inline typename enable_if
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
+typename enable_if
 <
 __is_forward_iterator<_ForwardIterator>::value
 && __libcpp_string_gets_noexcept_iterator<_ForwardIterator>::value,
@@ -988,7 +993,8 @@
 #endif
 basic_string& assign(const basic_string& __str, size_type __pos, size_type __n=npos);
 template 
-inline typename enable_if
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
+typename enable_if
 <
 __can_be_converted_to_string_view<_CharT, _Traits, _Tp>::value,
 basic_string&
@@ -998,15 +1004,17 @@
 basic_string& assign(const value_type* __s);
 basic_string& assign(size_type __n, value_type __c);
 template
-inline typename enable_if
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
+typename enable_if
 <
__is_exactly_input_iterator<_InputIterator>::value
 || !__libcpp_string_gets_noexcept_iterator<_InputIterator>::value,
 basic_string&
 >::type
 assign(_InputIterator __first, _InputIterator __last);
 template
-inline typename enable_if
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
+typename enable_if
 <
 __is_forward_iterator<_ForwardIterator>::value
  && __libcpp_string_gets_noexcept_iterator<_ForwardIterator>::value,
@@ -1023,7 +1031,8 @@
 _LIBCPP_INLINE_VISIBILITY
 basic_string& insert(size_type __pos1, __self_view __sv) { return insert(__pos1, __sv.data(), __sv.size()); }
 template 
-inline typename enable_if
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
+typename enable_if
 <
 __can_be_converted_to_string_view<_CharT, _Traits, _Tp>::value,
 basic_string&
@@ -1037,15 +1046,17 @@
 _LIBCPP_INLINE_VISIBILITY
 iterator  insert(const_iterator __pos, size_type __n, value_type __c);
 template
-inline typename enable_if
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
+typename enable_if
 <
__is_exactly_input_iterator<_InputIterator>::value
 || !__libcpp_string_gets_noexcept_iterator<_InputIterator>::value,
 iterator
 >::type
 insert(const_iterator __pos, _InputIterator __first, _InputIterator __last);
 template
-inline typename enable_if
+_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
+typename enable_if
 <
 __is_forward_iterator<_ForwardIterator>::value
  && __libcpp_string_gets_noexcept_iterator<_ForwardIterator>::value,
@@ -1070,7 +1081,8 @@
 basic_string& replace(size_type __pos1, size_type __n1, __self_view __sv) { return replace(__pos1, __n1, __sv.data(), __sv.size()); }
 basic_string& replace(size_type __pos1, size_type __n1, const basic_string& __str, size_type __pos2, size_type __n2=npos);
 template 

[PATCH] D28348: [analyzer] Taught the analyzer about Glib API to check Memory-leak

[Leslie Zhai via Phabricator via cfe-commits]

xiangzhai added a comment.

Hi Anna,

Thanks for your review!

> The information generated by this checker is used for array bounds checking. 
> For example, see https://reviews.llvm.org/D24307

I will read carefully about that ;-)

> This patch looks good. Do you have commit access or should I commit it on 
> your behalf?

Please commit it on my behalf, thanks a lot!

Regards,
Leslie Zhai


Repository:
  rL LLVM

https://reviews.llvm.org/D28348



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

RE: r296554 - [PS4] Set our default dialect to C++11. NFC for other targets.

[Robinson, Paul via cfe-commits]

Mehdi, again I invite you to discuss this on the cfe-dev thread rather than 
here.  I have posted a reply there to bump it up, and any comments you have 
will be more visible and better serve the community over there.
Thanks,
--paulr

From: mehdi.am...@apple.com [mailto:mehdi.am...@apple.com]
Sent: Wednesday, March 01, 2017 3:52 PM
To: Sean Silva
Cc: Robinson, Paul; cfe-commits
Subject: Re: r296554 - [PS4] Set our default dialect to C++11. NFC for other 
targets.


On Mar 1, 2017, at 3:24 PM, Sean Silva 
mailto:chisophu...@gmail.com>> wrote:



On Wed, Mar 1, 2017 at 10:35 AM, Mehdi Amini 
mailto:mehdi.am...@apple.com>> wrote:
I’m not sure I find this nice to see this upstream.

I not fond in general of this kind of difference in behavior. I don’t think it 
is good for clang to have different default for this kind of settings depending 
on the platform. It does not provide a very good user experience from a 
cross-platform point of view (i.e. my compiler behaves very differently when I 
target one platform instead of another).

What I like about it is that the upstream PS4 bots now test that we don't 
depend on the C++98 default language standard in tests, which is net positive 
IMO since it facilitates future changes. Should this be a point of 
vendor/platform extensibility? That's a question for cfe-dev, but I don't think 
it's unreasonable. (see also: PS4 has -fno-rtti and -fno-exceptions by default, 
even though users already know to pass the right flags and expect to have to).

I rather have the driver rejecting “unsupported configuration”, requiring the 
right flags, instead of diverging the default.

—
Mehdi






On Feb 28, 2017, at 11:22 PM, Sean Silva via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:

Nice!

-- Sean Silva

On Tue, Feb 28, 2017 at 5:01 PM, Paul Robinson via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: probinson
Date: Tue Feb 28 19:01:10 2017
New Revision: 296554

URL: http://llvm.org/viewvc/llvm-project?rev=296554&view=rev
Log:
[PS4] Set our default dialect to C++11. NFC for other targets.
Reapplies r296209 now that r296549 has fixed what really seems to be
the last problematic test.

Modified:
cfe/trunk/lib/Frontend/CompilerInvocation.cpp

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=296554&r1=296553&r2=296554&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Tue Feb 28 19:01:10 2017
@@ -1582,7 +1582,11 @@ void CompilerInvocation::setLangDefaults
 case IK_PreprocessedCXX:
 case IK_ObjCXX:
 case IK_PreprocessedObjCXX:
-  LangStd = LangStandard::lang_gnucxx98;
+  // The PS4 uses C++11 as the default C++ standard.
+  if (T.isPS4())
+LangStd = LangStandard::lang_gnucxx11;
+  else
+LangStd = LangStandard::lang_gnucxx98;
   break;
 case IK_RenderScript:
   LangStd = LangStandard::lang_c99;


___
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



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

[PATCH] D30518: Fix msc-version.c test to handle _MSC_VER=1910

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm


https://reviews.llvm.org/D30518



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

[PATCH] D30518: Fix msc-version.c test to handle _MSC_VER=1910

[Dave Bartolomeo via Phabricator via cfe-commits]

DaveBartolomeo created this revision.

Previously, VC++ has always set _MSC_VER to a four-digit value with the two 
least significant digits set to zero. Visual Studio 2017, however, sets 
_MSC_VER=1910, and we expect to update the least significant digit as we 
release major updates for VS 2017. This patch fixes the msc-version.c test to 
handle non-zero values in the two least significant digits of _MSC_VER.


https://reviews.llvm.org/D30518

Files:
  test/Driver/msc-version.c


Index: test/Driver/msc-version.c
===
--- test/Driver/msc-version.c
+++ test/Driver/msc-version.c
@@ -63,4 +63,4 @@
 
 // CHECK-MS-EXTENSIONS: _MSC_BUILD 1
 // CHECK-MS-EXTENSIONS: _MSC_FULL_VER {{.+}}
-// CHECK-MS-EXTENSIONS: _MSC_VER {{..}}00
+// CHECK-MS-EXTENSIONS: _MSC_VER {{}}


Index: test/Driver/msc-version.c
===
--- test/Driver/msc-version.c
+++ test/Driver/msc-version.c
@@ -63,4 +63,4 @@
 
 // CHECK-MS-EXTENSIONS: _MSC_BUILD 1
 // CHECK-MS-EXTENSIONS: _MSC_FULL_VER {{.+}}
-// CHECK-MS-EXTENSIONS: _MSC_VER {{..}}00
+// CHECK-MS-EXTENSIONS: _MSC_VER {{}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30517: [libc++abi] Add option to enable definitions for the new/delete overloads.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF created this revision.

Currently both libc++ and libc++abi provide definitions for operator 
new/delete. However I believe this is incorrect and that one or the other 
should offer them.

This patch adds the CMake option `-DLIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS` 
which defaults to `OFF` unless otherwise specified. This means that by default
only libc++ provides the new/delete definitions.


https://reviews.llvm.org/D30517

Files:
  CMakeLists.txt
  src/CMakeLists.txt


Index: src/CMakeLists.txt
===
--- src/CMakeLists.txt
+++ src/CMakeLists.txt
@@ -20,10 +20,9 @@
   private_typeinfo.cpp
 )
 
-# FIXME: This file should only be compiled in special configurations such
-#  as building the Apple system dylib where libc++abi is expected to provide
-#  the new/delete definitions instead of libc++.
-list(APPEND LIBCXXABI_SOURCES stdlib_new_delete.cpp)
+if (LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS)
+  list(APPEND LIBCXXABI_SOURCES stdlib_new_delete.cpp)
+endif()
 
 if (LIBCXXABI_ENABLE_EXCEPTIONS)
   list(APPEND LIBCXXABI_SOURCES cxa_exception.cpp)
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -141,6 +141,9 @@
 option(LIBCXXABI_BUILD_EXTERNAL_THREAD_LIBRARY
   "Build libc++abi with an externalized threading library.
This option may only be set to ON when LIBCXXABI_ENABLE_THREADS=ON" OFF)
+option(LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS
+"Build libc++abi with definitions for operator new/delete. Normally libc++
+provides these definitions" OFF)
 option(LIBCXXABI_BUILD_32_BITS "Build 32 bit libc++abi." ${LLVM_BUILD_32_BITS})
 option(LIBCXXABI_INCLUDE_TESTS "Generate build targets for the libc++abi unit 
tests." ${LLVM_INCLUDE_TESTS})
 set(LIBCXXABI_TARGET_TRIPLE "" CACHE STRING "Target triple for cross 
compiling.")
@@ -409,7 +412,10 @@
   endif()
 endif()
 
-if (LIBCXXABI_BUILD_EXTERNAL_THREAD_LIBRARY AND LIBCXXABI_ENABLE_SHARED)
+set(LIBCXXABI_HAS_UNDEFINED_SYMBOLS ((NOT 
LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS)
+OR (LIBCXXABI_BUILD_EXTERNAL_THREAD_LIBRARY AND LIBCXXABI_ENABLE_SHARED)))
+
+if (LIBCXXABI_HAS_UNDEFINED_SYMBOLS)
   # Need to allow unresolved symbols if this is to work with shared library 
builds
   if (APPLE)
 add_link_flags("-undefined dynamic_lookup")


Index: src/CMakeLists.txt
===
--- src/CMakeLists.txt
+++ src/CMakeLists.txt
@@ -20,10 +20,9 @@
   private_typeinfo.cpp
 )
 
-# FIXME: This file should only be compiled in special configurations such
-#  as building the Apple system dylib where libc++abi is expected to provide
-#  the new/delete definitions instead of libc++.
-list(APPEND LIBCXXABI_SOURCES stdlib_new_delete.cpp)
+if (LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS)
+  list(APPEND LIBCXXABI_SOURCES stdlib_new_delete.cpp)
+endif()
 
 if (LIBCXXABI_ENABLE_EXCEPTIONS)
   list(APPEND LIBCXXABI_SOURCES cxa_exception.cpp)
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -141,6 +141,9 @@
 option(LIBCXXABI_BUILD_EXTERNAL_THREAD_LIBRARY
   "Build libc++abi with an externalized threading library.
This option may only be set to ON when LIBCXXABI_ENABLE_THREADS=ON" OFF)
+option(LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS
+"Build libc++abi with definitions for operator new/delete. Normally libc++
+provides these definitions" OFF)
 option(LIBCXXABI_BUILD_32_BITS "Build 32 bit libc++abi." ${LLVM_BUILD_32_BITS})
 option(LIBCXXABI_INCLUDE_TESTS "Generate build targets for the libc++abi unit tests." ${LLVM_INCLUDE_TESTS})
 set(LIBCXXABI_TARGET_TRIPLE "" CACHE STRING "Target triple for cross compiling.")
@@ -409,7 +412,10 @@
   endif()
 endif()
 
-if (LIBCXXABI_BUILD_EXTERNAL_THREAD_LIBRARY AND LIBCXXABI_ENABLE_SHARED)
+set(LIBCXXABI_HAS_UNDEFINED_SYMBOLS ((NOT LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS)
+OR (LIBCXXABI_BUILD_EXTERNAL_THREAD_LIBRARY AND LIBCXXABI_ENABLE_SHARED)))
+
+if (LIBCXXABI_HAS_UNDEFINED_SYMBOLS)
   # Need to allow unresolved symbols if this is to work with shared library builds
   if (APPLE)
 add_link_flags("-undefined dynamic_lookup")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30516: [libc++] Add option to disable new/delete overloads when libc++abi provides them.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF created this revision.

Currently both libc++ and libc++abi provide definitions for operator 
new/delete. However I believe this is incorrect and that one or the other 
should offer them.

This patch adds the CMake option `-DLIBCXX_ENABLE_NEW_DELETE_DEFINITIONS` which 
defaults no `ON` unless `-DLIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS=ON` is 
specified.


https://reviews.llvm.org/D30516

Files:
  CMakeLists.txt
  src/new.cpp


Index: src/new.cpp
===
--- src/new.cpp
+++ src/new.cpp
@@ -53,7 +53,8 @@
 
 }  // std
 
-#if !defined(__GLIBCXX__) && !defined(_LIBCPP_ABI_MICROSOFT)
+#if !defined(__GLIBCXX__) && !defined(_LIBCPP_ABI_MICROSOFT) && \
+!defined(_LIBCPP_DISABLE_NEW_DELETE_DEFINITIONS)
 
 // Implement all new and delete operators as weak definitions
 // in this shared library, so that they can be overridden by programs
@@ -298,4 +299,4 @@
 }
 
 #endif // !_LIBCPP_HAS_NO_ALIGNED_ALLOCATION
-#endif // !__GLIBCXX__ && !_LIBCPP_ABI_MICROSOFT
+#endif // !__GLIBCXX__ && !_LIBCPP_ABI_MICROSOFT && 
!_LIBCPP_DISABLE_NEW_DELETE_DEFINITIONS
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -157,6 +157,16 @@
   "Use and install a linker script for the given ABI library"
   ${ENABLE_LINKER_SCRIPT_DEFAULT_VALUE})
 
+set(ENABLE_NEW_DELETE_DEFAULT ON)
+if (LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS)
+  set(ENABLE_NEW_DELETE_DEFAULT OFF)
+endif()
+
+option(LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS
+"Build libc++ with definitions for operator new/delete. This option can
+be used to disable the definitions when libc++abi is expected to provide
+them" ${ENABLE_NEW_DELETE_DEFAULT})
+
 # Build libc++abi with libunwind. We need this option to determine whether to
 # link with libunwind or libgcc_s while running the test cases.
 option(LIBCXXABI_USE_LLVM_UNWINDER "Build and use the LLVM unwinder." OFF)
@@ -433,6 +443,10 @@
 # library.
 add_definitions(-D_LIBCPP_BUILDING_LIBRARY)
 
+if (NOT LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS)
+  add_definitions(-D_LIBCPP_DISABLE_NEW_DELETE_DEFINITIONS)
+endif()
+
 # Warning flags ===
 add_definitions(-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 add_compile_flags_if_supported(


Index: src/new.cpp
===
--- src/new.cpp
+++ src/new.cpp
@@ -53,7 +53,8 @@
 
 }  // std
 
-#if !defined(__GLIBCXX__) && !defined(_LIBCPP_ABI_MICROSOFT)
+#if !defined(__GLIBCXX__) && !defined(_LIBCPP_ABI_MICROSOFT) && \
+!defined(_LIBCPP_DISABLE_NEW_DELETE_DEFINITIONS)
 
 // Implement all new and delete operators as weak definitions
 // in this shared library, so that they can be overridden by programs
@@ -298,4 +299,4 @@
 }
 
 #endif // !_LIBCPP_HAS_NO_ALIGNED_ALLOCATION
-#endif // !__GLIBCXX__ && !_LIBCPP_ABI_MICROSOFT
+#endif // !__GLIBCXX__ && !_LIBCPP_ABI_MICROSOFT && !_LIBCPP_DISABLE_NEW_DELETE_DEFINITIONS
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -157,6 +157,16 @@
   "Use and install a linker script for the given ABI library"
   ${ENABLE_LINKER_SCRIPT_DEFAULT_VALUE})
 
+set(ENABLE_NEW_DELETE_DEFAULT ON)
+if (LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS)
+  set(ENABLE_NEW_DELETE_DEFAULT OFF)
+endif()
+
+option(LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS
+"Build libc++ with definitions for operator new/delete. This option can
+be used to disable the definitions when libc++abi is expected to provide
+them" ${ENABLE_NEW_DELETE_DEFAULT})
+
 # Build libc++abi with libunwind. We need this option to determine whether to
 # link with libunwind or libgcc_s while running the test cases.
 option(LIBCXXABI_USE_LLVM_UNWINDER "Build and use the LLVM unwinder." OFF)
@@ -433,6 +443,10 @@
 # library.
 add_definitions(-D_LIBCPP_BUILDING_LIBRARY)
 
+if (NOT LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS)
+  add_definitions(-D_LIBCPP_DISABLE_NEW_DELETE_DEFINITIONS)
+endif()
+
 # Warning flags ===
 add_definitions(-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 add_compile_flags_if_supported(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r296716 - Fix python 3 syntax error in sym_diff

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Wed Mar  1 18:03:01 2017
New Revision: 296716

URL: http://llvm.org/viewvc/llvm-project?rev=296716&view=rev
Log:
Fix python 3 syntax error in sym_diff

Modified:
libcxx/trunk/utils/libcxx/sym_check/diff.py

Modified: libcxx/trunk/utils/libcxx/sym_check/diff.py
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/utils/libcxx/sym_check/diff.py?rev=296716&r1=296715&r2=296716&view=diff
==
--- libcxx/trunk/utils/libcxx/sym_check/diff.py (original)
+++ libcxx/trunk/utils/libcxx/sym_check/diff.py Wed Mar  1 18:03:01 2017
@@ -43,7 +43,7 @@ def changed_symbols(old, new):
 continue
 new_sym = _find_by_key(new, old_sym['name'])
 if (new_sym is not None and not new_sym in old
-and cmp(old_sym, new_sym) != 0):
+and old_sym != new_sym):
 changed += [(old_sym, new_sym)]
 return changed
 


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

[libcxxabi] r296715 - Cleanup new/delete definitions

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Wed Mar  1 17:59:34 2017
New Revision: 296715

URL: http://llvm.org/viewvc/llvm-project?rev=296715&view=rev
Log:
Cleanup new/delete definitions

This patch cleans up how libc++abi handles the definitions for new/delete.
It is in preperation for upcoming changes to fix how both libc++ and libc++abi
handle new/delete.

The primary changes in this patch are:

* Move the definitions for bad_array_length and bad_new_array_length
  into stdlib_exception.cpp. This way stdlib_new_delete.cpp only
  contains new/delete.

* Rename cxa_new_delete.cpp -> stdlib_new_delete.cpp for consistency
  with other files.

* Add a FIXME regarding when stdlib_new_delete.cpp is actually compiled
  as part of the dylib.

Added:
libcxxabi/trunk/src/stdlib_new_delete.cpp
  - copied, changed from r296612, libcxxabi/trunk/src/cxa_new_delete.cpp
Removed:
libcxxabi/trunk/src/cxa_new_delete.cpp
Modified:
libcxxabi/trunk/src/CMakeLists.txt
libcxxabi/trunk/src/stdlib_exception.cpp

Modified: libcxxabi/trunk/src/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/src/CMakeLists.txt?rev=296715&r1=296714&r2=296715&view=diff
==
--- libcxxabi/trunk/src/CMakeLists.txt (original)
+++ libcxxabi/trunk/src/CMakeLists.txt Wed Mar  1 17:59:34 2017
@@ -7,7 +7,6 @@ set(LIBCXXABI_SOURCES
   cxa_exception_storage.cpp
   cxa_guard.cpp
   cxa_handlers.cpp
-  cxa_new_delete.cpp
   cxa_unexpected.cpp
   cxa_vector.cpp
   cxa_virtual.cpp
@@ -21,6 +20,11 @@ set(LIBCXXABI_SOURCES
   private_typeinfo.cpp
 )
 
+# FIXME: This file should only be compiled in special configurations such
+#  as building the Apple system dylib where libc++abi is expected to provide
+#  the new/delete definitions instead of libc++.
+list(APPEND LIBCXXABI_SOURCES stdlib_new_delete.cpp)
+
 if (LIBCXXABI_ENABLE_EXCEPTIONS)
   list(APPEND LIBCXXABI_SOURCES cxa_exception.cpp)
   list(APPEND LIBCXXABI_SOURCES cxa_personality.cpp)

Removed: libcxxabi/trunk/src/cxa_new_delete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/src/cxa_new_delete.cpp?rev=296714&view=auto
==
--- libcxxabi/trunk/src/cxa_new_delete.cpp (original)
+++ libcxxabi/trunk/src/cxa_new_delete.cpp (removed)
@@ -1,244 +0,0 @@
-//=== cxa_new_delete.cpp 
--===//
-//
-// 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.
-//
-//
-// This file implements the new and delete operators.
-//===--===//
-
-#define _LIBCPP_BUILDING_NEW
-
-#include "__cxxabi_config.h"
-#include 
-#include 
-
-#if !defined(_THROW_BAD_ALLOC) || !defined(_NOEXCEPT)
-#error _THROW_BAD_ALLOC and _NOEXCEPT libc++ macros must already be defined \
-   by libc++.
-#endif
-
-/*
-[new.delete.single]
-
-* Executes a loop: Within the loop, the function first attempts to allocate
-  the requested storage. Whether the attempt involves a call to the Standard C
-  library function malloc is unspecified.
-
-* Returns a pointer to the allocated storage if the attempt is successful.
-  Otherwise, if the current new_handler (18.6.2.5) is a null pointer value,
-  throws bad_alloc.
-
-* Otherwise, the function calls the current new_handler function (18.6.2.3).
-  If the called function returns, the loop repeats.
-
-* The loop terminates when an attempt to allocate the requested storage is
-  successful or when a called new_handler function does not return.
-*/
-_LIBCXXABI_WEAK
-void *
-operator new(std::size_t size) _THROW_BAD_ALLOC
-{
-if (size == 0)
-size = 1;
-void* p;
-while ((p = std::malloc(size)) == 0)
-{
-std::new_handler nh = std::get_new_handler();
-if (nh)
-nh();
-else
-#ifndef _LIBCXXABI_NO_EXCEPTIONS
-throw std::bad_alloc();
-#else
-break;
-#endif
-}
-return p;
-}
-
-/*
-Note:  The relationships among these operators is both carefully considered
-and standard in C++11.  Please do not change them without fully understanding
-the consequences of doing so.  Reference:
-http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2158.html
-*/
-/*
-[new.delete.single]
-
-Calls operator new(size). If the call returns normally, returns the result of
-that call. Otherwise, returns a null pointer.
-*/
-_LIBCXXABI_WEAK
-void*
-operator new(size_t size, const std::nothrow_t&) _NOEXCEPT
-{
-void* p = 0;
-#ifndef _LIBCXXABI_NO_EXCEPTIONS
-try
-{
-#endif
-p = ::operator new(size);
-#ifndef _LIBCXXABI_NO_EXCEPTIONS
-}
-catch (...)
-{
-}
-#endif
-return p;
-}
-
-/*
-[new.delete.array]
-
-Returns operator new(size).
-*/
-_LIBCXXABI_WEAK
-void*
-operator new[](size_t size) _THROW_BAD_

[libcxx] r296712 - Fix Apple-specific XFAIL directive in libc++ test

[Mehdi Amini via cfe-commits]

Author: mehdi_amini
Date: Wed Mar  1 17:45:03 2017
New Revision: 296712

URL: http://llvm.org/viewvc/llvm-project?rev=296712&view=rev
Log:
Fix Apple-specific XFAIL directive in libc++ test

This tests is failing in XCode 7.0. But Xcode 7.3 that shipped
an updated clang has this test passing. This is fixing green dragon
which runs this configuration.

Modified:

libcxx/trunk/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.cons/default.pass.cpp

Modified: 
libcxx/trunk/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.cons/default.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.cons/default.pass.cpp?rev=296712&r1=296711&r2=296712&view=diff
==
--- 
libcxx/trunk/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.cons/default.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.cons/default.pass.cpp
 Wed Mar  1 17:45:03 2017
@@ -9,7 +9,7 @@
 
 // Usage of is_trivially_constructible is broken with these compilers.
 // See https://bugs.llvm.org/show_bug.cgi?id=31016
-// XFAIL: clang-3.7, apple-clang-7, apple-clang-7.0
+// XFAIL: clang-3.7, apple-clang-7.0
 
 // 
 


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

Re: r296554 - [PS4] Set our default dialect to C++11. NFC for other targets.

[Mehdi Amini via cfe-commits]

> On Mar 1, 2017, at 3:24 PM, Sean Silva  wrote:
> 
> 
> 
> On Wed, Mar 1, 2017 at 10:35 AM, Mehdi Amini  > wrote:
> I’m not sure I find this nice to see this upstream.
> 
> I not fond in general of this kind of difference in behavior. I don’t think 
> it is good for clang to have different default for this kind of settings 
> depending on the platform. It does not provide a very good user experience 
> from a cross-platform point of view (i.e. my compiler behaves very 
> differently when I target one platform instead of another).
> 
> What I like about it is that the upstream PS4 bots now test that we don't 
> depend on the C++98 default language standard in tests, which is net positive 
> IMO since it facilitates future changes. Should this be a point of 
> vendor/platform extensibility? That's a question for cfe-dev, but I don't 
> think it's unreasonable. (see also: PS4 has -fno-rtti and -fno-exceptions by 
> default, even though users already know to pass the right flags and expect to 
> have to).

I rather have the driver rejecting “unsupported configuration”, requiring the 
right flags, instead of diverging the default.

— 
Mehdi


> 
> 
> 
> 
>> On Feb 28, 2017, at 11:22 PM, Sean Silva via cfe-commits 
>> mailto:cfe-commits@lists.llvm.org>> wrote:
>> 
>> Nice!
>> 
>> -- Sean Silva
>> 
>> On Tue, Feb 28, 2017 at 5:01 PM, Paul Robinson via cfe-commits 
>> mailto:cfe-commits@lists.llvm.org>> wrote:
>> Author: probinson
>> Date: Tue Feb 28 19:01:10 2017
>> New Revision: 296554
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=296554&view=rev 
>> 
>> Log:
>> [PS4] Set our default dialect to C++11. NFC for other targets.
>> Reapplies r296209 now that r296549 has fixed what really seems to be
>> the last problematic test.
>> 
>> Modified:
>> cfe/trunk/lib/Frontend/CompilerInvocation.cpp
>> 
>> Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=296554&r1=296553&r2=296554&view=diff
>>  
>> 
>> ==
>> --- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
>> +++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Tue Feb 28 19:01:10 2017
>> @@ -1582,7 +1582,11 @@ void CompilerInvocation::setLangDefaults
>>  case IK_PreprocessedCXX:
>>  case IK_ObjCXX:
>>  case IK_PreprocessedObjCXX:
>> -  LangStd = LangStandard::lang_gnucxx98;
>> +  // The PS4 uses C++11 as the default C++ standard.
>> +  if (T.isPS4())
>> +LangStd = LangStandard::lang_gnucxx11;
>> +  else
>> +LangStd = LangStandard::lang_gnucxx98;
>>break;
>>  case IK_RenderScript:
>>LangStd = LangStandard::lang_c99;
>> 
>> 
>> ___
>> 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 
>> 
> 
> 

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

[PATCH] D30087: [Driver] Unify linking of OpenMP runtime

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added inline comments.



Comment at: lib/Driver/Tools.cpp:10334
 
-  if (Args.hasFlag(options::OPT_fopenmp, options::OPT_fopenmp_EQ,
-   options::OPT_fno_openmp, false)) {
+  // FIXME: Exclude this for platforms with libgomp that don't require
+  // librt. Most modern Linux platforms require it, but some may not.

Now that you've moved the comment, it is not clear what "this" means here. 
Please reword this to say that you should only pass true for GompNeedsRT on 
platforms that really need it.


https://reviews.llvm.org/D30087



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

[libcxx] r296704 - Fix Apple-specific XFAIL directive in libc++ test

[Mehdi Amini via cfe-commits]

Author: mehdi_amini
Date: Wed Mar  1 17:14:34 2017
New Revision: 296704

URL: http://llvm.org/viewvc/llvm-project?rev=296704&view=rev
Log:
Fix Apple-specific XFAIL directive in libc++ test

These tests are failing in XCode 8.0, 8.1, and 8.2, but not in Xcode
8.3. Annoyingly the version numbering for clang does not follow Xcode
and is bumped to 8.1 only in Xcode 8.3. So Xfailing apple-clang-8.0
should catch all cases here.

Modified:

libcxx/trunk/test/libcxx/utilities/variant/variant.variant/variant.assign/copy.pass.cpp

libcxx/trunk/test/libcxx/utilities/variant/variant.variant/variant.assign/move.pass.cpp

libcxx/trunk/test/std/utilities/variant/variant.variant/variant.status/index.pass.cpp

libcxx/trunk/test/std/utilities/variant/variant.variant/variant.status/valueless_by_exception.pass.cpp

Modified: 
libcxx/trunk/test/libcxx/utilities/variant/variant.variant/variant.assign/copy.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/utilities/variant/variant.variant/variant.assign/copy.pass.cpp?rev=296704&r1=296703&r2=296704&view=diff
==
--- 
libcxx/trunk/test/libcxx/utilities/variant/variant.variant/variant.assign/copy.pass.cpp
 (original)
+++ 
libcxx/trunk/test/libcxx/utilities/variant/variant.variant/variant.assign/copy.pass.cpp
 Wed Mar  1 17:14:34 2017
@@ -12,7 +12,7 @@
 
 // The following compilers don't generate constexpr special members correctly.
 // XFAIL: clang-3.5, clang-3.6, clang-3.7, clang-3.8
-// XFAIL: apple-clang-6, apple-clang-7, apple-clang-8
+// XFAIL: apple-clang-6, apple-clang-7, apple-clang-8.0
 
 // 
 

Modified: 
libcxx/trunk/test/libcxx/utilities/variant/variant.variant/variant.assign/move.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/utilities/variant/variant.variant/variant.assign/move.pass.cpp?rev=296704&r1=296703&r2=296704&view=diff
==
--- 
libcxx/trunk/test/libcxx/utilities/variant/variant.variant/variant.assign/move.pass.cpp
 (original)
+++ 
libcxx/trunk/test/libcxx/utilities/variant/variant.variant/variant.assign/move.pass.cpp
 Wed Mar  1 17:14:34 2017
@@ -12,7 +12,7 @@
 
 // The following compilers don't generate constexpr special members correctly.
 // XFAIL: clang-3.5, clang-3.6, clang-3.7, clang-3.8
-// XFAIL: apple-clang-6, apple-clang-7, apple-clang-8
+// XFAIL: apple-clang-6, apple-clang-7, apple-clang-8.0
 
 // 
 

Modified: 
libcxx/trunk/test/std/utilities/variant/variant.variant/variant.status/index.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/variant/variant.variant/variant.status/index.pass.cpp?rev=296704&r1=296703&r2=296704&view=diff
==
--- 
libcxx/trunk/test/std/utilities/variant/variant.variant/variant.status/index.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/utilities/variant/variant.variant/variant.status/index.pass.cpp
 Wed Mar  1 17:14:34 2017
@@ -16,7 +16,7 @@
 // virtual, private, nor protected (e.g. ConstexprTestTypes:::NoCtors).
 // XFAIL: gcc-5, gcc-6
 // XFAIL: clang-3.5, clang-3.6, clang-3.7, clang-3.8
-// XFAIL: apple-clang-6, apple-clang-7, apple-clang-8
+// XFAIL: apple-clang-6, apple-clang-7, apple-clang-8.0
 
 // 
 

Modified: 
libcxx/trunk/test/std/utilities/variant/variant.variant/variant.status/valueless_by_exception.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/variant/variant.variant/variant.status/valueless_by_exception.pass.cpp?rev=296704&r1=296703&r2=296704&view=diff
==
--- 
libcxx/trunk/test/std/utilities/variant/variant.variant/variant.status/valueless_by_exception.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/utilities/variant/variant.variant/variant.status/valueless_by_exception.pass.cpp
 Wed Mar  1 17:14:34 2017
@@ -16,7 +16,7 @@
 // virtual, private, nor protected (e.g. ConstexprTestTypes:::NoCtors).
 // XFAIL: gcc-5, gcc-6
 // XFAIL: clang-3.5, clang-3.6, clang-3.7, clang-3.8
-// XFAIL: apple-clang-6, apple-clang-7, apple-clang-8
+// XFAIL: apple-clang-6, apple-clang-7, apple-clang-8.0
 
 // 
 


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

Re: r296554 - [PS4] Set our default dialect to C++11. NFC for other targets.

[Sean Silva via cfe-commits]

On Wed, Mar 1, 2017 at 10:35 AM, Mehdi Amini  wrote:

> I’m not sure I find this nice to see this upstream.
>
> I not fond in general of this kind of difference in behavior. I don’t
> think it is good for clang to have different default for this kind of
> settings depending on the platform. It does not provide a very good user
> experience from a cross-platform point of view (i.e. my compiler behaves
> very differently when I target one platform instead of another).
>

What I like about it is that the upstream PS4 bots now test that we don't
depend on the C++98 default language standard in tests, which is net
positive IMO since it facilitates future changes. Should this be a point of
vendor/platform extensibility? That's a question for cfe-dev, but I don't
think it's unreasonable. (see also: PS4 has -fno-rtti and -fno-exceptions
by default, even though users already know to pass the right flags and
expect to have to).

-- Sean Silva


>
> —
> Mehdi
>
>
>
> On Feb 28, 2017, at 11:22 PM, Sean Silva via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> Nice!
>
> -- Sean Silva
>
> On Tue, Feb 28, 2017 at 5:01 PM, Paul Robinson via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: probinson
>> Date: Tue Feb 28 19:01:10 2017
>> New Revision: 296554
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=296554&view=rev
>> Log:
>> [PS4] Set our default dialect to C++11. NFC for other targets.
>> Reapplies r296209 now that r296549 has fixed what really seems to be
>> the last problematic test.
>>
>> Modified:
>> cfe/trunk/lib/Frontend/CompilerInvocation.cpp
>>
>> Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/
>> CompilerInvocation.cpp?rev=296554&r1=296553&r2=296554&view=diff
>> 
>> ==
>> --- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
>> +++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Tue Feb 28 19:01:10
>> 2017
>> @@ -1582,7 +1582,11 @@ void CompilerInvocation::setLangDefaults
>>  case IK_PreprocessedCXX:
>>  case IK_ObjCXX:
>>  case IK_PreprocessedObjCXX:
>> -  LangStd = LangStandard::lang_gnucxx98;
>> +  // The PS4 uses C++11 as the default C++ standard.
>> +  if (T.isPS4())
>> +LangStd = LangStandard::lang_gnucxx11;
>> +  else
>> +LangStd = LangStandard::lang_gnucxx98;
>>break;
>>  case IK_RenderScript:
>>LangStd = LangStandard::lang_c99;
>>
>>
>> ___
>> 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
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27627: [WIP] Supporting C++ based kernel languages on AMDGPU Target

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 90243.
yaxunl edited the summary of this revision.

https://reviews.llvm.org/D27627

Files:
  include/clang/AST/ASTContext.h
  include/clang/Basic/TargetInfo.h
  lib/AST/ASTContext.cpp
  lib/Basic/TargetInfo.cpp
  lib/Basic/Targets.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/CGClass.cpp
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CGDeclCXX.cpp
  lib/CodeGen/CGException.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGExprCXX.cpp
  lib/CodeGen/CGExprConstant.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/CodeGen/CGGPUBuiltin.cpp
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGVTT.cpp
  lib/CodeGen/CGVTables.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenTypes.cpp
  lib/CodeGen/CodeGenTypes.h
  lib/CodeGen/ItaniumCXXABI.cpp
  test/CodeGenCUDA/address-spaces.cu
  test/CodeGenCUDA/convergent.cu
  test/CodeGenCUDA/device-var-init.cu
  test/CodeGenCUDA/device-vtable.cu
  test/CodeGenCUDA/filter-decl.cu
  test/CodeGenCUDA/function-overload.cu
  test/CodeGenCUDA/kernel-args-alignment.cu
  test/CodeGenCUDA/llvm-used.cu
  test/CodeGenCUDA/printf.cu
  test/CodeGenCXX/amdgcn-global-init.cpp
  test/OpenMP/nvptx_parallel_codegen.cpp

Index: test/OpenMP/nvptx_parallel_codegen.cpp
===
--- test/OpenMP/nvptx_parallel_codegen.cpp
+++ test/OpenMP/nvptx_parallel_codegen.cpp
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-64
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple i386-unknown-unknown -fopenmp-targets=nvptx-nvidia-cuda -emit-llvm-bc %s -o %t-x86-host.bc
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple i386-unknown-unknown -fopenmp-targets=amdgcn -emit-llvm-bc %s -o %t-x86-host.bc
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx-unknown-unknown -fopenmp-targets=nvptx-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-x86-host.bc -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-32
 // RUN: %clang_cc1 -verify -fopenmp -fexceptions -fcxx-exceptions -x c++ -triple nvptx-unknown-unknown -fopenmp-targets=nvptx-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-x86-host.bc -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-32
 // expected-no-diagnostics
@@ -62,14 +63,14 @@
   return a;
 }
 
-  // CHECK-NOT: define {{.*}}void {{@__omp_offloading_.+template.+l17}}_worker()
+  // CHECK-NOT: define {{.*}}void {{@__omp_offloading_.+template.+l18}}_worker()
 
 
 
 
 
 
-  // CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+template.+l26}}_worker()
+  // CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+template.+l27}}_worker()
   // CHECK-DAG: [[OMP_EXEC_STATUS:%.+]] = alloca i8,
   // CHECK-DAG: [[OMP_WORK_FN:%.+]] = alloca i8*,
   // CHECK: store i8* null, i8** [[OMP_WORK_FN]],
@@ -122,7 +123,7 @@
   // CHECK: [[EXIT]]
   // CHECK: ret void
 
-  // CHECK: define {{.*}}void [[T6:@__omp_offloading_.+template.+l26]](i[[SZ:32|64]]
+  // CHECK: define {{.*}}void [[T6:@__omp_offloading_.+template.+l27]](i[[SZ:32|64]]
   // Create local storage for each capture.
   // CHECK:  [[LOCAL_A:%.+]] = alloca i[[SZ]],
   // CHECK-DAG:  store i[[SZ]] [[ARG_A:%.+]], i[[SZ]]* [[LOCAL_A]]
@@ -194,7 +195,7 @@
 
 
 
-  // CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+template.+l43}}_worker()
+  // CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+template.+l44}}_worker()
   // CHECK-DAG: [[OMP_EXEC_STATUS:%.+]] = alloca i8,
   // CHECK-DAG: [[OMP_WORK_FN:%.+]] = alloca i8*,
   // CHECK: store i8* null, i8** [[OMP_WORK_FN]],
@@ -238,7 +239,7 @@
   // CHECK: [[EXIT]]
   // CHECK: ret void
 
-  // CHECK: define {{.*}}void [[T6:@__omp_offloading_.+template.+l43]](i[[SZ:32|64]]
+  // CHECK: define {{.*}}void [[T6:@__omp_offloading_.+template.+l44]](i[[SZ:32|64]]
   // Create local storage for each capture.
   // CHECK:  [[LOCAL_N:%.+]] = alloca i[[SZ]],
   // CHECK:  [[LOCAL_A:%.+]] = alloca i[[SZ]],
Index: test/CodeGenCXX/amdgcn-global-init.cpp
===
--- /dev/null
+++ test/CodeGenCXX/amdgcn-global-init.cpp
@@ -0,0 +1,211 @@
+// RUN: %clang_cc1 -std=c++11 -triple=amdgcn-amd-amdhsa -emit-llvm -fexceptions %s -o - |FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -triple=amdgcn-amd-amdhsa -emit-llvm %s -o - |FileCheck -check-prefix CHECK-NOEXC %s
+// RUN: %clang_cc1 -std=c++11 -triple=amdgcn-amd-amdhsa -emit-llvm \
+// RUN: -momit-leaf-frame-pointer -mdisable-fp-elim %s -o - \
+// RUN:   | FileCheck -check-prefix CHECK-FP %s
+
+struct A {
+  A();

[PATCH] D29464: [MinGWToolChain] Don't use GCC headers on Win32

[Mateusz Mikuła via Phabricator via cfe-commits]

mati865 added a comment.

Ping.


https://reviews.llvm.org/D29464



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

[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

In https://reviews.llvm.org/D30158#690032, @madsravn wrote:

> Looks good for the two tests the are for `random_shuffle` in llvm libc++.


There were a lot more some time ago, before @mclow.lists performed this 
transformation on libc++'s testsuite. You might want to try this on the 
revision before that happened.


https://reviews.llvm.org/D30158



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

[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.

[Mads Ravn via Phabricator via cfe-commits]

madsravn added a comment.

Looks good for the two tests the are for `random_shuffle` in llvm libc++.


https://reviews.llvm.org/D30158



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

[PATCH] D30214: [Driver] Search for libc++ headers in ResourceDir

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

libc++ headers should not be installed in the resource dir.


https://reviews.llvm.org/D30214



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

[libcxx] r296685 - Generate the test configuration even when LIBCXX_INCLUDE_TESTS=OFF.

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Wed Mar  1 15:53:30 2017
New Revision: 296685

URL: http://llvm.org/viewvc/llvm-project?rev=296685&view=rev
Log:
Generate the test configuration even when LIBCXX_INCLUDE_TESTS=OFF.

This patch changes the CMake configuration so that it always
generates the test/lit.site.cfg file, even when testing is disabled.

This allows users to test libc++ without requiring them to have
a full LLVM checkout on their machine.

Modified:
libcxx/trunk/CMakeLists.txt
libcxx/trunk/test/CMakeLists.txt

Modified: libcxx/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/CMakeLists.txt?rev=296685&r1=296684&r2=296685&view=diff
==
--- libcxx/trunk/CMakeLists.txt (original)
+++ libcxx/trunk/CMakeLists.txt Wed Mar  1 15:53:30 2017
@@ -19,7 +19,6 @@ set(CMAKE_MODULE_PATH
   ${CMAKE_MODULE_PATH}
   )
 
-
 if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
   project(libcxx CXX C)
 
@@ -600,10 +599,15 @@ add_subdirectory(lib)
 if (LIBCXX_INCLUDE_BENCHMARKS)
   add_subdirectory(benchmarks)
 endif()
+
+# Create the lit.site.cfg file even when LIBCXX_INCLUDE_TESTS is OFF or
+# LLVM_FOUND is OFF. This allows users to run the tests manually using
+# LIT without requiring a full LLVM checkout.
+add_subdirectory(test)
 if (LIBCXX_INCLUDE_TESTS)
-  add_subdirectory(test)
   add_subdirectory(lib/abi)
 endif()
+
 if (LIBCXX_INCLUDE_DOCS)
   add_subdirectory(docs)
 endif()

Modified: libcxx/trunk/test/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/CMakeLists.txt?rev=296685&r1=296684&r2=296685&view=diff
==
--- libcxx/trunk/test/CMakeLists.txt (original)
+++ libcxx/trunk/test/CMakeLists.txt Wed Mar  1 15:53:30 2017
@@ -1,5 +1,3 @@
-include(AddLLVM) # for add_lit_testsuite
-
 macro(pythonize_bool var)
   if (${var})
 set(${var} True)
@@ -66,12 +64,15 @@ if (LIBCXX_BUILD_EXTERNAL_THREAD_LIBRARY
   list(APPEND LIBCXX_TEST_DEPS cxx_external_threads)
 endif()
 
-add_lit_testsuite(check-cxx
-  "Running libcxx tests"
-  ${CMAKE_CURRENT_BINARY_DIR}
-  DEPENDS cxx ${LIBCXX_TEST_DEPS})
+if (LIBCXX_INCLUDE_TESTS)
+  include(AddLLVM) # for add_lit_testsuite
+  add_lit_testsuite(check-cxx
+"Running libcxx tests"
+${CMAKE_CURRENT_BINARY_DIR}
+DEPENDS cxx ${LIBCXX_TEST_DEPS})
 
-add_custom_target(check-libcxx DEPENDS check-cxx)
+  add_custom_target(check-libcxx DEPENDS check-cxx)
+endif()
 
 if (LIBCXX_GENERATE_COVERAGE)
   include(CodeCoverage)


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

[PATCH] D28348: [analyzer] Taught the analyzer about Glib API to check Memory-leak

[Anna Zaks via Phabricator via cfe-commits]

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

> I am not clear why need to calculate the precise allocated size?

The information generated by this checker is used for array bounds checking. 
For example, see https://reviews.llvm.org/D24307

This patch looks good. Do you have commit access or should I commit it on your 
behalf?


Repository:
  rL LLVM

https://reviews.llvm.org/D28348



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

[PATCH] D30214: [Driver] Search for libc++ headers in ResourceDir

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

Ping


https://reviews.llvm.org/D30214



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

[PATCH] D30087: [Driver] Unify linking of OpenMP runtime

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

Ping


https://reviews.llvm.org/D30087



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

[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.

[Mads Ravn via Phabricator via cfe-commits]

madsravn added a comment.

In https://reviews.llvm.org/D30158#689904, @aaron.ballman wrote:

> Out of curiosity, have you run this over libc++ or libstdc++ test suites 
> involving `std::random_shuffle`? If so, were the results acceptable?


I haven't. Good idea. I will get onto that. I don't have either, so I will just 
fetch them and set them up.


https://reviews.llvm.org/D30158



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

[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.

[Mads Ravn via Phabricator via cfe-commits]

madsravn updated this revision to Diff 90223.
madsravn added a comment.

Last small changes based on comments.


https://reviews.llvm.org/D30158

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp
  clang-tidy/modernize/ReplaceRandomShuffleCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-replace-random-shuffle.rst
  test/clang-tidy/modernize-replace-random-shuffle.cpp

Index: test/clang-tidy/modernize-replace-random-shuffle.cpp
===
--- test/clang-tidy/modernize-replace-random-shuffle.cpp
+++ test/clang-tidy/modernize-replace-random-shuffle.cpp
@@ -0,0 +1,58 @@
+// RUN: %check_clang_tidy %s modernize-replace-random-shuffle %t -- -- -std=c++11
+
+//CHECK-FIXES: #include 
+
+namespace std {
+template  struct vec_iterator {
+  T *ptr;
+  vec_iterator operator++(int);
+};
+
+template  struct vector {
+  typedef vec_iterator iterator;
+
+  iterator begin();
+  iterator end();
+};
+
+template 
+void random_shuffle(FwIt begin, FwIt end);
+
+template 
+void random_shuffle(FwIt begin, FwIt end, randomFunc& randomfunc);
+
+template 
+void shuffle(FwIt begin, FwIt end);
+} // namespace std
+
+// Random Func
+int myrandom (int i) { return i;}
+
+using namespace std;
+
+int main() {
+  std::vector vec;
+
+  std::random_shuffle(vec.begin(), vec.end());
+  // CHECK-MESSAGE: [[@LINE-1]]:3: warning: 'std::random_shuffle' has been removed in C++17; use 'std::shuffle' instead
+  // CHECK-FIXES: std::shuffle(vec.begin(), vec.end(), std::mt19937(std::random_device()()));
+
+
+  std::shuffle(vec.begin(), vec.end());
+
+  random_shuffle(vec.begin(), vec.end());
+  // CHECK-MESSAGE: [[@LINE-1]]:3: warning: 'std::random_shuffle' has been removed in C++17; use 'std::shuffle' instead
+  // CHECK-FIXES: shuffle(vec.begin(), vec.end(), std::mt19937(std::random_device()()));
+  
+  std::random_shuffle(vec.begin(), vec.end(), myrandom);
+  // CHECK-MESSAGE: [[@LINE-1]]:3: warning: 'std::random_shuffle' has been removed in C++17; use 'std::shuffle' and an alternative random mechanism instead
+  // CHECK-FIXES: std::shuffle(vec.begin(), vec.end(), std::mt19937(std::random_device()()));
+
+  random_shuffle(vec.begin(), vec.end(), myrandom);
+  // CHECK-MESSAGE: [[@LINE-1]]:3: warning: 'std::random_shuffle' has been removed in C++17; use 'std::shuffle' and an alternative random mechanism instead
+  // CHECK-FIXES: shuffle(vec.begin(), vec.end(), std::mt19937(std::random_device()()));
+
+  shuffle(vec.begin(), vec.end());
+
+  return 0;
+}
Index: docs/clang-tidy/checks/modernize-replace-random-shuffle.rst
===
--- docs/clang-tidy/checks/modernize-replace-random-shuffle.rst
+++ docs/clang-tidy/checks/modernize-replace-random-shuffle.rst
@@ -0,0 +1,28 @@
+.. title:: clang-tidy - modernize-replace-random-shuffle
+
+modernize-replace-random-shuffle
+
+
+This check will find occurrences of ``std::random_shuffle`` and replace it with ``std::shuffle``. In C++17 ``std::random_shuffle`` will no longer be available and thus we need to replace it.
+
+Below are two examples of what kind of occurrences will be found and two examples of what it will be replaced with.
+
+.. code-block:: c++
+
+  std::vector v;
+
+  // First example
+  std::random_shuffle(vec.begin(), vec.end());
+
+  // Second example
+  std::random_shuffle(vec.begin(), vec.end(), randomFun);
+
+Both of these examples will be replaced with:
+
+.. code-block:: c++
+
+  std::shuffle(vec.begin(), vec.end(), std::mt19937(std::random_device()()));
+
+The second example will also receive a warning that ``randomFunc`` is no longer supported in the same way as before so if the user wants the same functionality, the user will need to change the implementation of the ``randomFunc``.
+
+One thing to be aware of here is that ``std::random_device`` is quite expensive to initialize. So if you are using the code in a performance critical place, you probably want to initialize it elsewhere. 
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -110,6 +110,7 @@
modernize-raw-string-literal
modernize-redundant-void-arg
modernize-replace-auto-ptr
+   modernize-replace-random-shuffle
modernize-return-braced-init-list
modernize-shrink-to-fit
modernize-use-auto
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -72,6 +72,11 @@
 
   Finds uses of inline assembler.
 
+- New `modernize-replace-random-shuffle
+  `_ check
+
+  Finds and fixes usage of ``std::random_shuffle`` as the fun

[PATCH] D30157: [analyzer] Improve valist check

[Anna Zaks via Phabricator via cfe-commits]

zaks.anna added inline comments.



Comment at: test/Analysis/valist-uninitialized-no-undef.c:25
+  va_list va;
+  vprintf(isstring ? "%s" : "%d", va); //expected-warning{{Function 'vprintf' 
is called with an uninitialized va_list argument}} expected-note{{Function 
'vprintf' is called with an uninitialized va_list argument}} 
expected-note{{Assuming 'isstring' is 0}} expected-note{{'?' condition is 
false}}
+}

Please, split the long "expected" lines into multiple lines - one per note. It 
will improve readability in non-wrapping editors. Thanks!


https://reviews.llvm.org/D30157



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

[PATCH] D30341: [analyzer] clarify error messages about uninitialized function arguments

[Anna Zaks via Phabricator via cfe-commits]

zaks.anna added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp:211
   // Generate a report for this bug.
-  StringRef Desc =
-  describeUninitializedArgumentInCall(Call, IsFirstArgument);
+  std::string Desc =
+  describeUninitializedArgumentInCall(Call, ArgumentNumber);

danielmarjamaki wrote:
> zaks.anna wrote:
> > Have you considered using  llvm::raw_svector_ostream here as well as 
> > passing it an argument to describeUninitializedArgumentInCall? For example, 
> > see  MallocChecker.cpp.
> I changed so describeUninitializedArgumentInCall() returns an llvm::Twine 
> instead of std::string. hope you like it.
> 
I do not think it's safe to use llvm:Twine here. See 
http://llvm.org/docs/ProgrammersManual.html#the-twine-class

How about using llvm::raw_svector_ostream as I suggested?


Repository:
  rL LLVM

https://reviews.llvm.org/D30341



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

[PATCH] D30489: [analyzer] catch out of bounds for VLA

[Anna Zaks via Phabricator via cfe-commits]

zaks.anna added a comment.

Gábor's suggestion sounds good to me. I think ArrayBoundCheckerV2 checker has a 
higher chance to be productized / moved out of alpha in the future.

Should we just remove ArrayBoundChecker.cpp or is there a value in keeping it 
around?


Repository:
  rL LLVM

https://reviews.llvm.org/D30489



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

[PATCH] D30326: [MS-ABI] Allow #pragma section to choose for ZI data

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

This functionality doesn't feel worth a driver flag. It customizes a very small 
aspect of a Microsoft extension that was only implemented for compatibility. It 
also reimplements the logic that LLVM uses in the backend to put global 
variables in .bss or .data. Can you elaborate on the actual use case? Is the 
goal to put all globals that would normally live in .bss into a custom section? 
If so, I think we should implement our own pragma to do that rather than 
piggybacking on the Microsoft one.




Comment at: lib/AST/Expr.cpp:2841
+/// Returns true if the initializer is all zeros.
+bool Expr::isZeroInitializer(ASTContext &Ctx, bool IsForRef,
+ const Expr **Culprit) const {

I seriously doubt we need this much code to check if something is a zero. I'm 
sure we already have a way to do this.


https://reviews.llvm.org/D30326



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

[PATCH] D30406: [Analyzer] Add support for displaying cross-file diagnostic paths in HTML output

[Anna Zaks via Phabricator via cfe-commits]

zaks.anna added a comment.

No multi-file support is a long outstanding limitation of scan-build html 
output. Great to see the patch!! Thank you for working on it!

> It's not as immediately clear this is a multi-file output.

In addition to Artem's suggestions, you might want to insert  multiple lines of 
padding to make the distinction on the border more clear. I think it would help 
especially when scrolling a large report like in the link for the Linux source.

Also, could you put this behind an option or introduce a new format like 
-analyzer-output=plist-multi-file but for html? Just in case someone is relying 
on a single file output format, we'd want to have an option to give them to 
turn it on.


https://reviews.llvm.org/D30406



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

RE: r296554 - [PS4] Set our default dialect to C++11. NFC for other targets.

[Robinson, Paul via cfe-commits]

Probably better to debate this on the cfe-dev thread "Setting default dialect 
to C++11" than here.
That said, it's far from the only target-dependent setting in the driver.  
There are things ranging from the default DWARF version to whether exceptions 
are on by default to whether Microsoft extensions are on by default.  
Personally I think mixing things up gets us better coverage; this is why we 
like having bots for a variety of platforms, and it's not just for the obvious 
backend reasons.
--paulr

From: mehdi.am...@apple.com [mailto:mehdi.am...@apple.com]
Sent: Wednesday, March 01, 2017 10:36 AM
To: Sean Silva
Cc: Robinson, Paul; cfe-commits
Subject: Re: r296554 - [PS4] Set our default dialect to C++11. NFC for other 
targets.

I’m not sure I find this nice to see this upstream.

I not fond in general of this kind of difference in behavior. I don’t think it 
is good for clang to have different default for this kind of settings depending 
on the platform. It does not provide a very good user experience from a 
cross-platform point of view (i.e. my compiler behaves very differently when I 
target one platform instead of another).

—
Mehdi



On Feb 28, 2017, at 11:22 PM, Sean Silva via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:

Nice!

-- Sean Silva

On Tue, Feb 28, 2017 at 5:01 PM, Paul Robinson via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: probinson
Date: Tue Feb 28 19:01:10 2017
New Revision: 296554

URL: http://llvm.org/viewvc/llvm-project?rev=296554&view=rev
Log:
[PS4] Set our default dialect to C++11. NFC for other targets.
Reapplies r296209 now that r296549 has fixed what really seems to be
the last problematic test.

Modified:
cfe/trunk/lib/Frontend/CompilerInvocation.cpp

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=296554&r1=296553&r2=296554&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Tue Feb 28 19:01:10 2017
@@ -1582,7 +1582,11 @@ void CompilerInvocation::setLangDefaults
 case IK_PreprocessedCXX:
 case IK_ObjCXX:
 case IK_PreprocessedObjCXX:
-  LangStd = LangStandard::lang_gnucxx98;
+  // The PS4 uses C++11 as the default C++ standard.
+  if (T.isPS4())
+LangStd = LangStandard::lang_gnucxx11;
+  else
+LangStd = LangStandard::lang_gnucxx98;
   break;
 case IK_RenderScript:
   LangStd = LangStandard::lang_c99;


___
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

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

[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Out of curiosity, have you run this over libc++ or libstdc++ test suites 
involving `std::random_shuffle`? If so, were the results acceptable?

I think this generally LGTM (aside from the minor nit about naming), but I'd 
like to hear from @mclow.lists where this meets his needs.




Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:77
+
+  auto Diag = [=]() {
+std::string Message = ReplaceMessage;

madsravn wrote:
> aaron.ballman wrote:
> > madsravn wrote:
> > > aaron.ballman wrote:
> > > > madsravn wrote:
> > > > > aaron.ballman wrote:
> > > > > > madsravn wrote:
> > > > > > > aaron.ballman wrote:
> > > > > > > > Is there a reason this needs to capture everything by copy? 
> > > > > > > > Also, no need for the empty parens. Actually, is the lambda 
> > > > > > > > even necessary at all?
> > > > > > > Is it OK to capture by reference then? Or how do we want it in 
> > > > > > > llvm? 
> > > > > > > 
> > > > > > > We need the lambda, because first I need to create the diag with 
> > > > > > > a message based on the count of arguments and then I need to find 
> > > > > > > fixits based on the same count. Example: 
> > > > > > > 
> > > > > > > 
> > > > > > > ```
> > > > > > > string message = "Message for 2 arguments";
> > > > > > > if(argumentCount == 3) {
> > > > > > >   message = "Message for 3 arguments";
> > > > > > > }
> > > > > > > auto Diag = diag(startLoc(), message);
> > > > > > > if(argumentCount == 3) {
> > > > > > >   Diag << FixitHint::FixForThreeArguments();
> > > > > > > } else {
> > > > > > >   Diag << FixitHint::FixForTwoArguments();
> > > > > > > }
> > > > > > > ```
> > > > > > > 
> > > > > > > So the idea with the lambda is to avoid doing the same 
> > > > > > > if-statement twice. 
> > > > > > But you call the lambda immediately rather than store it and reuse 
> > > > > > it? It seems like you should be able to hoist a `DiagnosticBuilder` 
> > > > > > variable outside of the if statement and skip the lambda entirely.
> > > > > I am not sure what you mean by this. Can you elaborate? Can you give 
> > > > > a short example how I would hoist a `DiagnosticBuilder` out?
> > > > > 
> > > > > I think I tried something like that, but it was not an option. 
> > > > It's entirely possible I'm missing something (I'm distracted with 
> > > > meetings this week), but I was envisioning:
> > > > ```
> > > > DiagnosticBuilder Diag;
> > > > if (MatchedCallExpr->getNumArgs() == 3) {
> > > >   Diag =
> > > >   diag(MatchedCallExpr->getLocStart(),
> > > >"'std::random_shuffle' has been removed in C++17; use "
> > > >"'std::shuffle' and an alternative random mechanism 
> > > > instead");
> > > >   Diag << FixItHint::CreateReplacement(
> > > >   MatchedArgumentThree->getSourceRange(),
> > > >   "std::mt19937(std::random_device()())");
> > > > } else {
> > > >   Diag = diag(MatchedCallExpr->getLocStart(),
> > > > "'std::random_shuffle' has been removed in C++17; 
> > > > use "
> > > > "'std::shuffle' instead");
> > > >   Diag << FixItHint::CreateInsertion(
> > > >   MatchedCallExpr->getRParenLoc(),
> > > >   ", std::mt19937(std::random_device()())");
> > > > }
> > > > ```
> > > The constructor for `DiagnosticBuilder` is private. So I cannot do that. 
> > > The idea had crossed my mind, but I think the lambda expression is nicer 
> > > to look at. 
> > > 
> > > Should I investigate if there is another way to hoist the 
> > > `DiagnosticBuilder` out, like using `diag()` to make a dummy 
> > > `DiagnosticBuilder` outside and then use the copy constructor to assign 
> > > inside the if-statement? Or can we live with the lambda expression? 
> > Ah, okay, that was the bit I was missing. Thank you for being patient. I 
> > think the lambda (with the reference capture) is fine as-is.
> > Thank you for being patient.
> 
> Right back at you. We are working towards the same goal after all :) 
> 
> For future reference: Should I try to avoid lambda expressions like this? 
> 
> 
No, this sort of expression is fine when it makes the code more simple.


https://reviews.llvm.org/D30158



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

r296664 - clang-format: [JS] Properly format object literals with shorthands.

[Daniel Jasper via cfe-commits]

Author: djasper
Date: Wed Mar  1 13:47:28 2017
New Revision: 296664

URL: http://llvm.org/viewvc/llvm-project?rev=296664&view=rev
Log:
clang-format: [JS] Properly format object literals with shorthands.

Before:
  return {
a,
b: 'b', c,
  };

After:
  return {
a,
b: 'b',
c,
  };

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

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=296664&r1=296663&r2=296664&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Wed Mar  1 13:47:28 2017
@@ -1517,13 +1517,14 @@ private:
 return prec::Conditional;
   if (NextNonComment && NextNonComment->is(tok::colon) &&
   NextNonComment->is(TT_DictLiteral))
-return prec::Comma;
+return prec::Assignment;
+  if (Current->is(TT_JsComputedPropertyName))
+return prec::Assignment;
   if (Current->is(TT_LambdaArrow))
 return prec::Comma;
   if (Current->is(TT_JsFatArrow))
 return prec::Assignment;
-  if (Current->isOneOf(tok::semi, TT_InlineASMColon, TT_SelectorName,
-   TT_JsComputedPropertyName) ||
+  if (Current->isOneOf(tok::semi, TT_InlineASMColon, TT_SelectorName) ||
   (Current->is(tok::comment) && NextNonComment &&
NextNonComment->is(TT_SelectorName)))
 return 0;

Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=296664&r1=296663&r2=296664&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp Wed Mar  1 13:47:28 2017
@@ -278,6 +278,11 @@ TEST_F(FormatTestJS, ContainerLiterals)
"  aaa,\n"
"  aaa,\n"
"};");
+  verifyFormat("return {\n"
+   "  a,\n"
+   "  b: 'b',\n"
+   "  c,\n"
+   "};");
 }
 
 TEST_F(FormatTestJS, MethodsInObjectLiterals) {


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

[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.

[Mads Ravn via Phabricator via cfe-commits]

madsravn added inline comments.



Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:77
+
+  auto Diag = [=]() {
+std::string Message = ReplaceMessage;

aaron.ballman wrote:
> madsravn wrote:
> > aaron.ballman wrote:
> > > madsravn wrote:
> > > > aaron.ballman wrote:
> > > > > madsravn wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > Is there a reason this needs to capture everything by copy? Also, 
> > > > > > > no need for the empty parens. Actually, is the lambda even 
> > > > > > > necessary at all?
> > > > > > Is it OK to capture by reference then? Or how do we want it in 
> > > > > > llvm? 
> > > > > > 
> > > > > > We need the lambda, because first I need to create the diag with a 
> > > > > > message based on the count of arguments and then I need to find 
> > > > > > fixits based on the same count. Example: 
> > > > > > 
> > > > > > 
> > > > > > ```
> > > > > > string message = "Message for 2 arguments";
> > > > > > if(argumentCount == 3) {
> > > > > >   message = "Message for 3 arguments";
> > > > > > }
> > > > > > auto Diag = diag(startLoc(), message);
> > > > > > if(argumentCount == 3) {
> > > > > >   Diag << FixitHint::FixForThreeArguments();
> > > > > > } else {
> > > > > >   Diag << FixitHint::FixForTwoArguments();
> > > > > > }
> > > > > > ```
> > > > > > 
> > > > > > So the idea with the lambda is to avoid doing the same if-statement 
> > > > > > twice. 
> > > > > But you call the lambda immediately rather than store it and reuse 
> > > > > it? It seems like you should be able to hoist a `DiagnosticBuilder` 
> > > > > variable outside of the if statement and skip the lambda entirely.
> > > > I am not sure what you mean by this. Can you elaborate? Can you give a 
> > > > short example how I would hoist a `DiagnosticBuilder` out?
> > > > 
> > > > I think I tried something like that, but it was not an option. 
> > > It's entirely possible I'm missing something (I'm distracted with 
> > > meetings this week), but I was envisioning:
> > > ```
> > > DiagnosticBuilder Diag;
> > > if (MatchedCallExpr->getNumArgs() == 3) {
> > >   Diag =
> > >   diag(MatchedCallExpr->getLocStart(),
> > >"'std::random_shuffle' has been removed in C++17; use "
> > >"'std::shuffle' and an alternative random mechanism instead");
> > >   Diag << FixItHint::CreateReplacement(
> > >   MatchedArgumentThree->getSourceRange(),
> > >   "std::mt19937(std::random_device()())");
> > > } else {
> > >   Diag = diag(MatchedCallExpr->getLocStart(),
> > > "'std::random_shuffle' has been removed in C++17; use 
> > > "
> > > "'std::shuffle' instead");
> > >   Diag << FixItHint::CreateInsertion(
> > >   MatchedCallExpr->getRParenLoc(),
> > >   ", std::mt19937(std::random_device()())");
> > > }
> > > ```
> > The constructor for `DiagnosticBuilder` is private. So I cannot do that. 
> > The idea had crossed my mind, but I think the lambda expression is nicer to 
> > look at. 
> > 
> > Should I investigate if there is another way to hoist the 
> > `DiagnosticBuilder` out, like using `diag()` to make a dummy 
> > `DiagnosticBuilder` outside and then use the copy constructor to assign 
> > inside the if-statement? Or can we live with the lambda expression? 
> Ah, okay, that was the bit I was missing. Thank you for being patient. I 
> think the lambda (with the reference capture) is fine as-is.
> Thank you for being patient.

Right back at you. We are working towards the same goal after all :) 

For future reference: Should I try to avoid lambda expressions like this? 




https://reviews.llvm.org/D30158



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

Re: [libcxx] r296561 - Fix PR32097 - is_abstract doesn't work on class templates.

[Eric Fiselier via cfe-commits]

4.0.1 it is. Thanks Hans!

/Eric

On Wed, Mar 1, 2017 at 10:08 AM, Hans Wennborg  wrote:

> We're at the "should have tagged 'final' days ago" stage :-)
>
> Since it's not a regression, I would prefer not to merge it unless you
> feel super strongly about it.
>
> Sounds like a good candidate for 4.0.1 (tracking bug is PR32061).
>
> Thanks,
> Hans
>
> On Wed, Mar 1, 2017 at 1:36 AM, Eric Fiselier  wrote:
> > @Hans Where are we in the release process? I would like to merge this
> into
> > 4.0.
> >
> > Although it's not a regression, it is a significant bug. This patch fixes
> > the bug by
> > forwarding to a compiler builtin, which is strictly better than what we
> > have. I'm
> > confident this patch is safe.
> >
> > /Eric
> >
> > On Tue, Feb 28, 2017 at 6:27 PM, Eric Fiselier via cfe-commits
> >  wrote:
> >>
> >> Author: ericwf
> >> Date: Tue Feb 28 19:27:14 2017
> >> New Revision: 296561
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=296561&view=rev
> >> Log:
> >> Fix PR32097 - is_abstract doesn't work on class templates.
> >>
> >> This patch fixes llvm.org/PR32097 by using the __is_abstract
> >> builtin type-trait instead of the previous library-only implementation.
> >>
> >> All supported compilers provide this trait. I've tested as far
> >> back as Clang 3.2, GCC 4.6 and MSVC trunk.
> >>
> >> Modified:
> >> libcxx/trunk/include/type_traits
> >>
> >> libcxx/trunk/test/std/utilities/meta/meta.unary/
> meta.unary.prop/is_abstract.pass.cpp
> >>
> >> Modified: libcxx/trunk/include/type_traits
> >> URL:
> >> http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/
> type_traits?rev=296561&r1=296560&r2=296561&view=diff
> >>
> >> 
> ==
> >> --- libcxx/trunk/include/type_traits (original)
> >> +++ libcxx/trunk/include/type_traits Tue Feb 28 19:27:14 2017
> >> @@ -1297,18 +1297,8 @@ template  using decay_t = typ
> >>
> >>  // is_abstract
> >>
> >> -namespace __is_abstract_imp
> >> -{
> >> -template  char  __test(_Tp (*)[1]);
> >> -template  __two __test(...);
> >> -}
> >> -
> >> -template ::value>
> >> -struct __libcpp_abstract : public integral_constant >> sizeof(__is_abstract_imp::__test<_Tp>(0)) != 1> {};
> >> -
> >> -template  struct __libcpp_abstract<_Tp, false> : public
> >> false_type {};
> >> -
> >> -template  struct _LIBCPP_TEMPLATE_VIS is_abstract : public
> >> __libcpp_abstract<_Tp> {};
> >> +template  struct _LIBCPP_TEMPLATE_VIS is_abstract
> >> +: public integral_constant {};
> >>
> >>  #if _LIBCPP_STD_VER > 14 && !defined(_LIBCPP_HAS_NO_
> VARIABLE_TEMPLATES)
> >>  template  _LIBCPP_CONSTEXPR bool is_abstract_v
> >>
> >> Modified:
> >> libcxx/trunk/test/std/utilities/meta/meta.unary/
> meta.unary.prop/is_abstract.pass.cpp
> >> URL:
> >> http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/
> utilities/meta/meta.unary/meta.unary.prop/is_abstract.
> pass.cpp?rev=296561&r1=296560&r2=296561&view=diff
> >>
> >> 
> ==
> >> ---
> >> libcxx/trunk/test/std/utilities/meta/meta.unary/
> meta.unary.prop/is_abstract.pass.cpp
> >> (original)
> >> +++
> >> libcxx/trunk/test/std/utilities/meta/meta.unary/
> meta.unary.prop/is_abstract.pass.cpp
> >> Tue Feb 28 19:27:14 2017
> >> @@ -65,6 +65,14 @@ class Abstract
> >>  virtual ~Abstract() = 0;
> >>  };
> >>
> >> +template 
> >> +struct AbstractTemplate {
> >> +  virtual void test() = 0;
> >> +};
> >> +
> >> +template <>
> >> +struct AbstractTemplate {};
> >> +
> >>  int main()
> >>  {
> >>  test_is_not_abstract();
> >> @@ -81,4 +89,6 @@ int main()
> >>  test_is_not_abstract();
> >>
> >>  test_is_abstract();
> >> +test_is_abstract >();
> >> +test_is_not_abstract >();
> >>  }
> >>
> >>
> >> ___
> >> 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

[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:77
+
+  auto Diag = [=]() {
+std::string Message = ReplaceMessage;

madsravn wrote:
> aaron.ballman wrote:
> > madsravn wrote:
> > > aaron.ballman wrote:
> > > > madsravn wrote:
> > > > > aaron.ballman wrote:
> > > > > > Is there a reason this needs to capture everything by copy? Also, 
> > > > > > no need for the empty parens. Actually, is the lambda even 
> > > > > > necessary at all?
> > > > > Is it OK to capture by reference then? Or how do we want it in llvm? 
> > > > > 
> > > > > We need the lambda, because first I need to create the diag with a 
> > > > > message based on the count of arguments and then I need to find 
> > > > > fixits based on the same count. Example: 
> > > > > 
> > > > > 
> > > > > ```
> > > > > string message = "Message for 2 arguments";
> > > > > if(argumentCount == 3) {
> > > > >   message = "Message for 3 arguments";
> > > > > }
> > > > > auto Diag = diag(startLoc(), message);
> > > > > if(argumentCount == 3) {
> > > > >   Diag << FixitHint::FixForThreeArguments();
> > > > > } else {
> > > > >   Diag << FixitHint::FixForTwoArguments();
> > > > > }
> > > > > ```
> > > > > 
> > > > > So the idea with the lambda is to avoid doing the same if-statement 
> > > > > twice. 
> > > > But you call the lambda immediately rather than store it and reuse it? 
> > > > It seems like you should be able to hoist a `DiagnosticBuilder` 
> > > > variable outside of the if statement and skip the lambda entirely.
> > > I am not sure what you mean by this. Can you elaborate? Can you give a 
> > > short example how I would hoist a `DiagnosticBuilder` out?
> > > 
> > > I think I tried something like that, but it was not an option. 
> > It's entirely possible I'm missing something (I'm distracted with meetings 
> > this week), but I was envisioning:
> > ```
> > DiagnosticBuilder Diag;
> > if (MatchedCallExpr->getNumArgs() == 3) {
> >   Diag =
> >   diag(MatchedCallExpr->getLocStart(),
> >"'std::random_shuffle' has been removed in C++17; use "
> >"'std::shuffle' and an alternative random mechanism instead");
> >   Diag << FixItHint::CreateReplacement(
> >   MatchedArgumentThree->getSourceRange(),
> >   "std::mt19937(std::random_device()())");
> > } else {
> >   Diag = diag(MatchedCallExpr->getLocStart(),
> > "'std::random_shuffle' has been removed in C++17; use "
> > "'std::shuffle' instead");
> >   Diag << FixItHint::CreateInsertion(
> >   MatchedCallExpr->getRParenLoc(),
> >   ", std::mt19937(std::random_device()())");
> > }
> > ```
> The constructor for `DiagnosticBuilder` is private. So I cannot do that. The 
> idea had crossed my mind, but I think the lambda expression is nicer to look 
> at. 
> 
> Should I investigate if there is another way to hoist the `DiagnosticBuilder` 
> out, like using `diag()` to make a dummy `DiagnosticBuilder` outside and then 
> use the copy constructor to assign inside the if-statement? Or can we live 
> with the lambda expression? 
Ah, okay, that was the bit I was missing. Thank you for being patient. I think 
the lambda (with the reference capture) is fine as-is.


https://reviews.llvm.org/D30158



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

[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.

[Mads Ravn via Phabricator via cfe-commits]

madsravn added inline comments.



Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:77
+
+  auto Diag = [=]() {
+std::string Message = ReplaceMessage;

aaron.ballman wrote:
> madsravn wrote:
> > aaron.ballman wrote:
> > > madsravn wrote:
> > > > aaron.ballman wrote:
> > > > > Is there a reason this needs to capture everything by copy? Also, no 
> > > > > need for the empty parens. Actually, is the lambda even necessary at 
> > > > > all?
> > > > Is it OK to capture by reference then? Or how do we want it in llvm? 
> > > > 
> > > > We need the lambda, because first I need to create the diag with a 
> > > > message based on the count of arguments and then I need to find fixits 
> > > > based on the same count. Example: 
> > > > 
> > > > 
> > > > ```
> > > > string message = "Message for 2 arguments";
> > > > if(argumentCount == 3) {
> > > >   message = "Message for 3 arguments";
> > > > }
> > > > auto Diag = diag(startLoc(), message);
> > > > if(argumentCount == 3) {
> > > >   Diag << FixitHint::FixForThreeArguments();
> > > > } else {
> > > >   Diag << FixitHint::FixForTwoArguments();
> > > > }
> > > > ```
> > > > 
> > > > So the idea with the lambda is to avoid doing the same if-statement 
> > > > twice. 
> > > But you call the lambda immediately rather than store it and reuse it? It 
> > > seems like you should be able to hoist a `DiagnosticBuilder` variable 
> > > outside of the if statement and skip the lambda entirely.
> > I am not sure what you mean by this. Can you elaborate? Can you give a 
> > short example how I would hoist a `DiagnosticBuilder` out?
> > 
> > I think I tried something like that, but it was not an option. 
> It's entirely possible I'm missing something (I'm distracted with meetings 
> this week), but I was envisioning:
> ```
> DiagnosticBuilder Diag;
> if (MatchedCallExpr->getNumArgs() == 3) {
>   Diag =
>   diag(MatchedCallExpr->getLocStart(),
>"'std::random_shuffle' has been removed in C++17; use "
>"'std::shuffle' and an alternative random mechanism instead");
>   Diag << FixItHint::CreateReplacement(
>   MatchedArgumentThree->getSourceRange(),
>   "std::mt19937(std::random_device()())");
> } else {
>   Diag = diag(MatchedCallExpr->getLocStart(),
> "'std::random_shuffle' has been removed in C++17; use "
> "'std::shuffle' instead");
>   Diag << FixItHint::CreateInsertion(
>   MatchedCallExpr->getRParenLoc(),
>   ", std::mt19937(std::random_device()())");
> }
> ```
The constructor for `DiagnosticBuilder` is private. So I cannot do that. The 
idea had crossed my mind, but I think the lambda expression is nicer to look 
at. 

Should I investigate if there is another way to hoist the `DiagnosticBuilder` 
out, like using `diag()` to make a dummy `DiagnosticBuilder` outside and then 
use the copy constructor to assign inside the if-statement? Or can we live with 
the lambda expression? 


https://reviews.llvm.org/D30158



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

r296659 - clang-format: [JS/TS] Properly understand cast expressions.

[Daniel Jasper via cfe-commits]

Author: djasper
Date: Wed Mar  1 13:26:12 2017
New Revision: 296659

URL: http://llvm.org/viewvc/llvm-project?rev=296659&view=rev
Log:
clang-format: [JS/TS] Properly understand cast expressions.

Many things were wrong:
- We didn't always allow wrapping after "as", which can be necessary.
- We used to Undestand the identifier after "as" as a start of a name.
- We didn't properly parse the structure of the expression with "as"
  having the precedence of relational operators

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

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=296659&r1=296658&r2=296659&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Wed Mar  1 13:26:12 2017
@@ -1158,7 +1158,8 @@ private:
 if (Tok.isNot(tok::identifier) || !Tok.Previous)
   return false;
 
-if (Tok.Previous->isOneOf(TT_LeadingJavaAnnotation, 
Keywords.kw_instanceof))
+if (Tok.Previous->isOneOf(TT_LeadingJavaAnnotation, Keywords.kw_instanceof,
+  Keywords.kw_as))
   return false;
 if (Style.Language == FormatStyle::LK_JavaScript &&
 Tok.Previous->is(Keywords.kw_in))
@@ -1533,7 +1534,7 @@ private:
   Current->is(Keywords.kw_instanceof))
 return prec::Relational;
   if (Style.Language == FormatStyle::LK_JavaScript &&
-  Current->is(Keywords.kw_in))
+  Current->isOneOf(Keywords.kw_in, Keywords.kw_as))
 return prec::Relational;
   if (Current->is(TT_BinaryOperator) || Current->is(tok::comma))
 return Current->getPrecedence();
@@ -2531,6 +2532,8 @@ bool TokenAnnotator::canBreakBefore(cons
   return Style.BreakBeforeBinaryOperators != FormatStyle::BOS_None;
 if (Right.is(Keywords.kw_as))
   return false; // must not break before as in 'x as type' casts
+if (Left.is(Keywords.kw_as))
+  return true;
 if (Left.is(Keywords.kw_declare) &&
 Right.isOneOf(Keywords.kw_module, tok::kw_namespace,
   Keywords.kw_function, tok::kw_class, tok::kw_enum,

Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=296659&r1=296658&r2=296659&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp Wed Mar  1 13:26:12 2017
@@ -1502,6 +1502,9 @@ TEST_F(FormatTestJS, CastSyntax) {
   verifyFormat("x = x as {a: string};");
   verifyFormat("x = x as (string);");
   verifyFormat("x = x! as (string);");
+  verifyFormat("var x = something.someFunction() as\n"
+   "something;",
+   getGoogleJSStyleWithColumns(40));
 }
 
 TEST_F(FormatTestJS, TypeArguments) {


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

[PATCH] D30268: Avoid copy of __atoms when char_type is char

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya updated this revision to Diff 90214.
hiraditya added a comment.

Guarded the new implementation with _LIBCPP_ABI_OPTIMIZED_LOCALE macro to avoid 
ABI incompatibilities.


https://reviews.llvm.org/D30268

Files:
  libcxx/include/__config
  libcxx/include/locale

Index: libcxx/include/locale
===
--- libcxx/include/locale
+++ libcxx/include/locale
@@ -380,19 +380,57 @@
 struct __num_get
 : protected __num_get_base
 {
-static string __stage2_int_prep(ios_base& __iob, _CharT* __atoms, _CharT& __thousands_sep);
 static string __stage2_float_prep(ios_base& __iob, _CharT* __atoms, _CharT& __decimal_point,
   _CharT& __thousands_sep);
-static int __stage2_int_loop(_CharT __ct, int __base, char* __a, char*& __a_end,
-  unsigned& __dc, _CharT __thousands_sep, const string& __grouping,
-  unsigned* __g, unsigned*& __g_end, _CharT* __atoms);
+
 static int __stage2_float_loop(_CharT __ct, bool& __in_units, char& __exp,
char* __a, char*& __a_end,
_CharT __decimal_point, _CharT __thousands_sep,
const string& __grouping, unsigned* __g,
unsigned*& __g_end, unsigned& __dc, _CharT* __atoms);
+#ifndef _LIBCPP_ABI_OPTIMIZED_LOCALE
+static string __stage2_int_prep(ios_base& __iob, _CharT* __atoms, _CharT& __thousands_sep);
+static int __stage2_int_loop(_CharT __ct, int __base, char* __a, char*& __a_end,
+  unsigned& __dc, _CharT __thousands_sep, const string& __grouping,
+  unsigned* __g, unsigned*& __g_end, _CharT* __atoms);
+
+#else
+static string __stage2_int_prep(ios_base& __iob, _CharT& __thousands_sep)
+{
+locale __loc = __iob.getloc();
+const numpunct<_CharT>& __np = use_facet >(__loc);
+__thousands_sep = __np.thousands_sep();
+return __np.grouping();
+}
+
+const _CharT* __do_widen(ios_base& __iob, _CharT* __atoms) const
+{
+  return __do_widen_p(__iob, __atoms);
+}
+
+
+static int __stage2_int_loop(_CharT __ct, int __base, char* __a, char*& __a_end,
+  unsigned& __dc, _CharT __thousands_sep, const string& __grouping,
+  unsigned* __g, unsigned*& __g_end, const _CharT* __atoms);
+private:
+template
+const T* __do_widen_p(ios_base& __iob, T* __atoms) const
+{
+  locale __loc = __iob.getloc();
+  use_facet >(__loc).widen(__src, __src + 26, __atoms);
+  return __atoms;
+}
+
+const char* __do_widen_p(ios_base& __iob, char* __atoms) const
+{
+  (void)__iob;
+  (void)__atoms;
+  return __src;
+}
+#endif
 };
 
+#ifndef _LIBCPP_ABI_OPTIMIZED_LOCALE
 template 
 string
 __num_get<_CharT>::__stage2_int_prep(ios_base& __iob, _CharT* __atoms, _CharT& __thousands_sep)
@@ -403,6 +441,7 @@
 __thousands_sep = __np.thousands_sep();
 return __np.grouping();
 }
+#endif
 
 template 
 string
@@ -419,9 +458,17 @@
 
 template 
 int
+#ifndef _LIBCPP_ABI_OPTIMIZED_LOCALE
 __num_get<_CharT>::__stage2_int_loop(_CharT __ct, int __base, char* __a, char*& __a_end,
   unsigned& __dc, _CharT __thousands_sep, const string& __grouping,
   unsigned* __g, unsigned*& __g_end, _CharT* __atoms)
+#else
+__num_get<_CharT>::__stage2_int_loop(_CharT __ct, int __base, char* __a, char*& __a_end,
+  unsigned& __dc, _CharT __thousands_sep, const string& __grouping,
+  unsigned* __g, unsigned*& __g_end, const _CharT* __atoms)
+
+#endif
+
 {
 if (__a_end == __a && (__ct == __atoms[24] || __ct == __atoms[25]))
 {
@@ -841,6 +888,7 @@
 return __b;
 }
 
+#ifndef _LIBCPP_ABI_OPTIMIZED_LOCALE
 // signed
 
 template 
@@ -941,6 +989,110 @@
 return __b;
 }
 
+#else
+//signed
+
+template 
+template 
+_InputIterator
+num_get<_CharT, _InputIterator>::__do_get_signed(iter_type __b, iter_type __e,
+ios_base& __iob,
+ios_base::iostate& __err,
+_Signed& __v) const
+{
+// Stage 1
+int __base = this->__get_base(__iob);
+// Stage 2
+char_type __atoms1[26];
+char_type __thousands_sep;
+const char_type *__atoms = this->__do_widen(__iob, __atoms1);
+string __grouping = this->__stage2_int_prep(__iob, __thousands_sep);
+string __buf;
+__buf.resize(__buf.capacity());
+char* __a = &__buf[0];
+char* __a_end = __a;
+unsigned __g[__num_get_base::__num_get_buf_sz];
+unsigned* __g_end = __g;
+unsigned __dc = 0;
+for (; __b != __e; ++__b)
+{
+if (__a_end == __a + __buf.size())
+{
+size_t __tmp = __buf.size();
+__buf.resize(2*__buf.size());
+__buf.resize(__buf.capacity());
+__a = &__buf[0]

[PATCH] D29753: [PCH] Avoid early VarDecl emission attempt if no owning module avaiable

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL296656: [PCH] Avoid VarDecl emission attempt if no owning 
module avaiable (authored by bruno).

Changed prior to commit:
  https://reviews.llvm.org/D29753?vs=87774&id=90212#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D29753

Files:
  cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
  cfe/trunk/test/PCH/empty-def-fwd-struct.h


Index: cfe/trunk/test/PCH/empty-def-fwd-struct.h
===
--- cfe/trunk/test/PCH/empty-def-fwd-struct.h
+++ cfe/trunk/test/PCH/empty-def-fwd-struct.h
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -emit-pch -x c++-header %s -std=c++14 -o %t.pch
+// RUN: %clang_cc1 -emit-llvm-only -x c++ /dev/null -std=c++14 -include-pch 
%t.pch -o %t.o
+struct FVector;
+struct FVector {};
+struct FBox {
+  FVector Min;
+  FBox(int);
+};
+namespace {
+FBox InvalidBoundingBox(0);
+}
+
Index: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
===
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
@@ -2530,8 +2530,8 @@
 
   // An ImportDecl or VarDecl imported from a module will get emitted when
   // we import the relevant module.
-  if ((isa(D) || isa(D)) && Ctx.DeclMustBeEmitted(D) &&
-  D->getImportedOwningModule())
+  if ((isa(D) || isa(D)) && D->getImportedOwningModule() 
&&
+  Ctx.DeclMustBeEmitted(D))
 return false;
 
   if (isa(D) || 


Index: cfe/trunk/test/PCH/empty-def-fwd-struct.h
===
--- cfe/trunk/test/PCH/empty-def-fwd-struct.h
+++ cfe/trunk/test/PCH/empty-def-fwd-struct.h
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -emit-pch -x c++-header %s -std=c++14 -o %t.pch
+// RUN: %clang_cc1 -emit-llvm-only -x c++ /dev/null -std=c++14 -include-pch %t.pch -o %t.o
+struct FVector;
+struct FVector {};
+struct FBox {
+  FVector Min;
+  FBox(int);
+};
+namespace {
+FBox InvalidBoundingBox(0);
+}
+
Index: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
===
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
@@ -2530,8 +2530,8 @@
 
   // An ImportDecl or VarDecl imported from a module will get emitted when
   // we import the relevant module.
-  if ((isa(D) || isa(D)) && Ctx.DeclMustBeEmitted(D) &&
-  D->getImportedOwningModule())
+  if ((isa(D) || isa(D)) && D->getImportedOwningModule() &&
+  Ctx.DeclMustBeEmitted(D))
 return false;
 
   if (isa(D) || 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r296656 - [PCH] Avoid VarDecl emission attempt if no owning module avaiable

[Bruno Cardoso Lopes via cfe-commits]

Author: bruno
Date: Wed Mar  1 13:18:42 2017
New Revision: 296656

URL: http://llvm.org/viewvc/llvm-project?rev=296656&view=rev
Log:
[PCH] Avoid VarDecl emission attempt if no owning module avaiable

This is a stopgap fix for PR31863, a regression introduced in r276159.

Consider this snippet:

struct FVector;
struct FVector {};
struct FBox {
  FVector Min;
  FBox(int);
};
namespace {
FBox InvalidBoundingBox(0);
}

While parsing the DECL_VAR for 'struct FBox', clang recursively read all the
dep decls until it finds the DECL_CXX_RECORD forward declaration for 'struct
FVector'. Then, it resumes all the way up back to DECL_VAR handling in
`ReadDeclRecord`, where it checks if `isConsumerInterestedIn` for the decl.

One of the condition for `isConsumerInterestedIn` to return false is if the
VarDecl is imported from a module `D->getImportedOwningModule()`, because it
will get emitted when we import the relevant module. However, before checking
if it comes from a module, clang checks if `Ctx.DeclMustBeEmitted(D)`, which
triggers the emission of 'struct FBox'. Since one of its fields is still
incomplete, it crashes.

Instead, check if `D->getImportedOwningModule()` is true before calling
`Ctx.DeclMustBeEmitted(D)`.

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

rdar://problem/30173654

Added:
cfe/trunk/test/PCH/empty-def-fwd-struct.h
Modified:
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp

Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=296656&r1=296655&r2=296656&view=diff
==
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Wed Mar  1 13:18:42 2017
@@ -2530,8 +2530,8 @@ static bool isConsumerInterestedIn(ASTCo
 
   // An ImportDecl or VarDecl imported from a module will get emitted when
   // we import the relevant module.
-  if ((isa(D) || isa(D)) && Ctx.DeclMustBeEmitted(D) &&
-  D->getImportedOwningModule())
+  if ((isa(D) || isa(D)) && D->getImportedOwningModule() 
&&
+  Ctx.DeclMustBeEmitted(D))
 return false;
 
   if (isa(D) || 

Added: cfe/trunk/test/PCH/empty-def-fwd-struct.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/empty-def-fwd-struct.h?rev=296656&view=auto
==
--- cfe/trunk/test/PCH/empty-def-fwd-struct.h (added)
+++ cfe/trunk/test/PCH/empty-def-fwd-struct.h Wed Mar  1 13:18:42 2017
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -emit-pch -x c++-header %s -std=c++14 -o %t.pch
+// RUN: %clang_cc1 -emit-llvm-only -x c++ /dev/null -std=c++14 -include-pch 
%t.pch -o %t.o
+struct FVector;
+struct FVector {};
+struct FBox {
+  FVector Min;
+  FBox(int);
+};
+namespace {
+FBox InvalidBoundingBox(0);
+}
+


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

[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:77
+
+  auto Diag = [=]() {
+std::string Message = ReplaceMessage;

madsravn wrote:
> aaron.ballman wrote:
> > madsravn wrote:
> > > aaron.ballman wrote:
> > > > Is there a reason this needs to capture everything by copy? Also, no 
> > > > need for the empty parens. Actually, is the lambda even necessary at 
> > > > all?
> > > Is it OK to capture by reference then? Or how do we want it in llvm? 
> > > 
> > > We need the lambda, because first I need to create the diag with a 
> > > message based on the count of arguments and then I need to find fixits 
> > > based on the same count. Example: 
> > > 
> > > 
> > > ```
> > > string message = "Message for 2 arguments";
> > > if(argumentCount == 3) {
> > >   message = "Message for 3 arguments";
> > > }
> > > auto Diag = diag(startLoc(), message);
> > > if(argumentCount == 3) {
> > >   Diag << FixitHint::FixForThreeArguments();
> > > } else {
> > >   Diag << FixitHint::FixForTwoArguments();
> > > }
> > > ```
> > > 
> > > So the idea with the lambda is to avoid doing the same if-statement 
> > > twice. 
> > But you call the lambda immediately rather than store it and reuse it? It 
> > seems like you should be able to hoist a `DiagnosticBuilder` variable 
> > outside of the if statement and skip the lambda entirely.
> I am not sure what you mean by this. Can you elaborate? Can you give a short 
> example how I would hoist a `DiagnosticBuilder` out?
> 
> I think I tried something like that, but it was not an option. 
It's entirely possible I'm missing something (I'm distracted with meetings this 
week), but I was envisioning:
```
DiagnosticBuilder Diag;
if (MatchedCallExpr->getNumArgs() == 3) {
  Diag =
  diag(MatchedCallExpr->getLocStart(),
   "'std::random_shuffle' has been removed in C++17; use "
   "'std::shuffle' and an alternative random mechanism instead");
  Diag << FixItHint::CreateReplacement(
  MatchedArgumentThree->getSourceRange(),
  "std::mt19937(std::random_device()())");
} else {
  Diag = diag(MatchedCallExpr->getLocStart(),
"'std::random_shuffle' has been removed in C++17; use "
"'std::shuffle' instead");
  Diag << FixItHint::CreateInsertion(
  MatchedCallExpr->getRParenLoc(),
  ", std::mt19937(std::random_device()())");
}
```


https://reviews.llvm.org/D30158



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

[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.

[Mads Ravn via Phabricator via cfe-commits]

madsravn marked an inline comment as done.
madsravn added inline comments.



Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:77
+
+  auto Diag = [=]() {
+std::string Message = ReplaceMessage;

aaron.ballman wrote:
> madsravn wrote:
> > aaron.ballman wrote:
> > > Is there a reason this needs to capture everything by copy? Also, no need 
> > > for the empty parens. Actually, is the lambda even necessary at all?
> > Is it OK to capture by reference then? Or how do we want it in llvm? 
> > 
> > We need the lambda, because first I need to create the diag with a message 
> > based on the count of arguments and then I need to find fixits based on the 
> > same count. Example: 
> > 
> > 
> > ```
> > string message = "Message for 2 arguments";
> > if(argumentCount == 3) {
> >   message = "Message for 3 arguments";
> > }
> > auto Diag = diag(startLoc(), message);
> > if(argumentCount == 3) {
> >   Diag << FixitHint::FixForThreeArguments();
> > } else {
> >   Diag << FixitHint::FixForTwoArguments();
> > }
> > ```
> > 
> > So the idea with the lambda is to avoid doing the same if-statement twice. 
> But you call the lambda immediately rather than store it and reuse it? It 
> seems like you should be able to hoist a `DiagnosticBuilder` variable outside 
> of the if statement and skip the lambda entirely.
I am not sure what you mean by this. Can you elaborate? Can you give a short 
example how I would hoist a `DiagnosticBuilder` out?

I think I tried something like that, but it was not an option. 


https://reviews.llvm.org/D30158



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

[PATCH] D29750: [PPC] Enable -fomit-frame-pointer by default for PPC

[Hal Finkel via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D29750



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

[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:77
+
+  auto Diag = [=]() {
+std::string Message = ReplaceMessage;

madsravn wrote:
> aaron.ballman wrote:
> > Is there a reason this needs to capture everything by copy? Also, no need 
> > for the empty parens. Actually, is the lambda even necessary at all?
> Is it OK to capture by reference then? Or how do we want it in llvm? 
> 
> We need the lambda, because first I need to create the diag with a message 
> based on the count of arguments and then I need to find fixits based on the 
> same count. Example: 
> 
> 
> ```
> string message = "Message for 2 arguments";
> if(argumentCount == 3) {
>   message = "Message for 3 arguments";
> }
> auto Diag = diag(startLoc(), message);
> if(argumentCount == 3) {
>   Diag << FixitHint::FixForThreeArguments();
> } else {
>   Diag << FixitHint::FixForTwoArguments();
> }
> ```
> 
> So the idea with the lambda is to avoid doing the same if-statement twice. 
But you call the lambda immediately rather than store it and reuse it? It seems 
like you should be able to hoist a `DiagnosticBuilder` variable outside of the 
if statement and skip the lambda entirely.



Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:89
+
+  std::string newname = "shuffle";
+  StringRef ContainerText = Lexer::getSourceText(

Should be `NewName` instead.


https://reviews.llvm.org/D30158



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

Re: r296554 - [PS4] Set our default dialect to C++11. NFC for other targets.

[Mehdi Amini via cfe-commits]

I’m not sure I find this nice to see this upstream.

I not fond in general of this kind of difference in behavior. I don’t think it 
is good for clang to have different default for this kind of settings depending 
on the platform. It does not provide a very good user experience from a 
cross-platform point of view (i.e. my compiler behaves very differently when I 
target one platform instead of another).

— 
Mehdi



> On Feb 28, 2017, at 11:22 PM, Sean Silva via cfe-commits 
>  wrote:
> 
> Nice!
> 
> -- Sean Silva
> 
> On Tue, Feb 28, 2017 at 5:01 PM, Paul Robinson via cfe-commits 
> mailto:cfe-commits@lists.llvm.org>> wrote:
> Author: probinson
> Date: Tue Feb 28 19:01:10 2017
> New Revision: 296554
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=296554&view=rev 
> 
> Log:
> [PS4] Set our default dialect to C++11. NFC for other targets.
> Reapplies r296209 now that r296549 has fixed what really seems to be
> the last problematic test.
> 
> Modified:
> cfe/trunk/lib/Frontend/CompilerInvocation.cpp
> 
> Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=296554&r1=296553&r2=296554&view=diff
>  
> 
> ==
> --- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
> +++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Tue Feb 28 19:01:10 2017
> @@ -1582,7 +1582,11 @@ void CompilerInvocation::setLangDefaults
>  case IK_PreprocessedCXX:
>  case IK_ObjCXX:
>  case IK_PreprocessedObjCXX:
> -  LangStd = LangStandard::lang_gnucxx98;
> +  // The PS4 uses C++11 as the default C++ standard.
> +  if (T.isPS4())
> +LangStd = LangStandard::lang_gnucxx11;
> +  else
> +LangStd = LangStandard::lang_gnucxx98;
>break;
>  case IK_RenderScript:
>LangStd = LangStandard::lang_c99;
> 
> 
> ___
> 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

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

r296649 - Introduce an 'external_source_symbol' attribute that describes the origin

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Wed Mar  1 12:06:25 2017
New Revision: 296649

URL: http://llvm.org/viewvc/llvm-project?rev=296649&view=rev
Log:
Introduce an 'external_source_symbol' attribute that describes the origin
and the nature of a declaration

This commit adds an external_source_symbol attribute to Clang. This attribute
specifies that a declaration originates from an external source and describes
the nature of that source. This attribute will be used to improve IDE features
like 'jump-to-definition' for mixed-language projects or project that use
auto-generated code.

rdar://30423368

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

Added:
cfe/trunk/test/Parser/attr-external-source-symbol-cxx11.cpp
cfe/trunk/test/Parser/attr-external-source-symbol.m
cfe/trunk/test/Sema/attr-external-source-symbol.c
Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/AttrDocs.td
cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td
cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Parse/Parser.h
cfe/trunk/include/clang/Sema/AttributeList.h
cfe/trunk/lib/Parse/ParseDecl.cpp
cfe/trunk/lib/Parse/ParseDeclCXX.cpp
cfe/trunk/lib/Parse/Parser.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/Misc/ast-dump-attr.cpp

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=296649&r1=296648&r2=296649&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Wed Mar  1 12:06:25 2017
@@ -527,6 +527,17 @@ def Availability : InheritableAttr {
   let Documentation = [AvailabilityDocs];
 }
 
+def ExternalSourceSymbol : InheritableAttr {
+  let Spellings = [GNU<"external_source_symbol">,
+   CXX11<"clang", "external_source_symbol">];
+  let Args = [StringArgument<"language", 1>,
+  StringArgument<"definedIn", 1>,
+  BoolArgument<"generatedDeclaration", 1>];
+  let HasCustomParsing = 1;
+//  let Subjects = SubjectList<[Named]>;
+  let Documentation = [ExternalSourceSymbolDocs];
+}
+
 def Blocks : InheritableAttr {
   let Spellings = [GNU<"blocks">];
   let Args = [EnumArgument<"Type", "BlockType", ["byref"], ["ByRef"]>];

Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=296649&r1=296648&r2=296649&view=diff
==
--- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
+++ cfe/trunk/include/clang/Basic/AttrDocs.td Wed Mar  1 12:06:25 2017
@@ -960,6 +960,63 @@ When one method overrides another, the o
   }];
 }
 
+def ExternalSourceSymbolDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``external_source_symbol`` attribute specifies that a declaration 
originates
+from an external source and describes the nature of that source.
+
+The fact that Clang is capable of recognizing declarations that were defined
+externally can be used to provide better tooling support for mixed-language
+projects or projects that rely on auto-generated code. For instance, an IDE 
that
+uses Clang and that supports mixed-language projects can use this attribute to
+provide a correct 'jump-to-definition' feature. For a concrete example,
+consider a protocol that's defined in a Swift file:
+
+.. code-block:: swift
+
+  @objc public protocol SwiftProtocol {
+func method()
+  }
+
+This protocol can be used from Objective-C code by including a header file that
+was generated by the Swift compiler. The declarations in that header can use
+the ``external_source_symbol`` attribute to make Clang aware of the fact
+that ``SwiftProtocol`` actually originates from a Swift module:
+
+.. code-block:: objc
+
+  __attribute__((external_source_symbol(language=Swift,defined_in="module")))
+  @protocol SwiftProtocol
+  @required
+  - (void) method;
+  @end
+
+Consequently, when 'jump-to-definition' is performed at a location that
+references ``SwiftProtocol``, the IDE can jump to the original definition in
+the Swift source file rather than jumping to the Objective-C declaration in the
+auto-generated header file.
+
+The ``external_source_symbol`` attribute is a comma-separated list that 
includes
+clauses that describe the origin and the nature of the particular declaration.
+Those clauses can be:
+
+language=\ *string-literal*
+  The name of the source language in which this declaration was defined.
+
+defined_in=\ *string-literal*
+  The name of the source container in which the declaration was defined. The
+  exact definition of source container is language-specific, e.g. Swift's
+  source containers are modules, so ``defined_in`` should specify the Swift
+  module name.
+

[PATCH] D29819: Introduce an 'external_source_symbol' attribute that describes the origin and the nature of a declaration

[Alex Lorenz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL296649: Introduce an 'external_source_symbol' attribute that 
describes the origin (authored by arphaman).

Changed prior to commit:
  https://reviews.llvm.org/D29819?vs=90042&id=90205#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D29819

Files:
  cfe/trunk/include/clang/Basic/Attr.td
  cfe/trunk/include/clang/Basic/AttrDocs.td
  cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td
  cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/include/clang/Parse/Parser.h
  cfe/trunk/include/clang/Sema/AttributeList.h
  cfe/trunk/lib/Parse/ParseDecl.cpp
  cfe/trunk/lib/Parse/ParseDeclCXX.cpp
  cfe/trunk/lib/Parse/Parser.cpp
  cfe/trunk/lib/Sema/SemaDeclAttr.cpp
  cfe/trunk/test/Misc/ast-dump-attr.cpp
  cfe/trunk/test/Parser/attr-external-source-symbol-cxx11.cpp
  cfe/trunk/test/Parser/attr-external-source-symbol.m
  cfe/trunk/test/Sema/attr-external-source-symbol.c

Index: cfe/trunk/include/clang/Sema/AttributeList.h
===
--- cfe/trunk/include/clang/Sema/AttributeList.h
+++ cfe/trunk/include/clang/Sema/AttributeList.h
@@ -927,6 +927,7 @@
   ExpectedStructClassVariableFunctionOrInlineNamespace,
   ExpectedForMaybeUnused,
   ExpectedEnumOrClass,
+  ExpectedNamedDecl,
 };
 
 }  // end namespace clang
Index: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
@@ -859,6 +859,12 @@
 def err_availability_query_repeated_star : Error<
   "'*' query has already been specified">;
 
+// External source symbol attribute
+def err_external_source_symbol_expected_keyword : Error<
+  "expected 'language', 'defined_in', or 'generated_declaration'">;
+def err_external_source_symbol_duplicate_clause : Error<
+  "duplicate %0 clause in an 'external_source_symbol' attribute">;
+
 // Type safety attributes
 def err_type_safety_unknown_flag : Error<
   "invalid comparison flag %0; use 'layout_compatible' or 'must_be_null'">;
Index: cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td
@@ -45,7 +45,9 @@
   "must end with ':'">;
 def err_expected_string_literal : Error<"expected string literal "
   "%select{in %1|for diagnostic message in static_assert|"
-  "for optional message in 'availability' attribute}0">;
+  "for optional message in 'availability' attribute|"
+  "for %select{language|source container}1 name in "
+  "'external_source_symbol' attribute}0">;
 def err_invalid_string_udl : Error<
   "string literal with user-defined suffix cannot be used here">;
 def err_invalid_character_udl : Error<
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2754,7 +2754,8 @@
   "|functions, methods, enums, and classes"
   "|structs, classes, variables, functions, and inline namespaces"
   "|variables, functions, methods, types, enumerations, enumerators, labels, and non-static data members"
-  "|classes and enumerations}1">,
+  "|classes and enumerations"
+  "|named declarations}1">,
   InGroup;
 def err_attribute_wrong_decl_type : Error;
 def warn_type_attribute_wrong_type : Warning<
Index: cfe/trunk/include/clang/Basic/AttrDocs.td
===
--- cfe/trunk/include/clang/Basic/AttrDocs.td
+++ cfe/trunk/include/clang/Basic/AttrDocs.td
@@ -960,6 +960,63 @@
   }];
 }
 
+def ExternalSourceSymbolDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``external_source_symbol`` attribute specifies that a declaration originates
+from an external source and describes the nature of that source.
+
+The fact that Clang is capable of recognizing declarations that were defined
+externally can be used to provide better tooling support for mixed-language
+projects or projects that rely on auto-generated code. For instance, an IDE that
+uses Clang and that supports mixed-language projects can use this attribute to
+provide a correct 'jump-to-definition' feature. For a concrete example,
+consider a protocol that's defined in a Swift file:
+
+.. code-block:: swift
+
+  @objc public protocol SwiftProtocol {
+func method()
+  }
+
+This protocol can be used from Objective-C code by including a header file that
+was generated by the Swift compiler. The declarations in that header can use
+the ``external_source_symbol`` attribute to make Clang aware of the fact
+th

[PATCH] D29819: Introduce an 'external_source_symbol' attribute that describes the origin and the nature of a declaration

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: lib/Parse/ParseDeclCXX.cpp:3830-3837
+  unsigned NumArgs;
+  // Some Clang-scoped attributes have some special parsing behavior.
+  if (ScopeName && ScopeName->getName() == "clang")
+NumArgs =
+ParseClangAttributeArgs(AttrName, AttrNameLoc, Attrs, EndLoc, 
ScopeName,
+ScopeLoc, AttributeList::AS_CXX11);
+  else

aaron.ballman wrote:
> This code is equivalent to the old code, which is good. However, it points 
> out what I think may be a latent bug -- for attributes in the clang namespace 
> that are also standard attributes, I'm not certain we should be generating 
> parse errors for the clang-namespaced spelling. This only impacts 
> [[fallthrough]] vs [[clang::fallthrough]], according to the implementation of 
> `IsBuiltInOrStandardCXX11Attribute()`.
> 
> All of that is to say: I'm not certain we need to hoist `NumArgs` to get to 
> the call to `IsBuiltInOrStandardCXX11Attribute()` below, because attributes 
> in the clang namespace are not built-in or standard attributes.
> 
> This requires a bit further exploration, but I won't have time to do that for 
> a few weeks. I don't want to hold your patch up for that, so I think this is 
> fine for now -- I can modify the code later when I have the chance to finish 
> my exploration.
Thanks!

I agree with the comment about `NumArgs`, I wasn't sure if we need it for 
`clang::` attributes as well.


Repository:
  rL LLVM

https://reviews.llvm.org/D29819



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

r296648 - [Test] NFC: Fixed typo in comments

[Charles Li via cfe-commits]

Author: lcharles
Date: Wed Mar  1 11:55:03 2017
New Revision: 296648

URL: http://llvm.org/viewvc/llvm-project?rev=296648&view=rev
Log:
[Test] NFC: Fixed typo in comments

Changed "declerations" to "declarations"

Modified:
cfe/trunk/test/Modules/Inputs/merge-using-decls/b.h
cfe/trunk/test/Modules/merge-using-decls.cpp

Modified: cfe/trunk/test/Modules/Inputs/merge-using-decls/b.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-using-decls/b.h?rev=296648&r1=296647&r2=296648&view=diff
==
--- cfe/trunk/test/Modules/Inputs/merge-using-decls/b.h (original)
+++ cfe/trunk/test/Modules/Inputs/merge-using-decls/b.h Wed Mar  1 11:55:03 2017
@@ -29,7 +29,7 @@ template struct D : X, T {
   using typename X::t;
 };
 
-#if __cplusplus <= 199711L // C++11 does not allow access declerations
+#if __cplusplus <= 199711L // C++11 does not allow access declarations
 template struct E : X, T {
   // Mismatch in using/access-declaration-ness.
   T::value;
@@ -49,7 +49,7 @@ template struct F : X, T {
 typedef C::type I;
 typedef D::t I;
 
-#if __cplusplus <= 199711L // C++11 does not allow access declerations
+#if __cplusplus <= 199711L // C++11 does not allow access declarations
 typedef E::type I;
 #endif
 

Modified: cfe/trunk/test/Modules/merge-using-decls.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-using-decls.cpp?rev=296648&r1=296647&r2=296648&view=diff
==
--- cfe/trunk/test/Modules/merge-using-decls.cpp (original)
+++ cfe/trunk/test/Modules/merge-using-decls.cpp Wed Mar  1 11:55:03 2017
@@ -28,7 +28,7 @@ template int Use() {
 }
 
 template int UseAll() {
-#if __cplusplus <= 199711L // C++11 does not allow access declerations
+#if __cplusplus <= 199711L // C++11 does not allow access declarations
   return Use >() + Use >() + Use >() + Use >(); // 
expected-note 0-2{{instantiation of}}
 #else
   return Use >() + Use >() + Use >(); // expected-note 
0-2{{instantiation of}}
@@ -45,7 +45,7 @@ template int UseAll();
 // Here, we're instantiating the definition from 'A' and merging the definition
 // from 'B' into it.
 
-#if __cplusplus <= 199711L // C++11 does not allow access declerations
+#if __cplusplus <= 199711L // C++11 does not allow access declarations
 // expected-error@b.h:* {{'E::value' from module 'B' is not present in 
definition of 'E' in module 'A'}}
 // expected-error@b.h:* {{'E::v' from module 'B' is not present in definition 
of 'E' in module 'A'}}
 #endif
@@ -65,7 +65,7 @@ template int UseAll();
 // expected-error@b.h:* 2{{'typename' keyword used on a non-type}}
 // expected-error@b.h:* 2{{dependent using declaration resolved to type 
without 'typename'}}
 
-#if __cplusplus <= 199711L // C++11 does not allow access declerations
+#if __cplusplus <= 199711L // C++11 does not allow access declarations
 // expected-error@a.h:* {{'E::type' from module 'A' is not present in 
definition of 'E' in module 'B'}}
 // expected-error@a.h:* {{'E::t' from module 'A' is not present in definition 
of 'E' in module 'B'}}
 // expected-error@a.h:* {{'E::value' from module 'A' is not present in 
definition of 'E' in module 'B'}}


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

r296646 - [analyzer] pr32088: Don't destroy the temporary if its initializer causes return.

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Wed Mar  1 11:48:39 2017
New Revision: 296646

URL: http://llvm.org/viewvc/llvm-project?rev=296646&view=rev
Log:
[analyzer] pr32088: Don't destroy the temporary if its initializer causes 
return.

In the following code involving GNU statement-expression extension:
  struct S {
~S();
  };

  void foo() {
const S &x = ({ return; S(); });
  }
function 'foo()' returns before reference x is initialized. We shouldn't call
the destructor for the temporary object lifetime-extended by 'x' in this case,
because the object never gets constructed in the first place.

The real problem is probably in the CFG somewhere, so this is a quick-and-dirty
hotfix rather than the perfect solution.

A patch by Artem Dergachev!

rdar://problem/30759076

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
cfe/trunk/test/Analysis/temporaries.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=296646&r1=296645&r2=296646&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Wed Mar  1 11:48:39 2017
@@ -615,7 +615,15 @@ void ExprEngine::ProcessAutomaticObjDtor
   const MemRegion *Region = dest.castAs().getRegion();
 
   if (varType->isReferenceType()) {
-Region = state->getSVal(Region).getAsRegion()->getBaseRegion();
+const MemRegion *ValueRegion = state->getSVal(Region).getAsRegion();
+if (!ValueRegion) {
+  // FIXME: This should not happen. The language guarantees a presence
+  // of a valid initializer here, so the reference shall not be undefined.
+  // It seems that we're calling destructors over variables that
+  // were not initialized yet.
+  return;
+}
+Region = ValueRegion->getBaseRegion();
 varType = cast(Region)->getValueType();
   }
 

Modified: cfe/trunk/test/Analysis/temporaries.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/temporaries.cpp?rev=296646&r1=296645&r2=296646&view=diff
==
--- cfe/trunk/test/Analysis/temporaries.cpp (original)
+++ cfe/trunk/test/Analysis/temporaries.cpp Wed Mar  1 11:48:39 2017
@@ -493,3 +493,13 @@ namespace PR16629 {
 clang_analyzer_eval(x == 47); // expected-warning{{TRUE}}
   }
 }
+
+namespace PR32088 {
+  void testReturnFromStmtExprInitializer() {
+// We shouldn't try to destroy the object pointed to by `obj' upon return.
+const NonTrivial &obj = ({
+  return; // no-crash
+  NonTrivial(42);
+});
+  }
+}


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

[PATCH] D30499: [analyzer] pr32088: Don't destroy the temporary if its initializer causes return.

[Devin Coughlin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL296646: [analyzer] pr32088: Don't destroy the temporary if 
its initializer causes… (authored by dcoughlin).

Changed prior to commit:
  https://reviews.llvm.org/D30499?vs=90178&id=90201#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30499

Files:
  cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
  cfe/trunk/test/Analysis/temporaries.cpp


Index: cfe/trunk/test/Analysis/temporaries.cpp
===
--- cfe/trunk/test/Analysis/temporaries.cpp
+++ cfe/trunk/test/Analysis/temporaries.cpp
@@ -493,3 +493,13 @@
 clang_analyzer_eval(x == 47); // expected-warning{{TRUE}}
   }
 }
+
+namespace PR32088 {
+  void testReturnFromStmtExprInitializer() {
+// We shouldn't try to destroy the object pointed to by `obj' upon return.
+const NonTrivial &obj = ({
+  return; // no-crash
+  NonTrivial(42);
+});
+  }
+}
Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -615,7 +615,15 @@
   const MemRegion *Region = dest.castAs().getRegion();
 
   if (varType->isReferenceType()) {
-Region = state->getSVal(Region).getAsRegion()->getBaseRegion();
+const MemRegion *ValueRegion = state->getSVal(Region).getAsRegion();
+if (!ValueRegion) {
+  // FIXME: This should not happen. The language guarantees a presence
+  // of a valid initializer here, so the reference shall not be undefined.
+  // It seems that we're calling destructors over variables that
+  // were not initialized yet.
+  return;
+}
+Region = ValueRegion->getBaseRegion();
 varType = cast(Region)->getValueType();
   }
 


Index: cfe/trunk/test/Analysis/temporaries.cpp
===
--- cfe/trunk/test/Analysis/temporaries.cpp
+++ cfe/trunk/test/Analysis/temporaries.cpp
@@ -493,3 +493,13 @@
 clang_analyzer_eval(x == 47); // expected-warning{{TRUE}}
   }
 }
+
+namespace PR32088 {
+  void testReturnFromStmtExprInitializer() {
+// We shouldn't try to destroy the object pointed to by `obj' upon return.
+const NonTrivial &obj = ({
+  return; // no-crash
+  NonTrivial(42);
+});
+  }
+}
Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -615,7 +615,15 @@
   const MemRegion *Region = dest.castAs().getRegion();
 
   if (varType->isReferenceType()) {
-Region = state->getSVal(Region).getAsRegion()->getBaseRegion();
+const MemRegion *ValueRegion = state->getSVal(Region).getAsRegion();
+if (!ValueRegion) {
+  // FIXME: This should not happen. The language guarantees a presence
+  // of a valid initializer here, so the reference shall not be undefined.
+  // It seems that we're calling destructors over variables that
+  // were not initialized yet.
+  return;
+}
+Region = ValueRegion->getBaseRegion();
 varType = cast(Region)->getValueType();
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30345: [CodeGen][Blocks] Refactor capture handling in code that generates block copy/destroy routines

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 90197.
arphaman marked 4 inline comments as done.
arphaman added a comment.

Addressed John's comments.


Repository:
  rL LLVM

https://reviews.llvm.org/D30345

Files:
  lib/CodeGen/CGBlocks.cpp

Index: lib/CodeGen/CGBlocks.cpp
===
--- lib/CodeGen/CGBlocks.cpp
+++ lib/CodeGen/CGBlocks.cpp
@@ -1373,6 +1373,103 @@
   return fn;
 }
 
+namespace {
+
+/// Represents a type of copy/destroy operation that should be performed for an
+/// entity that's captured by a block.
+enum class BlockCaptureEntityKind {
+  CXXRecord, // Copy or destroy
+  ARCWeak,
+  ARCStrong,
+  BlockObject, // Assign or release
+  None
+};
+
+/// Represents a captured entity that requires extra operations in order for
+/// this entity to be copied or destroyed correctly.
+struct BlockCaptureManagedEntity {
+  BlockCaptureEntityKind Kind;
+  BlockFieldFlags Flags;
+  const BlockDecl::Capture &CI;
+  const CGBlockInfo::Capture &Capture;
+
+  BlockCaptureManagedEntity(BlockCaptureEntityKind Type, BlockFieldFlags Flags,
+const BlockDecl::Capture &CI,
+const CGBlockInfo::Capture &Capture)
+  : Kind(Type), Flags(Flags), CI(CI), Capture(Capture) {}
+};
+
+} // end anonymous namespace
+
+static std::pair
+computeCopyInfoForBlockCapture(const BlockDecl::Capture &CI, QualType T,
+   const LangOptions &LangOpts) {
+  if (CI.getCopyExpr()) {
+assert(!CI.isByRef());
+// don't bother computing flags
+return std::make_pair(BlockCaptureEntityKind::CXXRecord, BlockFieldFlags());
+  }
+  BlockFieldFlags Flags;
+  if (CI.isByRef()) {
+Flags = BLOCK_FIELD_IS_BYREF;
+if (T.isObjCGCWeak())
+  Flags |= BLOCK_FIELD_IS_WEAK;
+return std::make_pair(BlockCaptureEntityKind::BlockObject, Flags);
+  }
+  if (!T->isObjCRetainableType())
+// For all other types, the memcpy is fine.
+return std::make_pair(BlockCaptureEntityKind::None, Flags);
+
+  Flags = BLOCK_FIELD_IS_OBJECT;
+  bool isBlockPointer = T->isBlockPointerType();
+  if (isBlockPointer)
+Flags = BLOCK_FIELD_IS_BLOCK;
+
+  // Special rules for ARC captures:
+  Qualifiers QS = T.getQualifiers();
+
+  // We need to register __weak direct captures with the runtime.
+  if (QS.getObjCLifetime() == Qualifiers::OCL_Weak)
+return std::make_pair(BlockCaptureEntityKind::ARCWeak, Flags);
+
+  // We need to retain the copied value for __strong direct captures.
+  if (QS.getObjCLifetime() == Qualifiers::OCL_Strong) {
+// If it's a block pointer, we have to copy the block and
+// assign that to the destination pointer, so we might as
+// well use _Block_object_assign.  Otherwise we can avoid that.
+return std::make_pair(!isBlockPointer ? BlockCaptureEntityKind::ARCStrong
+  : BlockCaptureEntityKind::BlockObject,
+  Flags);
+  }
+
+  // Non-ARC captures of retainable pointers are strong and
+  // therefore require a call to _Block_object_assign.
+  if (!QS.getObjCLifetime() && !LangOpts.ObjCAutoRefCount)
+return std::make_pair(BlockCaptureEntityKind::BlockObject, Flags);
+
+  // Otherwise the memcpy is fine.
+  return std::make_pair(BlockCaptureEntityKind::None, Flags);
+}
+
+/// Find the set of block captures that need to be explicitly copied or destroy.
+static void findBlockCapturedManagedEntities(
+const CGBlockInfo &BlockInfo, const LangOptions &LangOpts,
+SmallVectorImpl &ManagedCaptures,
+llvm::function_ref(
+const BlockDecl::Capture &, QualType, const LangOptions &)>
+Predicate) {
+  for (const auto &CI : BlockInfo.getBlockDecl()->captures()) {
+const VarDecl *Variable = CI.getVariable();
+const CGBlockInfo::Capture &Capture = BlockInfo.getCapture(Variable);
+if (Capture.isConstant())
+  continue;
+
+auto Info = Predicate(CI, Variable->getType(), LangOpts);
+if (Info.first != BlockCaptureEntityKind::None)
+  ManagedCaptures.emplace_back(Info.first, Info.second, CI, Capture);
+  }
+}
+
 /// Generate the copy-helper function for a block closure object:
 ///   static void block_copy_helper(block_t *dst, block_t *src);
 /// The runtime will have previously initialized 'dst' by doing a
@@ -1431,78 +1528,28 @@
   dst = Address(Builder.CreateLoad(dst), blockInfo.BlockAlign);
   dst = Builder.CreateBitCast(dst, structPtrTy, "block.dest");
 
-  const BlockDecl *blockDecl = blockInfo.getBlockDecl();
-
-  for (const auto &CI : blockDecl->captures()) {
-const VarDecl *variable = CI.getVariable();
-QualType type = variable->getType();
-
-const CGBlockInfo::Capture &capture = blockInfo.getCapture(variable);
-if (capture.isConstant()) continue;
-
-const Expr *copyExpr = CI.getCopyExpr();
-BlockFieldFlags flags;
+  SmallVector CopiedCaptures;
+  findBlockCapturedManagedEntities(blockInfo, getLangOpts(), CopiedCaptures,
+

LLVM buildmaster will be updated and restarted tonight

[Galina Kistanova via cfe-commits]

Hello everyone,

LLVM buildmaster will be updated and restarted after 6 PM Pacific time
today.

Thanks

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

Re: [libcxx] r296561 - Fix PR32097 - is_abstract doesn't work on class templates.

[Hans Wennborg via cfe-commits]

We're at the "should have tagged 'final' days ago" stage :-)

Since it's not a regression, I would prefer not to merge it unless you
feel super strongly about it.

Sounds like a good candidate for 4.0.1 (tracking bug is PR32061).

Thanks,
Hans

On Wed, Mar 1, 2017 at 1:36 AM, Eric Fiselier  wrote:
> @Hans Where are we in the release process? I would like to merge this into
> 4.0.
>
> Although it's not a regression, it is a significant bug. This patch fixes
> the bug by
> forwarding to a compiler builtin, which is strictly better than what we
> have. I'm
> confident this patch is safe.
>
> /Eric
>
> On Tue, Feb 28, 2017 at 6:27 PM, Eric Fiselier via cfe-commits
>  wrote:
>>
>> Author: ericwf
>> Date: Tue Feb 28 19:27:14 2017
>> New Revision: 296561
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=296561&view=rev
>> Log:
>> Fix PR32097 - is_abstract doesn't work on class templates.
>>
>> This patch fixes llvm.org/PR32097 by using the __is_abstract
>> builtin type-trait instead of the previous library-only implementation.
>>
>> All supported compilers provide this trait. I've tested as far
>> back as Clang 3.2, GCC 4.6 and MSVC trunk.
>>
>> Modified:
>> libcxx/trunk/include/type_traits
>>
>> libcxx/trunk/test/std/utilities/meta/meta.unary/meta.unary.prop/is_abstract.pass.cpp
>>
>> Modified: libcxx/trunk/include/type_traits
>> URL:
>> http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/type_traits?rev=296561&r1=296560&r2=296561&view=diff
>>
>> ==
>> --- libcxx/trunk/include/type_traits (original)
>> +++ libcxx/trunk/include/type_traits Tue Feb 28 19:27:14 2017
>> @@ -1297,18 +1297,8 @@ template  using decay_t = typ
>>
>>  // is_abstract
>>
>> -namespace __is_abstract_imp
>> -{
>> -template  char  __test(_Tp (*)[1]);
>> -template  __two __test(...);
>> -}
>> -
>> -template ::value>
>> -struct __libcpp_abstract : public integral_constant> sizeof(__is_abstract_imp::__test<_Tp>(0)) != 1> {};
>> -
>> -template  struct __libcpp_abstract<_Tp, false> : public
>> false_type {};
>> -
>> -template  struct _LIBCPP_TEMPLATE_VIS is_abstract : public
>> __libcpp_abstract<_Tp> {};
>> +template  struct _LIBCPP_TEMPLATE_VIS is_abstract
>> +: public integral_constant {};
>>
>>  #if _LIBCPP_STD_VER > 14 && !defined(_LIBCPP_HAS_NO_VARIABLE_TEMPLATES)
>>  template  _LIBCPP_CONSTEXPR bool is_abstract_v
>>
>> Modified:
>> libcxx/trunk/test/std/utilities/meta/meta.unary/meta.unary.prop/is_abstract.pass.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/meta/meta.unary/meta.unary.prop/is_abstract.pass.cpp?rev=296561&r1=296560&r2=296561&view=diff
>>
>> ==
>> ---
>> libcxx/trunk/test/std/utilities/meta/meta.unary/meta.unary.prop/is_abstract.pass.cpp
>> (original)
>> +++
>> libcxx/trunk/test/std/utilities/meta/meta.unary/meta.unary.prop/is_abstract.pass.cpp
>> Tue Feb 28 19:27:14 2017
>> @@ -65,6 +65,14 @@ class Abstract
>>  virtual ~Abstract() = 0;
>>  };
>>
>> +template 
>> +struct AbstractTemplate {
>> +  virtual void test() = 0;
>> +};
>> +
>> +template <>
>> +struct AbstractTemplate {};
>> +
>>  int main()
>>  {
>>  test_is_not_abstract();
>> @@ -81,4 +89,6 @@ int main()
>>  test_is_not_abstract();
>>
>>  test_is_abstract();
>> +test_is_abstract >();
>> +test_is_not_abstract >();
>>  }
>>
>>
>> ___
>> 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

r296638 - [clang-format] Don't add namespace end comments for unbalanced right braces after namespace end

[Krasimir Georgiev via cfe-commits]

Author: krasimir
Date: Wed Mar  1 10:38:08 2017
New Revision: 296638

URL: http://llvm.org/viewvc/llvm-project?rev=296638&view=rev
Log:
[clang-format] Don't add namespace end comments for unbalanced right braces 
after namespace end

Modified:
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/unittests/Format/NamespaceEndCommentsFixerTest.cpp

Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=296638&r1=296637&r2=296638&view=diff
==
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Wed Mar  1 10:38:08 2017
@@ -2048,6 +2048,7 @@ void UnwrappedLineParser::addUnwrappedLi
   });
   CurrentLines->push_back(std::move(*Line));
   Line->Tokens.clear();
+  Line->MatchingOpeningBlockLineIndex = UnwrappedLine::kInvalidIndex;
   if (CurrentLines == &Lines && !PreprocessorDirectives.empty()) {
 CurrentLines->append(
 std::make_move_iterator(PreprocessorDirectives.begin()),

Modified: cfe/trunk/unittests/Format/NamespaceEndCommentsFixerTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/NamespaceEndCommentsFixerTest.cpp?rev=296638&r1=296637&r2=296638&view=diff
==
--- cfe/trunk/unittests/Format/NamespaceEndCommentsFixerTest.cpp (original)
+++ cfe/trunk/unittests/Format/NamespaceEndCommentsFixerTest.cpp Wed Mar  1 
10:38:08 2017
@@ -345,6 +345,18 @@ TEST_F(NamespaceEndCommentsFixerTest,
 "}\n"
 "}\n"));
 }
+
+TEST_F(NamespaceEndCommentsFixerTest,
+   DoesNotAddEndCommentForUnbalancedRBracesAfterNamespaceEnd) {
+  EXPECT_EQ("namespace {\n"
+"  int i;\n"
+"} // namespace\n"
+"}",
+fixNamespaceEndComments("namespace {\n"
+"  int i;\n"
+"} // namespace\n"
+"}"));
+}
 } // end namespace
 } // end namespace format
 } // end namespace clang


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

[PATCH] D30499: [analyzer] pr32088: Don't destroy the temporary if its initializer causes return.

[Devin Coughlin via Phabricator via cfe-commits]

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

Seems like a safe quick fix for the crash. Looks good to me!


https://reviews.llvm.org/D30499



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

[PATCH] D30326: [MS-ABI] Allow #pragma section to choose for ZI data

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Adding incompatible extensions to MS extensions sounds like bad precedent to 
me. I think it'd be good if a code owner could opine on this change before it 
goes in.


https://reviews.llvm.org/D30326



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

[clang-tools-extra] r296637 - [clangd] Unbreak the shared build.

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Wed Mar  1 10:23:40 2017
New Revision: 296637

URL: http://llvm.org/viewvc/llvm-project?rev=296637&view=rev
Log:
[clangd] Unbreak the shared build.

Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt

Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=296637&r1=296636&r2=296637&view=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Wed Mar  1 10:23:40 2017
@@ -13,5 +13,6 @@ target_link_libraries(clangd
   clangFormat
   clangFrontend
   clangTooling
+  clangToolingCore
   LLVMSupport
   )


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

[PATCH] D30498: [clangd] Add support for FixIts.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL296636: [clangd] Add support for FixIts. (authored by d0k).

Changed prior to commit:
  https://reviews.llvm.org/D30498?vs=90181&id=90187#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30498

Files:
  clang-tools-extra/trunk/clangd/ASTManager.cpp
  clang-tools-extra/trunk/clangd/ASTManager.h
  clang-tools-extra/trunk/clangd/ClangDMain.cpp
  clang-tools-extra/trunk/clangd/Protocol.cpp
  clang-tools-extra/trunk/clangd/Protocol.h
  clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp
  clang-tools-extra/trunk/clangd/ProtocolHandlers.h
  clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts
  clang-tools-extra/trunk/test/clangd/fixits.test
  clang-tools-extra/trunk/test/clangd/formatting.test

Index: clang-tools-extra/trunk/clangd/ClangDMain.cpp
===
--- clang-tools-extra/trunk/clangd/ClangDMain.cpp
+++ clang-tools-extra/trunk/clangd/ClangDMain.cpp
@@ -11,13 +11,20 @@
 #include "DocumentStore.h"
 #include "JSONRPCDispatcher.h"
 #include "ProtocolHandlers.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Program.h"
 #include 
 #include 
 using namespace clang::clangd;
 
+static llvm::cl::opt
+RunSynchronously("run-synchronously",
+ llvm::cl::desc("parse on main thread"),
+ llvm::cl::init(false), llvm::cl::Hidden);
+
 int main(int argc, char *argv[]) {
+  llvm::cl::ParseCommandLineOptions(argc, argv, "clangd");
   llvm::raw_ostream &Outs = llvm::outs();
   llvm::raw_ostream &Logs = llvm::errs();
   JSONOutput Out(Outs, Logs);
@@ -28,14 +35,14 @@
   // Set up a document store and intialize all the method handlers for JSONRPC
   // dispatching.
   DocumentStore Store;
-  ASTManager AST(Out, Store);
+  ASTManager AST(Out, Store, RunSynchronously);
   Store.addListener(&AST);
   JSONRPCDispatcher Dispatcher(llvm::make_unique(Out));
   Dispatcher.registerHandler("initialize",
  llvm::make_unique(Out));
   auto ShutdownPtr = llvm::make_unique(Out);
   auto *ShutdownHandler = ShutdownPtr.get();
-  Dispatcher.registerHandler("shutdown",std::move(ShutdownPtr));
+  Dispatcher.registerHandler("shutdown", std::move(ShutdownPtr));
   Dispatcher.registerHandler(
   "textDocument/didOpen",
   llvm::make_unique(Out, Store));
@@ -52,6 +59,8 @@
   Dispatcher.registerHandler(
   "textDocument/formatting",
   llvm::make_unique(Out, Store));
+  Dispatcher.registerHandler("textDocument/codeAction",
+ llvm::make_unique(Out, AST));
 
   while (std::cin.good()) {
 // A Language Server Protocol message starts with a HTTP header, delimited
Index: clang-tools-extra/trunk/clangd/ProtocolHandlers.h
===
--- clang-tools-extra/trunk/clangd/ProtocolHandlers.h
+++ clang-tools-extra/trunk/clangd/ProtocolHandlers.h
@@ -22,6 +22,7 @@
 
 namespace clang {
 namespace clangd {
+class ASTManager;
 class DocumentStore;
 
 struct InitializeHandler : Handler {
@@ -34,7 +35,8 @@
   "textDocumentSync": 1,
   "documentFormattingProvider": true,
   "documentRangeFormattingProvider": true,
-  "documentOnTypeFormattingProvider": {"firstTriggerCharacter":"}","moreTriggerCharacter":[]}
+  "documentOnTypeFormattingProvider": {"firstTriggerCharacter":"}","moreTriggerCharacter":[]},
+  "codeActionProvider": true
 }}})");
   }
 };
@@ -102,6 +104,16 @@
   DocumentStore &Store;
 };
 
+struct CodeActionHandler : Handler {
+  CodeActionHandler(JSONOutput &Output, ASTManager &AST)
+  : Handler(Output), AST(AST) {}
+
+  void handleMethod(llvm::yaml::MappingNode *Params, StringRef ID) override;
+
+private:
+  ASTManager *
+};
+
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/trunk/clangd/ASTManager.h
===
--- clang-tools-extra/trunk/clangd/ASTManager.h
+++ clang-tools-extra/trunk/clangd/ASTManager.h
@@ -12,6 +12,8 @@
 
 #include "DocumentStore.h"
 #include "JSONRPCDispatcher.h"
+#include "Protocol.h"
+#include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include 
 #include 
@@ -29,17 +31,27 @@
 
 class ASTManager : public DocumentStoreListener {
 public:
-  ASTManager(JSONOutput &Output, DocumentStore &Store);
+  ASTManager(JSONOutput &Output, DocumentStore &Store, bool RunSynchronously);
   ~ASTManager() override;
 
   void onDocumentAdd(StringRef Uri) override;
   // FIXME: Implement onDocumentRemove
   // FIXME: Implement codeComplete
 
+  /// Get the fixes associated with a certain diagnostic as replacements.
+  llvm::ArrayRef
+  getFixIts(const clangd::Diagnostic &D);
+
+  DocumentStore &getStore() const { return Store; }
+
 private:
   JSONOutput &Output;
   DocumentStore &Store;

[clang-tools-extra] r296636 - [clangd] Add support for FixIts.

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Wed Mar  1 10:16:29 2017
New Revision: 296636

URL: http://llvm.org/viewvc/llvm-project?rev=296636&view=rev
Log:
[clangd] Add support for FixIts.

Summary:
This uses CodeActions to show 'apply fix' actions when code actions are
requested for a location. The actions themselves make use of a
clangd.applyFix command which has to be implemented on the editor side. I
included an implementation for vscode.

This also adds a -run-synchronously flag which runs everything on the main
thread. This is useful for testing.

Reviewers: krasimir

Subscribers: cfe-commits

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

Added:
clang-tools-extra/trunk/test/clangd/fixits.test
Modified:
clang-tools-extra/trunk/clangd/ASTManager.cpp
clang-tools-extra/trunk/clangd/ASTManager.h
clang-tools-extra/trunk/clangd/ClangDMain.cpp
clang-tools-extra/trunk/clangd/Protocol.cpp
clang-tools-extra/trunk/clangd/Protocol.h
clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp
clang-tools-extra/trunk/clangd/ProtocolHandlers.h
clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts
clang-tools-extra/trunk/test/clangd/formatting.test

Modified: clang-tools-extra/trunk/clangd/ASTManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ASTManager.cpp?rev=296636&r1=296635&r2=296636&view=diff
==
--- clang-tools-extra/trunk/clangd/ASTManager.cpp (original)
+++ clang-tools-extra/trunk/clangd/ASTManager.cpp Wed Mar  1 10:16:29 2017
@@ -54,8 +54,9 @@ static int getSeverity(DiagnosticsEngine
   llvm_unreachable("Unknown diagnostic level!");
 }
 
-ASTManager::ASTManager(JSONOutput &Output, DocumentStore &Store)
-: Output(Output), Store(Store),
+ASTManager::ASTManager(JSONOutput &Output, DocumentStore &Store,
+   bool RunSynchronously)
+: Output(Output), Store(Store), RunSynchronously(RunSynchronously),
   PCHs(std::make_shared()),
   ClangWorker([this]() { runWorker(); }) {}
 
@@ -67,9 +68,8 @@ void ASTManager::runWorker() {
   std::unique_lock Lock(RequestLock);
   // Check if there's another request pending. We keep parsing until
   // our one-element queue is empty.
-  ClangRequestCV.wait(Lock, [this] {
-return !RequestQueue.empty() || Done;
-  });
+  ClangRequestCV.wait(Lock,
+  [this] { return !RequestQueue.empty() || Done; });
 
   if (RequestQueue.empty() && Done)
 return;
@@ -78,49 +78,67 @@ void ASTManager::runWorker() {
   RequestQueue.pop_back();
 } // unlock.
 
-auto &Unit = ASTs[File]; // Only one thread can access this at a time.
+parseFileAndPublishDiagnostics(File);
+  }
+}
 
-if (!Unit) {
-  Unit = createASTUnitForFile(File, this->Store);
-} else {
-  // Do a reparse if this wasn't the first parse.
-  // FIXME: This might have the wrong working directory if it changed in 
the
-  // meantime.
-  Unit->Reparse(PCHs, getRemappedFiles(this->Store));
-}
+void ASTManager::parseFileAndPublishDiagnostics(StringRef File) {
+  auto &Unit = ASTs[File]; // Only one thread can access this at a time.
 
-if (!Unit)
-  continue;
+  if (!Unit) {
+Unit = createASTUnitForFile(File, this->Store);
+  } else {
+// Do a reparse if this wasn't the first parse.
+// FIXME: This might have the wrong working directory if it changed in the
+// meantime.
+Unit->Reparse(PCHs, getRemappedFiles(this->Store));
+  }
+
+  if (!Unit)
+return;
 
-// Send the diagnotics to the editor.
-// FIXME: If the diagnostic comes from a different file, do we want to
-// show them all? Right now we drop everything not coming from the
-// main file.
-// FIXME: Send FixIts to the editor.
-std::string Diagnostics;
-for (ASTUnit::stored_diag_iterator D = Unit->stored_diag_begin(),
-   DEnd = Unit->stored_diag_end();
- D != DEnd; ++D) {
-  if (!D->getLocation().isValid() ||
-  !D->getLocation().getManager().isInMainFile(D->getLocation()))
-continue;
-  Position P;
-  P.line = D->getLocation().getSpellingLineNumber() - 1;
-  P.character = D->getLocation().getSpellingColumnNumber();
-  Range R = {P, P};
-  Diagnostics +=
-  R"({"range":)" + Range::unparse(R) +
-  R"(,"severity":)" + std::to_string(getSeverity(D->getLevel())) +
-  R"(,"message":")" + llvm::yaml::escape(D->getMessage()) +
-  R"("},)";
+  // Send the diagnotics to the editor.
+  // FIXME: If the diagnostic comes from a different file, do we want to
+  // show them all? Right now we drop everything not coming from the
+  // main file.
+  std::string Diagnostics;
+  DiagnosticToReplacementMap LocalFixIts; // Temporary storage
+  for (ASTUnit::stored_diag_iterator D = Unit->stored_diag_begin(),
+ DEn

[PATCH] D30326: [MS-ABI] Allow #pragma section to choose for ZI data

[Javed Absar via Phabricator via cfe-commits]

javed.absar added a comment.

Hi Nico:
This is for the case when using  '#pragma bss_seg..' feature outside of msvc 
context. That's the reason for the flag.
One could use attribute, but as attribute is attached to individual 
declaration, while pragma applies to whole file (unless superseded by another 
pragma),  the latter is easier to use in some cases (i.e. when there are lots 
of such declarations). Hope this clarifies the question you asked.
Best Regards
Javed


https://reviews.llvm.org/D30326



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

[PATCH] D30498: [clangd] Add support for FixIts.

[Krasimir Georgiev via Phabricator via cfe-commits]

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

Looks Great!


https://reviews.llvm.org/D30498



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

[PATCH] D30498: [clangd] Add support for FixIts.

[Benjamin Kramer via Phabricator via cfe-commits]

bkramer updated this revision to Diff 90181.
bkramer added a comment.

- Use typedef instead of decltype()
- Rename local variable not to shadow member.
- Give FixIts their own mutex
- Use const instead of let in typescript


https://reviews.llvm.org/D30498

Files:
  clangd/ASTManager.cpp
  clangd/ASTManager.h
  clangd/ClangDMain.cpp
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h
  clangd/clients/clangd-vscode/src/extension.ts
  test/clangd/fixits.test
  test/clangd/formatting.test

Index: test/clangd/formatting.test
===
--- test/clangd/formatting.test
+++ test/clangd/formatting.test
@@ -4,12 +4,13 @@
 Content-Length: 125
 
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
-# CHECK: Content-Length: 294
+# CHECK: Content-Length: 332
 # CHECK: {"jsonrpc":"2.0","id":0,"result":{"capabilities":{
 # CHECK:   "textDocumentSync": 1,
 # CHECK:   "documentFormattingProvider": true,
 # CHECK:   "documentRangeFormattingProvider": true,
-# CHECK:   "documentOnTypeFormattingProvider": {"firstTriggerCharacter":"}","moreTriggerCharacter":[]}
+# CHECK:   "documentOnTypeFormattingProvider": {"firstTriggerCharacter":"}","moreTriggerCharacter":[]},
+# CHECK:   "codeActionProvider": true
 # CHECK: }}}
 #
 Content-Length: 193
Index: test/clangd/fixits.test
===
--- /dev/null
+++ test/clangd/fixits.test
@@ -0,0 +1,22 @@
+# RUN: clangd -run-synchronously < %s | FileCheck %s
+# It is absolutely vital that this file has CRLF line endings.
+#
+Content-Length: 125
+
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+#
+Content-Length: 180
+
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///foo.c","languageId":"c","version":1,"text":"int main(int i, char **a) { if (i = 2) {}}"}}}
+#
+# CHECK: {"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"uri":"file:///foo.c","diagnostics":[{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":2,"message":"using the result of an assignment as a condition without parentheses"},{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":3,"message":"place parentheses around the assignment to silence this warning"},{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":3,"message":"use '==' to turn this assignment into an equality comparison"}]}}
+#
+Content-Length: 746
+
+ {"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"file:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":2,"message":"using the result of an assignment as a condition without parentheses"},{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":3,"message":"place parentheses around the assignment to silence this warning"},{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":3,"message":"use '==' to turn this assignment into an equality comparison"}]}}}
+#
+# CHECK: {"jsonrpc":"2.0","id":2, "result": [{"title":"Apply FixIt 'place parentheses around the assignment to silence this warning'", "command": "clangd.applyFix", "arguments": ["file:///foo.c", [{"range": {"start": {"line": 0, "character": 32}, "end": {"line": 0, "character": 32}}, "newText": "("},{"range": {"start": {"line": 0, "character": 37}, "end": {"line": 0, "character": 37}}, "newText": ")"}]]},{"title":"Apply FixIt 'use '==' to turn this assignment into an equality comparison'", "command": "clangd.applyFix", "arguments": ["file:///foo.c", [{"range": {"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 35}}, "newText": "=="}]]}]
+#
+Content-Length: 44
+
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
Index: clangd/clients/clangd-vscode/src/extension.ts
===
--- clangd/clients/clangd-vscode/src/extension.ts
+++ clangd/clients/clangd-vscode/src/extension.ts
@@ -18,9 +18,25 @@
 
 const clangdClient = new vscodelc.LanguageClient('Clang Language Server', serverOptions, clientOptions);
 
+function applyTextEdits(uri: string, edits: vscodelc.TextEdit[]) {
+let textEditor = vscode.window.activeTextEditor;
+
+if (textEditor && textEditor.document.uri.toString() === uri) {
+textEditor.edit(mutator => {
+for (const edit of edits) {
+mutator.replace(vscodelc.Protocol2Code.asRange(edit.range), edit.newText);
+}
+}

[PATCH] D30111: [clang-format] Add a test to check at once all the Mozilla coding style

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added a comment.

Manuel, is that ok with you? thanks (I will rename the file)


https://reviews.llvm.org/D30111



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

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

[Daniel Jasper via Phabricator via cfe-commits]

djasper added a comment.

Could you please upload a diff with the entire file as context? That makes 
reviewing this easier.




Comment at: docs/ClangFormatStyleOptions.rst:428
 
+**BreakBeforeInhertianceDelimiter** (``boolt``)
+  If ``true``, in the class inheritance expression clang-format will

Auto-generate this with docs/tools/dump_format_style.py



Comment at: include/clang/Format/Format.h:309
+  /// inheritance.
+  bool BreakBeforeInhertianceDelimiter;
+  

s/Delimiter/Colon/

Not because it is better, but just because it's more consistent with much of 
the rest of clang-format.



Comment at: lib/Format/TokenAnnotator.cpp:950
 bool InCtorInitializer = false;
+bool InInhertiance = false;
 bool CaretFound = false;

Maybe "InInheritanceList"?



Comment at: lib/Format/TokenAnnotator.cpp:1012
+   Current.Previous->is(TT_InheritanceColon)) {
+  Contexts.back().IsExpression = true;
+  Contexts.back().InInhertiance = true;

Why would we be in an expression here?



Comment at: lib/Format/TokenAnnotator.cpp:2398
 
+// Determine if the next token from the closing scope is an inheritance token
+static bool hasMultipleInheritance(const FormatToken &Tok) {

I don't understand this comment or what the function is supposed to do. It 
seems to search whether there is an inheritance comma somewhere in the rest of 
the line.



Comment at: lib/Format/TokenAnnotator.cpp:2490
 return true;
+  //Break only if we have multiple inheritance
+  if (Style.BreakBeforeInhertianceDelimiter &&

nit: Use periods of the end of sentences and a space after //.



Comment at: unittests/Format/FormatTest.cpp:1023
 
+TEST_F(FormatTest, BreakBeforeInhertianceDelimiter) {
+  FormatStyle StyleWithInheritanceBreak = getLLVMStyle();

I am missing tests that show the behavior when there are multiple base classes, 
but they do fit on one line.



Comment at: unittests/Format/FormatTest.cpp:1033
+   StyleWithInheritanceBreak);
+  verifyFormat("class MyClass"
+   "  : public X\n"

What is this supposed to test?



Comment at: unittests/Format/FormatTest.cpp:1039
+  verifyFormat("class MyClass\n"
+   ": public X // When deriving from more than one class, put "
+   "each on its own\n"

Sure, but the comment is forcing that, so I don't know what this test does.


Repository:
  rL LLVM

https://reviews.llvm.org/D30487



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

[PATCH] D30405: [clang-format] Add a new flag FixNamespaceComments to FormatStyle

[Krasimir Georgiev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL296632: [clang-format] Add a new flag FixNamespaceComments 
to FormatStyle (authored by krasimir).

Changed prior to commit:
  https://reviews.llvm.org/D30405?vs=90172&id=90179#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30405

Files:
  cfe/trunk/docs/ClangFormatStyleOptions.rst
  cfe/trunk/include/clang/Format/Format.h
  cfe/trunk/lib/Format/Format.cpp
  cfe/trunk/lib/Index/CommentToXML.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp

Index: cfe/trunk/lib/Index/CommentToXML.cpp
===
--- cfe/trunk/lib/Index/CommentToXML.cpp
+++ cfe/trunk/lib/Index/CommentToXML.cpp
@@ -593,9 +593,11 @@
   unsigned Length = Declaration.size();
 
   bool IncompleteFormat = false;
+  format::FormatStyle Style = format::getLLVMStyle();
+  Style.FixNamespaceComments = false;
   tooling::Replacements Replaces =
-  reformat(format::getLLVMStyle(), StringDecl,
-   tooling::Range(Offset, Length), "xmldecl.xd", &IncompleteFormat);
+  reformat(Style, StringDecl, tooling::Range(Offset, Length), "xmldecl.xd",
+   &IncompleteFormat);
   auto FormattedStringDecl = applyAllReplacements(StringDecl, Replaces);
   if (static_cast(FormattedStringDecl)) {
 Declaration = *FormattedStringDecl;
Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -308,6 +308,7 @@
 IO.mapOptional("DisableFormat", Style.DisableFormat);
 IO.mapOptional("ExperimentalAutoDetectBinPacking",
Style.ExperimentalAutoDetectBinPacking);
+IO.mapOptional("FixNamespaceComments", Style.FixNamespaceComments);
 IO.mapOptional("ForEachMacros", Style.ForEachMacros);
 IO.mapOptional("IncludeCategories", Style.IncludeCategories);
 IO.mapOptional("IncludeIsMainRegex", Style.IncludeIsMainRegex);
@@ -529,6 +530,7 @@
   LLVMStyle.Cpp11BracedListStyle = true;
   LLVMStyle.DerivePointerAlignment = false;
   LLVMStyle.ExperimentalAutoDetectBinPacking = false;
+  LLVMStyle.FixNamespaceComments = true;
   LLVMStyle.ForEachMacros.push_back("foreach");
   LLVMStyle.ForEachMacros.push_back("Q_FOREACH");
   LLVMStyle.ForEachMacros.push_back("BOOST_FOREACH");
@@ -676,6 +678,7 @@
   MozillaStyle.ConstructorInitializerIndentWidth = 2;
   MozillaStyle.ContinuationIndentWidth = 2;
   MozillaStyle.Cpp11BracedListStyle = false;
+  MozillaStyle.FixNamespaceComments = false;
   MozillaStyle.IndentCaseLabels = true;
   MozillaStyle.ObjCSpaceAfterProperty = true;
   MozillaStyle.ObjCSpaceBeforeProtocolList = false;
@@ -696,6 +699,7 @@
   Style.BreakConstructorInitializersBeforeComma = true;
   Style.Cpp11BracedListStyle = false;
   Style.ColumnLimit = 0;
+  Style.FixNamespaceComments = false;
   Style.IndentWidth = 4;
   Style.NamespaceIndentation = FormatStyle::NI_Inner;
   Style.ObjCBlockIndentWidth = 4;
@@ -713,6 +717,7 @@
   Style.BreakBeforeTernaryOperators = true;
   Style.Cpp11BracedListStyle = false;
   Style.ColumnLimit = 79;
+  Style.FixNamespaceComments = false;
   Style.SpaceBeforeParens = FormatStyle::SBPO_Always;
   Style.Standard = FormatStyle::LS_Cpp03;
   return Style;
@@ -1829,20 +1834,32 @@
 return tooling::Replacements();
   auto Env = Environment::CreateVirtualEnvironment(Code, FileName, Ranges);
 
-  if (Style.Language == FormatStyle::LK_JavaScript &&
-  Style.JavaScriptQuotes != FormatStyle::JSQS_Leave) {
-JavaScriptRequoter Requoter(*Env, Expanded);
-tooling::Replacements Requotes = Requoter.process();
-if (!Requotes.empty()) {
-  auto NewCode = applyAllReplacements(Code, Requotes);
+  auto reformatAfterApplying = [&] (TokenAnalyzer& Fixer) {
+tooling::Replacements Fixes = Fixer.process();
+if (!Fixes.empty()) {
+  auto NewCode = applyAllReplacements(Code, Fixes);
   if (NewCode) {
 auto NewEnv = Environment::CreateVirtualEnvironment(
 *NewCode, FileName,
-tooling::calculateRangesAfterReplacements(Requotes, Ranges));
+tooling::calculateRangesAfterReplacements(Fixes, Ranges));
 Formatter Format(*NewEnv, Expanded, IncompleteFormat);
-return Requotes.merge(Format.process());
+return Fixes.merge(Format.process());
   }
 }
+Formatter Format(*Env, Expanded, IncompleteFormat);
+return Format.process();
+  };
+
+  if (Style.Language == FormatStyle::LK_Cpp &&
+  Style.FixNamespaceComments) {
+NamespaceEndCommentsFixer CommentsFixer(*Env, Expanded);
+return reformatAfterApplying(CommentsFixer);
+  }
+
+  if (Style.Language == FormatStyle::LK_JavaScript &&
+  Style.JavaScriptQuotes != FormatStyle::JSQS_Leave) {
+JavaScriptRequoter Requoter(*Env, Expanded);
+return reformatAfterApplying(Requoter);
   }
 
   Formatter Format(*Env, Expanded, IncompleteFormat);
Index: cfe/trunk/unittests

r296632 - [clang-format] Add a new flag FixNamespaceComments to FormatStyle

[Krasimir Georgiev via cfe-commits]

Author: krasimir
Date: Wed Mar  1 09:35:39 2017
New Revision: 296632

URL: http://llvm.org/viewvc/llvm-project?rev=296632&view=rev
Log:
[clang-format] Add a new flag FixNamespaceComments to FormatStyle

Summary:
This patch enables namespace end comments under a new flag FixNamespaceComments,
which is enabled for the LLVM and Google styles.

Reviewers: djasper

Reviewed By: djasper

Subscribers: cfe-commits, klimek

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

Modified:
cfe/trunk/docs/ClangFormatStyleOptions.rst
cfe/trunk/include/clang/Format/Format.h
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Index/CommentToXML.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=296632&r1=296631&r2=296632&view=diff
==
--- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst Wed Mar  1 09:35:39 2017
@@ -471,6 +471,10 @@ the configuration (without a prefix: ``A
   NOTE: This is an experimental flag, that might go away or be renamed. Do
   not use this in config files, etc. Use at your own risk.
 
+**FixNamespaceComments** (``bool``)
+  If ``true``, clang-format adds missing namespace end comments and
+  fixes invalid existing ones.
+
 **ForEachMacros** (``std::vector``)
   A vector of macros that should be interpreted as foreach loops
   instead of as function calls.
@@ -561,6 +565,9 @@ the configuration (without a prefix: ``A
 
 
 
+**JavaScriptWrapImports** (``bool``)
+  Whether to wrap JavaScript import/export statements.
+
 **KeepEmptyLinesAtTheStartOfBlocks** (``bool``)
   If true, empty lines at the start of blocks are kept.
 
@@ -573,7 +580,7 @@ the configuration (without a prefix: ``A
 Do not use.
 
   * ``LK_Cpp`` (in configuration: ``Cpp``)
-Should be used for C, C++, ObjectiveC, ObjectiveC++.
+Should be used for C, C++.
 
   * ``LK_Java`` (in configuration: ``Java``)
 Should be used for Java.
@@ -581,6 +588,9 @@ the configuration (without a prefix: ``A
   * ``LK_JavaScript`` (in configuration: ``JavaScript``)
 Should be used for JavaScript.
 
+  * ``LK_ObjC`` (in configuration: ``ObjC``)
+Should be used for Objective-C, Objective-C++.
+
   * ``LK_Proto`` (in configuration: ``Proto``)
 Should be used for Protocol Buffers
 (https://developers.google.com/protocol-buffers/).
@@ -755,6 +765,9 @@ the configuration (without a prefix: ``A
   * ``UT_ForIndentation`` (in configuration: ``ForIndentation``)
 Use tabs only for indentation.
 
+  * ``UT_ForContinuationAndIndentation`` (in configuration: 
``ForContinuationAndIndentation``)
+Use tabs only for line continuation and indentation.
+
   * ``UT_Always`` (in configuration: ``Always``)
 Use tabs whenever we need to fill whitespace that spans at least from
 one tab stop to the next one.

Modified: cfe/trunk/include/clang/Format/Format.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=296632&r1=296631&r2=296632&view=diff
==
--- cfe/trunk/include/clang/Format/Format.h (original)
+++ cfe/trunk/include/clang/Format/Format.h Wed Mar  1 09:35:39 2017
@@ -349,6 +349,10 @@ struct FormatStyle {
   /// not use this in config files, etc. Use at your own risk.
   bool ExperimentalAutoDetectBinPacking;
 
+  /// \brief If ``true``, clang-format adds missing namespace end comments and
+  /// fixes invalid existing ones.
+  bool FixNamespaceComments;
+
   /// \brief A vector of macros that should be interpreted as foreach loops
   /// instead of as function calls.
   ///
@@ -679,6 +683,7 @@ struct FormatStyle {
DisableFormat == R.DisableFormat &&
ExperimentalAutoDetectBinPacking ==
R.ExperimentalAutoDetectBinPacking &&
+   FixNamespaceComments == R.FixNamespaceComments &&
ForEachMacros == R.ForEachMacros &&
IncludeCategories == R.IncludeCategories &&
IndentCaseLabels == R.IndentCaseLabels &&

Modified: cfe/trunk/lib/Format/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=296632&r1=296631&r2=296632&view=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Wed Mar  1 09:35:39 2017
@@ -308,6 +308,7 @@ template <> struct MappingTraitshttp://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/CommentToXML.cpp?rev=296632&r1=296631&r2=296632&view=diff
==
--- cfe/trunk/lib/Index/CommentToXML.cpp (original)
+++ cfe/trunk/lib/Index/CommentToXML.cpp Wed Mar  1 09:35:39 2017
@@ -593,9 +593,11 @@ void CommentASTToXMLConverter::formatTex
   unsigned

[PATCH] D30498: [clangd] Add support for FixIts.

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.



Comment at: clangd/ASTManager.cpp:105
+  std::string Diagnostics;
+  decltype(this->FixIts) LocalFixIts; // Temporary storage
+  for (ASTUnit::stored_diag_iterator D = Unit->stored_diag_begin(),

Why not typedef the type of the FixIts map?



Comment at: clangd/ASTManager.cpp:124
+clangd::Diagnostic Diag = {R, getSeverity(D->getLevel()), D->getMessage()};
+auto &FixIts = LocalFixIts[Diag];
+for (const FixItHint &Fix : D->getFixIts()) {

I prefer this local variable to have a different name.



Comment at: clangd/ASTManager.cpp:134
+std::lock_guard Guard(RequestLock);
+FixIts = std::move(LocalFixIts);
   }

Consider the following situation:
1. worker thread is at line 130 just before the critical section with full info 
about file A in LocalFixits;
main thread takes the lock at line 159 in onDocumentAdd(file B)
2. main thread finishes the critical section, calling CV.notify_one(), which 
has no effect and releases RequestLock.
3. worker thread then gets RequestLock and enters the critical section at line 
133, which outputs essentially stale diagnostics.

Arguably this seems OK, but maybe we want a separate mutex controlling access 
to the FixIts, which also might be more logical.
I can't find a real issue with the code as-is, you can leave it like this, I'm 
just wondering.



Comment at: clangd/clients/clangd-vscode/src/extension.ts:26
+textEditor.edit(mutator => {
+for (let edit of edits) {
+
mutator.replace(vscodelc.Protocol2Code.asRange(edit.range), edit.newText);

Consider using `const` instead of `let`.


https://reviews.llvm.org/D30498



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

[PATCH] D30373: [analyzer] NFC: Update test infrastructure to support multiple constraint managers

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

With the first method, I'm not sure how to refer to the `AnalyzerTest` class 
defined in `lit.cfg` from `lit.local.cfg`. It doesn't seem to be in scope, so 
unless I store an instantiation in the `config` object, I don't think it's 
accessible.


Repository:
  rL LLVM

https://reviews.llvm.org/D30373



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

[PATCH] D30499: [analyzer] pr32088: Don't destroy the temporary if its initializer causes return.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.

In the following code involving GNU statement-expression extension:

  struct S {
~S();
  };
  
  void foo() {
const S &x = ({ return; S(); });
  }

function `foo()` returns before reference `x` is initialized. We shouldn't call 
the destructor for the temporary object lifetime-exteneded by `x` in this case, 
because the object never gets constructed in the first place.

The real problem is probably in the CFG somewhere, so this is a quick-and-dirty 
hotfix rather than the perfect solution.


https://reviews.llvm.org/D30499

Files:
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Analysis/temporaries.cpp


Index: test/Analysis/temporaries.cpp
===
--- test/Analysis/temporaries.cpp
+++ test/Analysis/temporaries.cpp
@@ -493,3 +493,13 @@
 clang_analyzer_eval(x == 47); // expected-warning{{TRUE}}
   }
 }
+
+namespace PR32088 {
+  void testReturnFromStmtExprInitializer() {
+// We shouldn't try to destroy the object pointed to by `obj' upon return.
+const NonTrivial &obj = ({
+  return; // no-crash
+  NonTrivial(42);
+});
+  }
+}
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -615,7 +615,15 @@
   const MemRegion *Region = dest.castAs().getRegion();
 
   if (varType->isReferenceType()) {
-Region = state->getSVal(Region).getAsRegion()->getBaseRegion();
+const MemRegion *ValueRegion = state->getSVal(Region).getAsRegion();
+if (!ValueRegion) {
+  // FIXME: This should not happen. The language guarantees a presence
+  // of a valid initializer here, so the reference shall not be undefined.
+  // It seems that we're calling destructors over variables that
+  // were not initialized yet.
+  return;
+}
+Region = ValueRegion->getBaseRegion();
 varType = cast(Region)->getValueType();
   }
 


Index: test/Analysis/temporaries.cpp
===
--- test/Analysis/temporaries.cpp
+++ test/Analysis/temporaries.cpp
@@ -493,3 +493,13 @@
 clang_analyzer_eval(x == 47); // expected-warning{{TRUE}}
   }
 }
+
+namespace PR32088 {
+  void testReturnFromStmtExprInitializer() {
+// We shouldn't try to destroy the object pointed to by `obj' upon return.
+const NonTrivial &obj = ({
+  return; // no-crash
+  NonTrivial(42);
+});
+  }
+}
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -615,7 +615,15 @@
   const MemRegion *Region = dest.castAs().getRegion();
 
   if (varType->isReferenceType()) {
-Region = state->getSVal(Region).getAsRegion()->getBaseRegion();
+const MemRegion *ValueRegion = state->getSVal(Region).getAsRegion();
+if (!ValueRegion) {
+  // FIXME: This should not happen. The language guarantees a presence
+  // of a valid initializer here, so the reference shall not be undefined.
+  // It seems that we're calling destructors over variables that
+  // were not initialized yet.
+  return;
+}
+Region = ValueRegion->getBaseRegion();
 varType = cast(Region)->getValueType();
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added inline comments.



Comment at: docs/ClangFormatStyleOptions.rst:430
+  If ``true``, in the class inheritance expression clang-format will
+  break before operands ``:`` and ``,`` only if there is multiple
+  inheritance.

s/is/are/




Comment at: docs/ClangFormatStyleOptions.rst:431
+  break before operands ``:`` and ``,`` only if there is multiple
+  inheritance.
+

s/inheritance/ inheritances/




Comment at: include/clang/Format/Format.h:307
+  /// \brief If ``true``, in the class inheritance expression clang-format will
+  /// break before operands ``:`` and ``,`` only if there is multiple
+  /// inheritance.

same typo


Repository:
  rL LLVM

https://reviews.llvm.org/D30487



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

[PATCH] D30405: [clang-format] Add a new flag FixNamespaceComments to FormatStyle

[Daniel Jasper via Phabricator via cfe-commits]

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

LG


https://reviews.llvm.org/D30405



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

[clang-tools-extra] r296627 - [clang-tidy] Attempt to fix the test where exceptions are disabled by default.

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Wed Mar  1 08:41:11 2017
New Revision: 296627

URL: http://llvm.org/viewvc/llvm-project?rev=296627&view=rev
Log:
[clang-tidy] Attempt to fix the test where exceptions are disabled by default.

Modified:
clang-tools-extra/trunk/test/clang-tidy/modernize-use-override.cpp

Modified: clang-tools-extra/trunk/test/clang-tidy/modernize-use-override.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-use-override.cpp?rev=296627&r1=296626&r2=296627&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/modernize-use-override.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-use-override.cpp Wed Mar  
1 08:41:11 2017
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s modernize-use-override %t
+// RUN: %check_clang_tidy %s modernize-use-override %t -- -- -std=c++11 
-fexceptions
 
 #define ABSTRACT = 0
 


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

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

[Andi via Phabricator via cfe-commits]

Abpostelnicu updated this revision to Diff 90176.
Abpostelnicu added a comment.

added patch against the updated repo.


Repository:
  rL LLVM

https://reviews.llvm.org/D30487

Files:
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1020,6 +1020,29 @@
   verifyFormat("class ::A::B {};");
 }
 
+TEST_F(FormatTest, BreakBeforeInhertianceDelimiter) {
+  FormatStyle StyleWithInheritanceBreak = getLLVMStyle();
+  LocalStyle.BreakBeforeInhertianceDelimiter = true;
+
+  verifyFormat("class MyClass : public X {}", LocalStyle);
+  verifyFormat("class MyClass\n"
+   ": public X\n"
+   ", public Y // some difficult comment\n"
+   ", public Z {}",
+   StyleWithInheritanceBreak);
+  verifyFormat("class MyClass"
+   "  : public X\n"
+   "  , public Y // some difficult comment\n"
+   "  , public Z\n",
+   StyleWithInheritanceBreak);
+  verifyFormat("class MyClass\n"
+   ": public X // When deriving from more than one class, put "
+   "each on its own\n"
+   "   // line.\n"
+   ", public Y {}",
+   StyleWithInheritanceBreak);
+}
+
 TEST_F(FormatTest, FormatsVariableDeclarationsAfterStructOrClass) {
   verifyFormat("class A {\n} a, b;");
   verifyFormat("struct A {\n} a, b;");
@@ -8468,6 +8491,7 @@
   CHECK_PARSE_BOOL(BreakBeforeTernaryOperators);
   CHECK_PARSE_BOOL(BreakConstructorInitializersBeforeComma);
   CHECK_PARSE_BOOL(BreakStringLiterals);
+  CHECK_PARSE_BOOL(BreakBeforeInhertianceDelimiter)
   CHECK_PARSE_BOOL(ConstructorInitializerAllOnOneLineOrOnePerLine);
   CHECK_PARSE_BOOL(DerivePointerAlignment);
   CHECK_PARSE_BOOL_FIELD(DerivePointerAlignment, "DerivePointerBinding");
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -676,6 +676,8 @@
 case tok::comma:
   if (Contexts.back().InCtorInitializer)
 Tok->Type = TT_CtorInitializerComma;
+  else if (Contexts.back().InInhertiance)
+Tok->Type = TT_InheritanceComma;
   else if (Contexts.back().FirstStartOfName &&
(Contexts.size() == 1 || Line.startsWith(tok::kw_for))) {
 Contexts.back().FirstStartOfName->PartOfMultiVariableDeclStmt = true;
@@ -945,6 +947,7 @@
 bool CanBeExpression = true;
 bool InTemplateArgument = false;
 bool InCtorInitializer = false;
+bool InInhertiance = false;
 bool CaretFound = false;
 bool IsForEachMacro = false;
   };
@@ -1004,6 +1007,10 @@
Current.Previous->is(TT_CtorInitializerColon)) {
   Contexts.back().IsExpression = true;
   Contexts.back().InCtorInitializer = true;
+} else if (Current.Previous &&
+   Current.Previous->is(TT_InheritanceColon)) {
+  Contexts.back().IsExpression = true;
+  Contexts.back().InInhertiance = true;
 } else if (Current.isOneOf(tok::r_paren, tok::greater, tok::comma)) {
   for (FormatToken *Previous = Current.Previous;
Previous && Previous->isOneOf(tok::star, tok::amp);
@@ -2388,6 +2395,15 @@
  !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral);
 }
 
+// Determine if the next token from the closing scope is an inheritance token
+static bool hasMultipleInheritance(const FormatToken &Tok) {
+  // Look for a column of type TT_InheritanceComma untill we find it or
+  // we find a declaration termination
+  for (const FormatToken* nextTok = Tok.Next; nextTok; nextTok = nextTok->Next)
+if (nextTok->is(TT_InheritanceComma)) return true;
+  return false;
+}
+  
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
  const FormatToken &Right) {
   const FormatToken &Left = *Right.Previous;
@@ -2471,6 +2487,11 @@
   Style.BreakConstructorInitializersBeforeComma &&
   !Style.ConstructorInitializerAllOnOneLineOrOnePerLine)
 return true;
+  //Break only if we have multiple inheritance
+  if (Style.BreakBeforeInhertianceDelimiter &&
+  ((Right.is(TT_InheritanceColon) && hasMultipleInheritance(Right)) ||
+  Right.is(TT_InheritanceComma)))
+return true;
   if (Right.is(tok::string_literal) && Right.TokenText.startswith("R\""))
 // Raw string literals are special wrt. line breaks. The author has made a
 // deliberate choice and might have aligned the contents of the string
@@ -2648,6 +2669,12 @@
   if (Right.is(TT_CtorInitializerComma) &&
   Style.BreakConstructorInitializersBeforeComma)
 return true;
+  if (Left.is(TT_Inheritan

[PATCH] D30498: [clangd] Add support for FixIts.

[Benjamin Kramer via Phabricator via cfe-commits]

bkramer created this revision.

This uses CodeActions to show 'apply fix' actions when code actions are
requested for a location. The actions themselves make use of a
clangd.applyFix command which has to be implemented on the editor side. I
included an implementation for vscode.

This also adds a -run-synchronously flag which runs everything on the main
thread. This is useful for testing.


https://reviews.llvm.org/D30498

Files:
  clangd/ASTManager.cpp
  clangd/ASTManager.h
  clangd/ClangDMain.cpp
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h
  clangd/clients/clangd-vscode/src/extension.ts
  test/clangd/fixits.test

Index: test/clangd/fixits.test
===
--- /dev/null
+++ test/clangd/fixits.test
@@ -0,0 +1,22 @@
+# RUN: clangd -run-synchronously < %s | FileCheck %s
+# It is absolutely vital that this file has CRLF line endings.
+#
+Content-Length: 125
+
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+#
+Content-Length: 180
+
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///foo.c","languageId":"c","version":1,"text":"int main(int i, char **a) { if (i = 2) {}}"}}}
+#
+# CHECK-DAG: {"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"uri":"file:///foo.c","diagnostics":[{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":2,"message":"using the result of an assignment as a condition without parentheses"},{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":3,"message":"place parentheses around the assignment to silence this warning"},{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":3,"message":"use '==' to turn this assignment into an equality comparison"}]}}}
+#
+Content-Length: 746
+
+ {"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"file:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":2,"message":"using the result of an assignment as a condition without parentheses"},{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":3,"message":"place parentheses around the assignment to silence this warning"},{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":3,"message":"use '==' to turn this assignment into an equality comparison"}]}}}
+#
+# CHECK: {"jsonrpc":"2.0","id":2, "result": [{"title":"Apply FixIt 'place parentheses around the assignment to silence this warning'", "command": "clangd.applyFix", "arguments": ["file:///foo.c", [{"range": {"start": {"line": 0, "character": 32}, "end": {"line": 0, "character": 32}}, "newText": "("},{"range": {"start": {"line": 0, "character": 37}, "end": {"line": 0, "character": 37}}, "newText": ")"}]]},{"title":"Apply FixIt 'use '==' to turn this assignment into an equality comparison'", "command": "clangd.applyFix", "arguments": ["file:///foo.c", [{"range": {"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 35}}, "newText": "=="}]]}]
+#
+Content-Length: 44
+
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
Index: clangd/clients/clangd-vscode/src/extension.ts
===
--- clangd/clients/clangd-vscode/src/extension.ts
+++ clangd/clients/clangd-vscode/src/extension.ts
@@ -18,9 +18,25 @@
 
 const clangdClient = new vscodelc.LanguageClient('Clang Language Server', serverOptions, clientOptions);
 
+function applyTextEdits(uri: string, edits: vscodelc.TextEdit[]) {
+let textEditor = vscode.window.activeTextEditor;
+
+if (textEditor && textEditor.document.uri.toString() === uri) {
+textEditor.edit(mutator => {
+for (let edit of edits) {
+mutator.replace(vscodelc.Protocol2Code.asRange(edit.range), edit.newText);
+}
+}).then((success) => {
+if (!success) {
+vscode.window.showErrorMessage('Failed to apply fixes to the document.');
+}
+});
+}
+}
+
 console.log('Clang Language Server is now active!');
 
 const disposable = clangdClient.start();
 
-context.subscriptions.push(disposable);
+context.subscriptions.push(disposable, vscode.commands.registerCommand('clangd.applyFix', applyTextEdits));
 }
\ No newline at end of file
Index: clangd/ProtocolHandlers.h
===
--- clangd/ProtocolHandlers.h
+++ clangd/ProtocolHandlers.h
@@ -22,6 +22,7 @@
 
 namespace clang {
 namespace clangd {
+class ASTMana

[PATCH] D30476: [clangd] Add a toy Eclipse integration for development purposes

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle-ericsson added a comment.

To be honest, I don't think this needs to be merged but it makes sense to put 
it somewhere people can try it.


Repository:
  rL LLVM

https://reviews.llvm.org/D30476



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

[clang-tools-extra] r296618 - [clangd] Add a toy VS Code integration for development purposes

[Krasimir Georgiev via cfe-commits]

Author: krasimir
Date: Wed Mar  1 07:53:12 2017
New Revision: 296618

URL: http://llvm.org/viewvc/llvm-project?rev=296618&view=rev
Log:
[clangd] Add a toy VS Code integration for development purposes

Summary: This patch adds bare-bone VS Code integration for development purposes 
of clangd.

Reviewers: klimek, bkramer, mprobst

Reviewed By: bkramer

Subscribers: mprobst

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

Added:
clang-tools-extra/trunk/clangd/clients/
clang-tools-extra/trunk/clangd/clients/clangd-vscode/
clang-tools-extra/trunk/clangd/clients/clangd-vscode/.gitignore
clang-tools-extra/trunk/clangd/clients/clangd-vscode/.vscode/
clang-tools-extra/trunk/clangd/clients/clangd-vscode/.vscode/launch.json
clang-tools-extra/trunk/clangd/clients/clangd-vscode/.vscode/settings.json
clang-tools-extra/trunk/clangd/clients/clangd-vscode/.vscode/tasks.json
clang-tools-extra/trunk/clangd/clients/clangd-vscode/.vscodeignore
clang-tools-extra/trunk/clangd/clients/clangd-vscode/README.txt
clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/
clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts
clang-tools-extra/trunk/clangd/clients/clangd-vscode/test/
clang-tools-extra/trunk/clangd/clients/clangd-vscode/test/extension.test.ts
clang-tools-extra/trunk/clangd/clients/clangd-vscode/test/index.ts
clang-tools-extra/trunk/clangd/clients/clangd-vscode/tsconfig.json

clang-tools-extra/trunk/clangd/clients/clangd-vscode/vsc-extension-quickstart.md

Added: clang-tools-extra/trunk/clangd/clients/clangd-vscode/.gitignore
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/.gitignore?rev=296618&view=auto
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/.gitignore (added)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/.gitignore Wed Mar  1 
07:53:12 2017
@@ -0,0 +1,2 @@
+out
+node_modules
\ No newline at end of file

Added: clang-tools-extra/trunk/clangd/clients/clangd-vscode/.vscode/launch.json
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/.vscode/launch.json?rev=296618&view=auto
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/.vscode/launch.json 
(added)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/.vscode/launch.json 
Wed Mar  1 07:53:12 2017
@@ -0,0 +1,28 @@
+// A launch configuration that compiles the extension and then opens it inside 
a new window
+{
+"version": "0.1.0",
+"configurations": [
+{
+"name": "Launch Extension",
+"type": "extensionHost",
+"request": "launch",
+"runtimeExecutable": "${execPath}",
+"args": ["--extensionDevelopmentPath=${workspaceRoot}" ],
+"stopOnEntry": false,
+"sourceMaps": true,
+"outFiles": [ "${workspaceRoot}/out/src/**/*.js" ],
+"preLaunchTask": "npm"
+},
+{
+"name": "Launch Tests",
+"type": "extensionHost",
+"request": "launch",
+"runtimeExecutable": "${execPath}",
+"args": ["--extensionDevelopmentPath=${workspaceRoot}", 
"--extensionTestsPath=${workspaceRoot}/out/test" ],
+"stopOnEntry": false,
+"sourceMaps": true,
+"outFiles": [ "${workspaceRoot}/out/test/**/*.js" ],
+"preLaunchTask": "npm"
+}
+]
+}

Added: 
clang-tools-extra/trunk/clangd/clients/clangd-vscode/.vscode/settings.json
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/.vscode/settings.json?rev=296618&view=auto
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/.vscode/settings.json 
(added)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/.vscode/settings.json 
Wed Mar  1 07:53:12 2017
@@ -0,0 +1,9 @@
+// Place your settings in this file to overwrite default and user settings.
+{
+"files.exclude": {
+"out": false // set this to true to hide the "out" folder with the 
compiled JS files
+},
+"search.exclude": {
+"out": true // set this to false to include "out" folder in search 
results
+}
+}
\ No newline at end of file

Added: clang-tools-extra/trunk/clangd/clients/clangd-vscode/.vscode/tasks.json
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/.vscode/tasks.json?rev=296618&view=auto
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/.vscode/tasks.json 
(added)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/.vscode/t

[PATCH] D30405: [clang-format] Add a new flag FixNamespaceEndComments to FormatStyle

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir updated this revision to Diff 90172.
krasimir marked 2 inline comments as done.
krasimir added a comment.

- Address review comments


https://reviews.llvm.org/D30405

Files:
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Index/CommentToXML.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -185,7 +185,7 @@
   EXPECT_EQ("namespace N {\n"
 "\n"
 "int i;\n"
-"}",
+"}  // namespace N",
 format("namespace N {\n"
"\n"
"inti;\n"
@@ -270,12 +270,22 @@
"}"));
 
   // FIXME: This is slightly inconsistent.
+  FormatStyle LLVMWithNoNamespaceFix = getLLVMStyle();
+  LLVMWithNoNamespaceFix.FixNamespaceComments = false;
   EXPECT_EQ("namespace {\n"
 "int i;\n"
 "}",
 format("namespace {\n"
"int i;\n"
"\n"
+   "}", LLVMWithNoNamespaceFix));
+  EXPECT_EQ("namespace {\n"
+"int i;\n"
+"\n"
+"} // namespace",
+format("namespace {\n"
+   "int i;\n"
+   "\n"
"}"));
   EXPECT_EQ("namespace {\n"
 "int i;\n"
@@ -1197,33 +1207,43 @@
 }
 
 TEST_F(FormatTest, FormatsNamespaces) {
+  FormatStyle LLVMWithNoNamespaceFix = getLLVMStyle();
+  LLVMWithNoNamespaceFix.FixNamespaceComments = false;
+
   verifyFormat("namespace some_namespace {\n"
"class A {};\n"
"void f() { f(); }\n"
-   "}");
+   "}",
+   LLVMWithNoNamespaceFix);
   verifyFormat("namespace {\n"
"class A {};\n"
"void f() { f(); }\n"
-   "}");
+   "}",
+   LLVMWithNoNamespaceFix);
   verifyFormat("inline namespace X {\n"
"class A {};\n"
"void f() { f(); }\n"
-   "}");
+   "}",
+   LLVMWithNoNamespaceFix);
   verifyFormat("using namespace some_namespace;\n"
"class A {};\n"
-   "void f() { f(); }");
+   "void f() { f(); }",
+   LLVMWithNoNamespaceFix);
 
   // This code is more common than we thought; if we
   // layout this correctly the semicolon will go into
   // its own line, which is undesirable.
-  verifyFormat("namespace {};");
+  verifyFormat("namespace {};",
+   LLVMWithNoNamespaceFix);
   verifyFormat("namespace {\n"
"class A {};\n"
-   "};");
+   "};",
+   LLVMWithNoNamespaceFix);
 
   verifyFormat("namespace {\n"
"int SomeVariable = 0; // comment\n"
-   "} // namespace");
+   "} // namespace",
+   LLVMWithNoNamespaceFix);
   EXPECT_EQ("#ifndef HEADER_GUARD\n"
 "#define HEADER_GUARD\n"
 "namespace my_namespace {\n"
@@ -1235,44 +1255,46 @@
"   namespace my_namespace {\n"
"int i;\n"
"}// my_namespace\n"
-   "#endif// HEADER_GUARD"));
+   "#endif// HEADER_GUARD",
+   LLVMWithNoNamespaceFix));
 
   EXPECT_EQ("namespace A::B {\n"
 "class C {};\n"
 "}",
 format("namespace A::B {\n"
"class C {};\n"
-   "}"));
+   "}",
+   LLVMWithNoNamespaceFix));
 
   FormatStyle Style = getLLVMStyle();
   Style.NamespaceIndentation = FormatStyle::NI_All;
   EXPECT_EQ("namespace out {\n"
 "  int i;\n"
 "  namespace in {\n"
 "int i;\n"
-"  } // namespace\n"
-"} // namespace",
+"  } // namespace in\n"
+"} // namespace out",
 format("namespace out {\n"
"int i;\n"
"namespace in {\n"
"int i;\n"
-   "} // namespace\n"
-   "} // namespace",
+   "} // namespace in\n"
+   "} // namespace out",
Style));
 
   Style.NamespaceIndentation = FormatStyle::NI_Inner;
   EXPECT_EQ("namespace out {\n"
 "int i;\n"
 "namespace in {\n"
 "  int i;\n"
-"} // namespace\n"
-"} // namespace",
+"} // namespace in\n"
+"} // namespace out",
 format("namespace out {\n"
"int i;\n"
"namespace in {\n"
"int i;\n"
-   "} // namespace\n"
-   "} // namespace",
+   "} // namespace in\n"
+   "} // namespace out",
  

[PATCH] D30489: [analyzer] catch out of bounds for VLA

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

There is an alternative approach idea:

This is not found by ArrayBoundCheckerV2? If no, an alternative approach would 
be to properly set the constraints on the extent of the VLA's memory region. 
After that, maybe ArrayBoundCheckerV2 would work automatically on this case. If 
the extent size is not set for VLAs properly yet, it should be similar to this 
patch: https://reviews.llvm.org/D24307

@zaks.anna, @dcoughlin WDYT?


Repository:
  rL LLVM

https://reviews.llvm.org/D30489



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

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

[Andi via Phabricator via cfe-commits]

Abpostelnicu updated this revision to Diff 90171.
Abpostelnicu added a comment.

Also added tests.


Repository:
  rL LLVM

https://reviews.llvm.org/D30487

Files:
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1011,6 +1011,29 @@
   verifyFormat("class ::A::B {};");
 }
 
+TEST_F(FormatTest, BreakBeforeInhertianceDelimiter) {
+  FormatStyle StyleWithInheritanceBreak = getLLVMStyle();
+  LocalStyle.BreakBeforeInhertianceDelimiter = true;
+
+  verifyFormat("class MyClass : public X {}", LocalStyle);
+  verifyFormat("class MyClass\n"
+   ": public X\n"
+   ", public Y // some difficult comment\n"
+   ", public Z {}",
+   StyleWithInheritanceBreak);
+  verifyFormat("class MyClass"
+   "  : public X\n"
+   "  , public Y // some difficult comment\n"
+   "  , public Z\n",
+   StyleWithInheritanceBreak);
+  verifyFormat("class MyClass\n"
+   ": public X // When deriving from more than one class, put "
+   "each on its own\n"
+   "   // line.\n"
+   ", public Y {}",
+   StyleWithInheritanceBreak);
+}
+
 TEST_F(FormatTest, FormatsVariableDeclarationsAfterStructOrClass) {
   verifyFormat("class A {\n} a, b;");
   verifyFormat("struct A {\n} a, b;");
@@ -8459,6 +8482,7 @@
   CHECK_PARSE_BOOL(BreakBeforeTernaryOperators);
   CHECK_PARSE_BOOL(BreakConstructorInitializersBeforeComma);
   CHECK_PARSE_BOOL(BreakStringLiterals);
+  CHECK_PARSE_BOOL(BreakBeforeInhertianceDelimiter)
   CHECK_PARSE_BOOL(ConstructorInitializerAllOnOneLineOrOnePerLine);
   CHECK_PARSE_BOOL(DerivePointerAlignment);
   CHECK_PARSE_BOOL_FIELD(DerivePointerAlignment, "DerivePointerBinding");
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -676,6 +676,8 @@
 case tok::comma:
   if (Contexts.back().InCtorInitializer)
 Tok->Type = TT_CtorInitializerComma;
+  else if (Contexts.back().InInhertiance)
+Tok->Type = TT_InheritanceComma;
   else if (Contexts.back().FirstStartOfName &&
(Contexts.size() == 1 || Line.startsWith(tok::kw_for))) {
 Contexts.back().FirstStartOfName->PartOfMultiVariableDeclStmt = true;
@@ -945,6 +947,7 @@
 bool CanBeExpression = true;
 bool InTemplateArgument = false;
 bool InCtorInitializer = false;
+bool InInhertiance = false;
 bool CaretFound = false;
 bool IsForEachMacro = false;
   };
@@ -1004,6 +1007,10 @@
Current.Previous->is(TT_CtorInitializerColon)) {
   Contexts.back().IsExpression = true;
   Contexts.back().InCtorInitializer = true;
+} else if (Current.Previous &&
+   Current.Previous->is(TT_InheritanceColon)) {
+  Contexts.back().IsExpression = true;
+  Contexts.back().InInhertiance = true;
 } else if (Current.isOneOf(tok::r_paren, tok::greater, tok::comma)) {
   for (FormatToken *Previous = Current.Previous;
Previous && Previous->isOneOf(tok::star, tok::amp);
@@ -2377,6 +2384,15 @@
  !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral);
 }
 
+// Determine if the next token from the closing scope is an inheritance token
+static bool hasMultipleInheritance(const FormatToken &Tok) {
+  // Look for a column of type TT_InheritanceComma untill we find it or
+  // we find a declaration termination
+  for (const FormatToken* nextTok = Tok.Next; nextTok; nextTok = nextTok->Next)
+if (nextTok->is(TT_InheritanceComma)) return true;
+  return false;
+}
+  
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
  const FormatToken &Right) {
   const FormatToken &Left = *Right.Previous;
@@ -2460,6 +2476,11 @@
   Style.BreakConstructorInitializersBeforeComma &&
   !Style.ConstructorInitializerAllOnOneLineOrOnePerLine)
 return true;
+  //Break only if we have multiple inheritance
+  if (Style.BreakBeforeInhertianceDelimiter &&
+  ((Right.is(TT_InheritanceColon) && hasMultipleInheritance(Right)) ||
+  Right.is(TT_InheritanceComma)))
+return true;
   if (Right.is(tok::string_literal) && Right.TokenText.startswith("R\""))
 // Raw string literals are special wrt. line breaks. The author has made a
 // deliberate choice and might have aligned the contents of the string
@@ -2634,6 +2655,12 @@
   if (Right.is(TT_CtorInitializerComma) &&
   Style.BreakConstructorInitializersBeforeComma)
 return true;
+  if (Left.is(TT_InheritanceComma) &&
+  S

[PATCH] D27054: Introducing clang::tooling::AtomicChange for refactoring tools.

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL296616: Introducing clang::tooling::AtomicChange for 
refactoring tools. (authored by ioeric).

Changed prior to commit:
  https://reviews.llvm.org/D27054?vs=90018&id=90165#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D27054

Files:
  cfe/trunk/include/clang/Tooling/Refactoring/AtomicChange.h
  cfe/trunk/lib/Tooling/CMakeLists.txt
  cfe/trunk/lib/Tooling/Refactoring/AtomicChange.cpp
  cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt
  cfe/trunk/unittests/Tooling/CMakeLists.txt
  cfe/trunk/unittests/Tooling/RefactoringTest.cpp

Index: cfe/trunk/include/clang/Tooling/Refactoring/AtomicChange.h
===
--- cfe/trunk/include/clang/Tooling/Refactoring/AtomicChange.h
+++ cfe/trunk/include/clang/Tooling/Refactoring/AtomicChange.h
@@ -0,0 +1,129 @@
+//===--- AtomicChange.h - AtomicChange class *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+//  This file defines AtomicChange which is used to create a set of source
+//  changes, e.g. replacements and header insertions.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLING_REFACTOR_ATOMICCHANGE_H
+#define LLVM_CLANG_TOOLING_REFACTOR_ATOMICCHANGE_H
+
+#include "clang/Basic/SourceManager.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+
+namespace clang {
+namespace tooling {
+
+/// \brief An atomic change is used to create and group a set of source edits,
+/// e.g. replacements or header insertions. Edits in an AtomicChange should be
+/// related, e.g. replacements for the same type reference and the corresponding
+/// header insertion/deletion.
+///
+/// An AtomicChange is uniquely identified by a key and will either be fully
+/// applied or not applied at all.
+///
+/// Calling setError on an AtomicChange stores the error message and marks it as
+/// bad, i.e. none of its source edits will be applied.
+class AtomicChange {
+public:
+  /// \brief Creates an atomic change around \p KeyPosition with the key being a
+  /// concatenation of the file name and the offset of \p KeyPosition.
+  /// \p KeyPosition should be the location of the key syntactical element that
+  /// is being changed, e.g. the call to a refactored method.
+  AtomicChange(const SourceManager &SM, SourceLocation KeyPosition);
+
+  /// \brief Creates an atomic change for \p FilePath with a customized key.
+  AtomicChange(llvm::StringRef FilePath, llvm::StringRef Key)
+  : Key(Key), FilePath(FilePath) {}
+
+  /// \brief Returns the atomic change as a YAML string.
+  std::string toYAMLString();
+
+  /// \brief Converts a YAML-encoded automic change to AtomicChange.
+  static AtomicChange convertFromYAML(llvm::StringRef YAMLContent);
+
+  /// \brief Returns the key of this change, which is a concatenation of the
+  /// file name and offset of the key position.
+  const std::string &getKey() const { return Key; }
+
+  /// \brief Returns the path of the file containing this atomic change.
+  const std::string &getFilePath() const { return FilePath; }
+
+  /// \brief If this change could not be created successfully, e.g. because of
+  /// conflicts among replacements, use this to set an error description.
+  /// Thereby, places that cannot be fixed automatically can be gathered when
+  /// applying changes.
+  void setError(llvm::StringRef Error) { this->Error = Error; }
+
+  /// \brief Returns whether an error has been set on this list.
+  bool hasError() const { return !Error.empty(); }
+
+  /// \brief Returns the error message or an empty string if it does not exist.
+  const std::string &getError() const { return Error; }
+
+  /// \brief Adds a replacement that replaces range [Loc, Loc+Length) with
+  /// \p Text.
+  /// \returns An llvm::Error carrying ReplacementError on error.
+  llvm::Error replace(const SourceManager &SM, SourceLocation Loc,
+  unsigned Length, llvm::StringRef Text);
+
+  /// \brief Adds a replacement that inserts \p Text at \p Loc. If this
+  /// insertion conflicts with an existing insertion (at the same position),
+  /// this will be inserted before/after the existing insertion depending on
+  /// \p InsertAfter. Users should use `replace` with `Length=0` instead if they
+  /// do not want conflict resolving by default. If the conflicting replacement
+  /// is not an insertion, an error is returned.
+  ///
+  /// \returns An llvm::Error carrying ReplacementError on error.
+  llvm::Error insert(const SourceManager &SM, SourceLocation Loc,
+ llvm::StringRef Text, bool InsertAfter = true);
+
+  ///

r296616 - Introducing clang::tooling::AtomicChange for refactoring tools.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Wed Mar  1 07:14:01 2017
New Revision: 296616

URL: http://llvm.org/viewvc/llvm-project?rev=296616&view=rev
Log:
Introducing clang::tooling::AtomicChange for refactoring tools.

Summary:
An AtomicChange is used to create and group a set of source edits, e.g.
replacements or header insertions. Edits in an AtomicChange should be related,
e.g. replacements for the same type reference and the corresponding header
insertion/deletion.

An AtomicChange is uniquely identified by a key position and will either be
fully applied or not applied at all. The key position should be the location
of the key syntactical element that is being changed, e.g. the call to a
refactored method.

Next step: add a tool that applies AtomicChange.

Reviewers: klimek, djasper

Reviewed By: klimek

Subscribers: alexshap, cfe-commits, djasper, mgorny

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

Added:
cfe/trunk/include/clang/Tooling/Refactoring/
cfe/trunk/include/clang/Tooling/Refactoring/AtomicChange.h
cfe/trunk/lib/Tooling/Refactoring/
cfe/trunk/lib/Tooling/Refactoring/AtomicChange.cpp
cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt
Modified:
cfe/trunk/lib/Tooling/CMakeLists.txt
cfe/trunk/unittests/Tooling/CMakeLists.txt
cfe/trunk/unittests/Tooling/RefactoringTest.cpp

Added: cfe/trunk/include/clang/Tooling/Refactoring/AtomicChange.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/AtomicChange.h?rev=296616&view=auto
==
--- cfe/trunk/include/clang/Tooling/Refactoring/AtomicChange.h (added)
+++ cfe/trunk/include/clang/Tooling/Refactoring/AtomicChange.h Wed Mar  1 
07:14:01 2017
@@ -0,0 +1,129 @@
+//===--- AtomicChange.h - AtomicChange class *- C++ 
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+//  This file defines AtomicChange which is used to create a set of source
+//  changes, e.g. replacements and header insertions.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLING_REFACTOR_ATOMICCHANGE_H
+#define LLVM_CLANG_TOOLING_REFACTOR_ATOMICCHANGE_H
+
+#include "clang/Basic/SourceManager.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+
+namespace clang {
+namespace tooling {
+
+/// \brief An atomic change is used to create and group a set of source edits,
+/// e.g. replacements or header insertions. Edits in an AtomicChange should be
+/// related, e.g. replacements for the same type reference and the 
corresponding
+/// header insertion/deletion.
+///
+/// An AtomicChange is uniquely identified by a key and will either be fully
+/// applied or not applied at all.
+///
+/// Calling setError on an AtomicChange stores the error message and marks it 
as
+/// bad, i.e. none of its source edits will be applied.
+class AtomicChange {
+public:
+  /// \brief Creates an atomic change around \p KeyPosition with the key being 
a
+  /// concatenation of the file name and the offset of \p KeyPosition.
+  /// \p KeyPosition should be the location of the key syntactical element that
+  /// is being changed, e.g. the call to a refactored method.
+  AtomicChange(const SourceManager &SM, SourceLocation KeyPosition);
+
+  /// \brief Creates an atomic change for \p FilePath with a customized key.
+  AtomicChange(llvm::StringRef FilePath, llvm::StringRef Key)
+  : Key(Key), FilePath(FilePath) {}
+
+  /// \brief Returns the atomic change as a YAML string.
+  std::string toYAMLString();
+
+  /// \brief Converts a YAML-encoded automic change to AtomicChange.
+  static AtomicChange convertFromYAML(llvm::StringRef YAMLContent);
+
+  /// \brief Returns the key of this change, which is a concatenation of the
+  /// file name and offset of the key position.
+  const std::string &getKey() const { return Key; }
+
+  /// \brief Returns the path of the file containing this atomic change.
+  const std::string &getFilePath() const { return FilePath; }
+
+  /// \brief If this change could not be created successfully, e.g. because of
+  /// conflicts among replacements, use this to set an error description.
+  /// Thereby, places that cannot be fixed automatically can be gathered when
+  /// applying changes.
+  void setError(llvm::StringRef Error) { this->Error = Error; }
+
+  /// \brief Returns whether an error has been set on this list.
+  bool hasError() const { return !Error.empty(); }
+
+  /// \brief Returns the error message or an empty string if it does not exist.
+  const std::string &getError() const { return Error; }
+
+  /// \brief Adds a replacement that replaces range [Loc, Loc+Length) with
+  /// \p Text