[libcxx] r291961 - Add _LIBCPP_DIAGNOSE_WARNING and _LIBCPP_DIAGNOSE_ERROR macros.
Nico Weber via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 23 08:36:50 PST 2017
This happens to fire in practice in protobuf. It's probably a true positive
and it's cool that this warning found it, but it means we have to disable
Wuser-defined-warnings for a bit -- which then disables all of these
user-defined warnings. Right now there aren't any others, but it feels like
we'd want to have the ability to turn individual user-defined warnings on
or off instead of just having a single toggle for all of them. Are there
plans for something like that?
On Fri, Jan 13, 2017 at 5:02 PM, Eric Fiselier via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
> Author: ericwf
> Date: Fri Jan 13 16:02:08 2017
> New Revision: 291961
>
> URL: http://llvm.org/viewvc/llvm-project?rev=291961&view=rev
> Log:
> Add _LIBCPP_DIAGNOSE_WARNING and _LIBCPP_DIAGNOSE_ERROR macros.
>
> Clang recently added a `diagnose_if(cond, msg, type)` attribute
> which can be used to generate diagnostics when `cond` is a constant
> expression that evaluates to true. Otherwise no attribute has no
> effect.
>
> This patch adds _LIBCPP_DIAGNOSE_ERROR/WARNING macros which
> use this new attribute. Additionally this patch implements
> a diagnostic message when a non-const-callable comparator is
> given to a container.
>
> Note: For now the warning version of the diagnostic is useless
> within libc++ since warning diagnostics are suppressed by the
> system header pragma. I'm going to work on fixing this.
>
> Added:
> libcxx/trunk/test/libcxx/containers/associative/non_
> const_comparator.fail.cpp
> Modified:
> libcxx/trunk/docs/UsingLibcxx.rst
> libcxx/trunk/include/__config
> libcxx/trunk/include/__tree
> libcxx/trunk/include/map
> libcxx/trunk/include/type_traits
> libcxx/trunk/test/libcxx/compiler.py
> libcxx/trunk/test/libcxx/test/config.py
> libcxx/trunk/test/libcxx/test/format.py
> libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/
> diagnose_reference_binding.fail.cpp
>
> Modified: libcxx/trunk/docs/UsingLibcxx.rst
> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/docs/
> UsingLibcxx.rst?rev=291961&r1=291960&r2=291961&view=diff
> ============================================================
> ==================
> --- libcxx/trunk/docs/UsingLibcxx.rst (original)
> +++ libcxx/trunk/docs/UsingLibcxx.rst Fri Jan 13 16:02:08 2017
> @@ -173,3 +173,10 @@ thread safety annotations.
> return Tup{"hello world", 42}; // explicit constructor called. OK.
> }
>
> +**_LIBCPP_DISABLE_ADDITIONAL_DIAGNOSTICS**:
> + This macro disables the additional diagnostics generated by libc++
> using the
> + `diagnose_if` attribute. These additional diagnostics include checks
> for:
> +
> + * Giving `set`, `map`, `multiset`, `multimap` a comparator which is
> not
> + const callable.
> +
>
> Modified: libcxx/trunk/include/__config
> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/_
> _config?rev=291961&r1=291960&r2=291961&view=diff
> ============================================================
> ==================
> --- libcxx/trunk/include/__config (original)
> +++ libcxx/trunk/include/__config Fri Jan 13 16:02:08 2017
> @@ -1006,6 +1006,16 @@ _LIBCPP_FUNC_VIS extern "C" void __sanit
> #endif
> #endif
>
> +#if __has_attribute(diagnose_if) && !defined(_LIBCPP_DISABLE_
> ADDITIONAL_DIAGNOSTICS)
> +# define _LIBCPP_DIAGNOSE_WARNING(...) \
> + __attribute__((__diagnose_if__(__VA_ARGS__, "warning")))
> +# define _LIBCPP_DIAGNOSE_ERROR(...) \
> + __attribute__((__diagnose_if__(__VA_ARGS__, "error")))
> +#else
> +# define _LIBCPP_DIAGNOSE_WARNING(...)
> +# define _LIBCPP_DIAGNOSE_ERROR(...)
> +#endif
> +
> #endif // __cplusplus
>
> #endif // _LIBCPP_CONFIG
>
> Modified: libcxx/trunk/include/__tree
> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/_
> _tree?rev=291961&r1=291960&r2=291961&view=diff
> ============================================================
> ==================
> --- libcxx/trunk/include/__tree (original)
> +++ libcxx/trunk/include/__tree Fri Jan 13 16:02:08 2017
> @@ -41,6 +41,10 @@ template <class _Key, class _Value>
> struct __value_type;
> #endif
>
> +template <class _Key, class _CP, class _Compare,
> + bool = is_empty<_Compare>::value && !__libcpp_is_final<_Compare>::
> value>
> +class __map_value_compare;
> +
> template <class _Allocator> class __map_node_destructor;
> template <class _TreeIterator> class _LIBCPP_TEMPLATE_VIS __map_iterator;
> template <class _TreeIterator> class _LIBCPP_TEMPLATE_VIS
> __map_const_iterator;
> @@ -955,6 +959,30 @@ private:
>
> };
>
> +#ifndef _LIBCPP_CXX03_LANG
> +template <class _Tp, class _Compare, class _Allocator>
> +struct __diagnose_tree_helper {
> + static constexpr bool __trigger_diagnostics()
> + _LIBCPP_DIAGNOSE_WARNING(!__is_const_comparable<_Compare,
> _Tp>::value,
> + "the specified comparator type does not provide a const call
> operator")
> + { return true; }
> +};
> +
> +template <class _Key, class _Value, class _KeyComp, class _Alloc>
> +struct __diagnose_tree_helper<
> + __value_type<_Key, _Value>,
> + __map_value_compare<_Key, __value_type<_Key, _Value>, _KeyComp>,
> + _Alloc
> +>
> +{
> + static constexpr bool __trigger_diagnostics()
> + _LIBCPP_DIAGNOSE_WARNING(!__is_const_comparable<_KeyComp,
> _Key>::value,
> + "the specified comparator type does not provide a const call
> operator")
> + { return true; }
> +};
> +
> +#endif
> +
> template <class _Tp, class _Compare, class _Allocator>
> class __tree
> {
> @@ -1787,7 +1815,11 @@ __tree<_Tp, _Compare, _Allocator>::~__tr
> {
> static_assert((is_copy_constructible<value_compare>::value),
> "Comparator must be copy-constructible.");
> - destroy(__root());
> +#ifndef _LIBCPP_CXX03_LANG
> + static_assert((__diagnose_tree_helper<_Tp, _Compare, _Allocator>::
> + __trigger_diagnostics()), "");
> +#endif
> + destroy(__root());
> }
>
> template <class _Tp, class _Compare, class _Allocator>
>
> Modified: libcxx/trunk/include/map
> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/
> map?rev=291961&r1=291960&r2=291961&view=diff
> ============================================================
> ==================
> --- libcxx/trunk/include/map (original)
> +++ libcxx/trunk/include/map Fri Jan 13 16:02:08 2017
> @@ -453,9 +453,7 @@ swap(multimap<Key, T, Compare, Allocator
>
> _LIBCPP_BEGIN_NAMESPACE_STD
>
> -template <class _Key, class _CP, class _Compare,
> - bool = is_empty<_Compare>::value &&
> !__libcpp_is_final<_Compare>::value
> - >
> +template <class _Key, class _CP, class _Compare, bool _IsSmall>
> class __map_value_compare
> : private _Compare
> {
>
> Modified: libcxx/trunk/include/type_traits
> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/
> type_traits?rev=291961&r1=291960&r2=291961&view=diff
> ============================================================
> ==================
> --- libcxx/trunk/include/type_traits (original)
> +++ libcxx/trunk/include/type_traits Fri Jan 13 16:02:08 2017
> @@ -4715,6 +4715,15 @@ struct __can_extract_map_key<_ValTy, _Ke
>
> #endif
>
> +template <class _Comp, class _ValueType, class = void>
> +struct __is_const_comparable : false_type {};
> +
> +template <class _Comp, class _ValueType>
> +struct __is_const_comparable<_Comp, _ValueType, typename __void_t<
> + decltype(_VSTD::declval<_Comp const&>()(_VSTD::declval<_ValueType
> const&>(),
> + _VSTD::declval<_ValueType
> const&>()))
> + >::type> : true_type {};
> +
> _LIBCPP_END_NAMESPACE_STD
>
> #endif // _LIBCPP_TYPE_TRAITS
>
> Modified: libcxx/trunk/test/libcxx/compiler.py
> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/
> libcxx/compiler.py?rev=291961&r1=291960&r2=291961&view=diff
> ============================================================
> ==================
> --- libcxx/trunk/test/libcxx/compiler.py (original)
> +++ libcxx/trunk/test/libcxx/compiler.py Fri Jan 13 16:02:08 2017
> @@ -296,7 +296,7 @@ class CXXCompiler(object):
>
> def addWarningFlagIfSupported(self, flag):
> if self.hasWarningFlag(flag):
> - assert flag not in self.warning_flags
> - self.warning_flags += [flag]
> + if flag not in self.warning_flags:
> + self.warning_flags += [flag]
> return True
> return False
>
> Added: libcxx/trunk/test/libcxx/containers/associative/non_
> const_comparator.fail.cpp
> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/
> libcxx/containers/associative/non_const_comparator.fail.cpp?
> rev=291961&view=auto
> ============================================================
> ==================
> --- libcxx/trunk/test/libcxx/containers/associative/non_const_comparator.fail.cpp
> (added)
> +++ libcxx/trunk/test/libcxx/containers/associative/non_const_comparator.fail.cpp
> Fri Jan 13 16:02:08 2017
> @@ -0,0 +1,45 @@
> +//===------------------------------------------------------
> ----------------===//
> +//
> +// 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.
> +//
> +//===------------------------------------------------------
> ----------------===//
> +
> +// UNSUPPORTED: c++98, c++03
> +// REQUIRES: diagnose-if-support, verify-support
> +
> +// Test that libc++ generates a warning diagnostic when the container is
> +// provided a non-const callable comparator.
> +
> +#include <set>
> +#include <map>
> +
> +struct BadCompare {
> + template <class T, class U>
> + bool operator()(T const& t, U const& u) {
> + return t < u;
> + }
> +};
> +
> +int main() {
> + static_assert(!std::__is_const_comparable<BadCompare, int>::value, "");
> + // expected-warning at __tree:* 4 {{the specified comparator type does
> not provide a const call operator}}
> + {
> + using C = std::set<int, BadCompare>;
> + C s;
> + }
> + {
> + using C = std::multiset<long, BadCompare>;
> + C s;
> + }
> + {
> + using C = std::map<int, int, BadCompare>;
> + C s;
> + }
> + {
> + using C = std::multimap<long, int, BadCompare>;
> + C s;
> + }
> +}
>
> Modified: libcxx/trunk/test/libcxx/test/config.py
> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/
> libcxx/test/config.py?rev=291961&r1=291960&r2=291961&view=diff
> ============================================================
> ==================
> --- libcxx/trunk/test/libcxx/test/config.py (original)
> +++ libcxx/trunk/test/libcxx/test/config.py Fri Jan 13 16:02:08 2017
> @@ -712,33 +712,35 @@ class Configuration(object):
> ['c++11', 'c++14', 'c++1z'])) != 0
> enable_warnings = self.get_lit_bool('enable_warnings',
> default_enable_warnings)
> - if enable_warnings:
> - self.cxx.useWarnings(True)
> - self.cxx.warning_flags += [
> - '-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER',
> - '-Wall', '-Wextra', '-Werror'
> - ]
> - self.cxx.addWarningFlagIfSupported('-Wshadow')
> - self.cxx.addWarningFlagIfSupported('-Wno-unused-command-line-
> argument')
> - self.cxx.addWarningFlagIfSupported('-Wno-attributes')
> - self.cxx.addWarningFlagIfSupported('-Wno-pessimizing-move')
> - self.cxx.addWarningFlagIfSupported('-Wno-c++11-extensions')
> - self.cxx.addWarningFlagIfSupported('-
> Wno-user-defined-literals')
> - # These warnings should be enabled in order to support the
> MSVC
> - # team using the test suite; They enable the warnings below
> and
> - # expect the test suite to be clean.
> - self.cxx.addWarningFlagIfSupported('-Wsign-compare')
> - self.cxx.addWarningFlagIfSupported('-Wunused-variable')
> - self.cxx.addWarningFlagIfSupported('-Wunused-parameter')
> - self.cxx.addWarningFlagIfSupported('-Wunreachable-code')
> - # FIXME: Enable the two warnings below.
> - self.cxx.addWarningFlagIfSupported('-Wno-conversion')
> + self.cxx.useWarnings(enable_warnings)
> + self.cxx.warning_flags += [
> + '-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER',
> + '-Wall', '-Wextra', '-Werror'
> + ]
> + if self.cxx.hasWarningFlag('-Wuser-defined-warnings'):
> + self.cxx.warning_flags += ['-Wuser-defined-warnings']
> + self.config.available_features.add('diagnose-if-support')
> + self.cxx.addWarningFlagIfSupported('-Wshadow')
> + self.cxx.addWarningFlagIfSupported('-Wno-unused-command-line-
> argument')
> + self.cxx.addWarningFlagIfSupported('-Wno-attributes')
> + self.cxx.addWarningFlagIfSupported('-Wno-pessimizing-move')
> + self.cxx.addWarningFlagIfSupported('-Wno-c++11-extensions')
> + self.cxx.addWarningFlagIfSupported('-Wno-user-defined-literals')
> + # These warnings should be enabled in order to support the MSVC
> + # team using the test suite; They enable the warnings below and
> + # expect the test suite to be clean.
> + self.cxx.addWarningFlagIfSupported('-Wsign-compare')
> + self.cxx.addWarningFlagIfSupported('-Wunused-variable')
> + self.cxx.addWarningFlagIfSupported('-Wunused-parameter')
> + self.cxx.addWarningFlagIfSupported('-Wunreachable-code')
> + # FIXME: Enable the two warnings below.
> + self.cxx.addWarningFlagIfSupported('-Wno-conversion')
> + self.cxx.addWarningFlagIfSupported('-Wno-unused-local-typedef')
> + std = self.get_lit_conf('std', None)
> + if std in ['c++98', 'c++03']:
> + # The '#define static_assert' provided by libc++ in C++03 mode
> + # causes an unused local typedef whenever it is used.
> self.cxx.addWarningFlagIfSupported('-
> Wno-unused-local-typedef')
> - std = self.get_lit_conf('std', None)
> - if std in ['c++98', 'c++03']:
> - # The '#define static_assert' provided by libc++ in C++03
> mode
> - # causes an unused local typedef whenever it is used.
> - self.cxx.addWarningFlagIfSupported('-
> Wno-unused-local-typedef')
>
> def configure_sanitizer(self):
> san = self.get_lit_conf('use_sanitizer', '').strip()
>
> Modified: libcxx/trunk/test/libcxx/test/format.py
> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/
> libcxx/test/format.py?rev=291961&r1=291960&r2=291961&view=diff
> ============================================================
> ==================
> --- libcxx/trunk/test/libcxx/test/format.py (original)
> +++ libcxx/trunk/test/libcxx/test/format.py Fri Jan 13 16:02:08 2017
> @@ -223,6 +223,10 @@ class LibcxxTestFormat(object):
> test_cxx.flags += ['-fsyntax-only']
> if use_verify:
> test_cxx.useVerify()
> + test_cxx.useWarnings()
> + if '-Wuser-defined-warnings' in test_cxx.warning_flags:
> + test_cxx.warning_flags += ['-Wno-error=user-defined-
> warnings']
> +
> cmd, out, err, rc = test_cxx.compile(source_path, out=os.devnull)
> expected_rc = 0 if use_verify else 1
> if rc == expected_rc:
>
> Modified: libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/
> diagnose_reference_binding.fail.cpp
> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/
> libcxx/utilities/tuple/tuple.tuple/diagnose_reference_
> binding.fail.cpp?rev=291961&r1=291960&r2=291961&view=diff
> ============================================================
> ==================
> --- libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/
> diagnose_reference_binding.fail.cpp (original)
> +++ libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/
> diagnose_reference_binding.fail.cpp Fri Jan 13 16:02:08 2017
> @@ -30,4 +30,11 @@ int main() {
> // bind rvalue to constructed non-rvalue
> std::tuple<std::string &&> t2("hello"); // expected-note {{requested
> here}}
> std::tuple<std::string &&> t3(std::allocator_arg, alloc, "hello"); //
> expected-note {{requested here}}
> +
> + // FIXME: The below warnings may get emitted as an error, a warning,
> or not emitted at all
> + // depending on the flags used to compile this test.
> + {
> + // expected-warning at tuple:* 0+ {{binding reference member 'value' to
> a temporary value}}
> + // expected-error at tuple:* 0+ {{binding reference member 'value' to a
> temporary value}}
> + }
> }
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170123/8d7f8a23/attachment-0001.html>
More information about the cfe-commits
mailing list