[libcxx] r291961 - Add _LIBCPP_DIAGNOSE_WARNING and _LIBCPP_DIAGNOSE_ERROR macros.

George Burgess via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 23 10:46:05 PST 2017


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.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.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-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/e0540d39/attachment-0001.html>


More information about the cfe-commits mailing list