I like the idea of tagging diagnose_ifs with tags, though I wonder how this could be made applicable to code outside of libcxx. Specifically, if I import some big library that uses diagnose_if, then I'd still need to use `-Wno-user-defined-warnings` if said lib had a single diagnose_if that I didn't want to fix immediately.
My thought is that we may be able to tag these IDs with namespaces, and we can match against those in the -Wno-user-defined-warnings=whatever. For example: namespace std { void foo(int i) __attribute__((diagnose_if(i, "oh no", "warning", "foo_tag"))); void bar(int i) __attribute__((diagnose_if(i, "oh no", "warning", "bar_tag"))); } -Wno-user-defined-warnings='std::' // all diagnose_ifs in std:: are disabled -Wno-user-defined-warnings='std::foo_tag' // std::bar can warn -Wno-user-defined-warnings='std::foo_tag,std::bar_tag' // neither std::foo nor std::bar warn If not this, then I'd like to at least consider some kind of wildcarding thing in the warning flag, since libraries will likely roll their own tag-namespaces anyway to avoid things like: namespace boost { void foo(int i) __attribute__((diagnose_if(i, "oh no", "warning", "foo"))); } namespace internal_lib { void foo(int i) __attribute__((diagnose_if(i, "oh no", "warning", "foo"))); } -Wno-user-defined-warnings='foo' // disables boost::foo and internal_lib::foo's warnings. To be clear, I'm imagining the wildcards would look something like #define _LIBCXX_TAG "libcxx_warning_" namespace std { void foo(int i) __attribute__((diagnose_if(i, "oh no", "warning", _LIBCXX_TAG "foo_tag"))); void bar(int i) __attribute__((diagnose_if(i, "oh no", "warning", _LIBCXX_TAG "bar_tag"))); } -Wno-user-defined-warnings='libcxx_warning*' // std::foo and std::bar warnings are disabled -Wno-user-defined-warnings='libcxx_warning_foo_tag' // std::bar can warn -Wno-user-defined-warnings='libcxx_warning_foo_tag,libcxx_warning_bar_tag' // neither std::foo nor std::bar warn ...Personally, I'm leaning toward the implicit namespacing, since it's impossible to forget (e.g. a library that doesn't use tags can be disabled with 'lib_namespace::', instead of having to disable all user-defined warnings), but I'd be content with either. On Mon, Jan 23, 2017 at 2:11 PM, Eric Fiselier via cfe-commits < cfe-commits@lists.llvm.org> wrote: > My dream, and something I would like to work towards is supporting > something like this: > > On Mon, Jan 23, 2017 at 2:11 PM, Eric Fiselier <e...@efcs.ca> wrote: > My dream, and something I would like to work towards is supporting > something like this: > > > [[clang::libcxx_diagnose_if(cond, "message", "warning", /* warning-id*/ > "non-const-functor")]] > > > > -Wno-libcxx-warnings=non-const-functor > > This way libc++ warnings get treated differently from all other users of > diagnose_if, and can be enabled > and disabled separately. > > @George does this sound like a reasonable goal? If so I'm happy to start > working on this. > > /Eric > > > On Mon, Jan 23, 2017 at 11:46 AM, George Burgess <g...@google.com> wrote: > >> The only plan that we have at the moment is basically for a >> -Wno-user-defined-warnings-in-system-headers type of flag. I agree that >> it would be nice if we could be more granular than this, so I'll think >> about what we can do. >> >> On Mon, Jan 23, 2017 at 8:36 AM, Nico Weber <tha...@chromium.org> wrote: >> >>> 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_co >>>> mparator.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/diagnos >>>> e_reference_binding.fail.cpp >>>> >>>> Modified: libcxx/trunk/docs/UsingLibcxx.rst >>>> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/docs/UsingL >>>> ibcxx.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/__c >>>> onfig?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_ADDIT >>>> IONAL_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/__t >>>> ree?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/typ >>>> e_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_co >>>> mparator.fail.cpp >>>> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx >>>> /containers/associative/non_const_comparator.fail.cpp?rev=29 >>>> 1961&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.addWarningFlagIfSuppo >>>> rted('-Wno-unused-command-line-argument') >>>> - self.cxx.addWarningFlagIfSupported('-Wno-attributes') >>>> - self.cxx.addWarningFlagIfSuppo >>>> rted('-Wno-pessimizing-move') >>>> - self.cxx.addWarningFlagIfSuppo >>>> rted('-Wno-c++11-extensions') >>>> - self.cxx.addWarningFlagIfSuppo >>>> rted('-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-litera >>>> ls') >>>> + # 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-typede >>>> f') >>>> + 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.addWarningFlagIfSupp >>>> orted('-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.addWarningFlagIfSuppo >>>> rted('-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/diagnos >>>> e_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