[libcxx] r291961 - Add _LIBCPP_DIAGNOSE_WARNING and _LIBCPP_DIAGNOSE_ERROR macros.
Eric Fiselier via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 23 14:11:37 PST 2017
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 <gbiv at 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 <thakis at 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 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_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 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.addWarningFlagIfSuppo
>>> rted('-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.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-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-typed
>>> ef')
>>> - 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-warn
>>> ings']
>>> +
>>> 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 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/47c59072/attachment-0001.html>
More information about the cfe-commits
mailing list