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@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@__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@tuple:* 0+ {{binding reference member 'value' to > a temporary value}} > + // expected-error@tuple:* 0+ {{binding reference member 'value' to a > temporary value}} > + } > } > > > _______________________________________________ > 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
