Re: debug mode maintenance patch
On 28/05/15 22:32 +0200, François Dumont wrote: Sorry, I saw it used so many times for macros that I though it was the right way to report macro modifications. The changelog style is fairly complicated :-) I also replicate Copyrights from debug.h to assertions.h. * include/debug/debug.h (_GLIBCXX_DEBUG_ASSERT, _GLIBCXX_DEBUG_PEDASSERT, _GLIBCXX_DEBUG_ONLY): Move definition... * include/debug/assertions.h: ...here, new. * include/debug/formatter.h (struct _Error_formatter::_Is_iterator_value_type): New. (struct _Error_formatter::_Is_instance): New. (struct _Error_formatter::_Parameter): Make public and not friend anymore. (_Error_formatter::_Parameter::__instance): New _M_kind enum entry. (_Error_formatter::_Parameter::__iterator_value_type): New _M_kind enum entry. (struct _Error_formatter::_Parameter::_Type): New. (struct _Error_formatter::_Parameter::_Instance): New, inherit from latter. (union _Error_formatter::_Parameter::_M_variant): Reorganize. I don't think we need to say 'struct' or 'union' either, because we're not writing C, and they aren't needed in C++, but no big deal. Ok to commit ? Yes, looks good, thanks.
Re: debug mode maintenance patch
On 25/05/2015 20:41, Jonathan Wakely wrote:
On 25/05/15 15:31 +0200, François Dumont wrote:
Hi
This is a patch to clean the debug mode code.
I have introduced a new debug header, assertions.h, so that
headers that only need _GLIBCXX_DEBUG_ASSERT do not have to include
the big debug.h. I also introduce functions.tcc to isolate
implementation of __foreign_iterator which require a number of other
headers.
All other uses of .tcc extensions are headers included automatically
at the bottom of the corresponding .h header, so other headers never
need to do #include foo.tcc because that appears at the end of
foo.h (or foo for standard headers).
Also, .tcc is meant to be for definitions of non-inline templates that
are declared (but not defined) in the header that includes the .tcc
file, but all the functions you're moving to debug/functions.tcc are
still small and inline.
If the point is just to move some functions to a different header
because not all consumers of functions.h need those functions then I
think it should have a different name, not pretend to be related to
functions.h
Ok, then I kept __foreign_iterator within functions.h. I checked and
none of the headers included are themselves including debug headers, as
long as it is like that it is fine.
I'm not convinced moving them to a separate header is even a good
idea. Surely most headers that include debug/functions.h already end
up including bits/move.h and type_traits anyway?
* include/debug/debug.h ([_GLIBCXX_DEBUG_ASSERT,
_GLIBCXX_DEBUG_PEDASSERT, _GLIBCXX_DEBUG_ONLY]): Move definition...
These names should not be in square brackets (square brackets are used
to indicate conditional changes, see
http://www.gnu.org/prep/standards/html_node/Conditional-Changes.html)
Sorry, I saw it used so many times for macros that I though it was the
right way to report macro modifications.
I also replicate Copyrights from debug.h to assertions.h.
* include/debug/debug.h (_GLIBCXX_DEBUG_ASSERT,
_GLIBCXX_DEBUG_PEDASSERT, _GLIBCXX_DEBUG_ONLY): Move definition...
* include/debug/assertions.h: ...here, new.
* include/debug/formatter.h
(struct _Error_formatter::_Is_iterator_value_type): New.
(struct _Error_formatter::_Is_instance): New.
(struct _Error_formatter::_Parameter): Make public and not friend
anymore.
(_Error_formatter::_Parameter::__instance): New _M_kind enum entry.
(_Error_formatter::_Parameter::__iterator_value_type): New _M_kind enum
entry.
(struct _Error_formatter::_Parameter::_Type): New.
(struct _Error_formatter::_Parameter::_Instance): New, inherit from
latter.
(union _Error_formatter::_Parameter::_M_variant): Reorganize.
(_Parameter(_Iterator const, const char*, _Is_iterator)): Make all
overloads take iterator through a const reference.
(_Parameter(const _Iterator, const char*, _Is_iterator_value_type)):
New.
(_Parameter(const _Type, const char*, _Is_instance)): New.
(_Error_formatter::_M_print_type): Delete.
(_Error_formatter::_M_iterator_value_type): New.
(_Error_formatter::_M_instance): New.
* include/Makefile.am: Add new above debug file.
* include/Makefile.in: Regenerate.
* include/debug/functions.h
(__check_dereferenceable(const _Safe_iterator),
__valid_range(const _Safe_iterator),
struct __is_safe_random_iterator_Safe_iterator): Move...
* include/debug/safe_iterator.h: ... here.
Replace debug.h include with assertions.h.
(__check_singular_aux): Move...
* include/debug/safe_base.h: ... here.
* include/debug/functions.h
(__check_dereferenceable(const _Safe_local_iterator),
__valid_range(const _Safe_local_iterator): Move...
* include/debug/safe_local_iterator.h: ...here.
* include/debug/safe_sequence.h: Replace debug.h with assertions.h.
Remove _Safe_iterator declaration.
* include/debug/safe_unordered_container.h: Replace debug.h with
assertions.h.
* include/debug/array: Replace safe_sequence.h include with
formatter.h and macros.h.
* include/debug/deque: Include functions.tcc.
* include/debug/forward_list: Likewise.
* include/debug/list: Likewise.
* include/debug/string: Likewise.
* include/debug/vector: Likewise.
* include/bits/unique_ptr.h: Replace debug.h include with new
assertions.h.
* include/bits/stl_iterator_base_funcs.h: Likewise.
* testsuite/23_containers/array/tuple_interface/get_debug_neg.cc:
Adjust dg-error line number.
* testsuite/23_containers/array/tuple_interface/
tuple_element_debug_neg.cc: Likewise.
* src/c++11/debug.cc: Adapt.
Tested under Linux x86_64.
Ok to commit ?
François
Index: include/Makefile.am
===
--- include/Makefile.am (revision 223846)
+++ include/Makefile.am (working copy)
@@ -759,6 +759,7 @@
debug_builddir = ./debug
debug_headers = \
${debug_srcdir}/array \
+
Re: debug mode maintenance patch
On 05/25/2015 07:41 PM, Jonathan Wakely wrote: Index: include/debug/functions.tcc === --- include/debug/functions.tcc (revision 0) +++ include/debug/functions.tcc (working copy) @@ -0,0 +1,159 @@ +// Debugging support implementation -*- C++ -*- + +// Copyright (C) 2015 Free Software Foundation, Inc. It worries me when we move existing code to new headers and only give it the current year as the copyright date. The copyright applies to the code, which is older than 2015, not to the file itself. I think I've asked about this before and been told that only adding the current year is correct, but that seems wrong to me. I'll try to confirm that. You should retain the old copyright years in the new file. Here's FSF's response when inquired about that a while ago: https://sourceware.org/ml/gdb-patches/2012-04/msg00640.html Thanks, Pedro Alves
Re: debug mode maintenance patch
On 26/05/15 09:45 +0100, Pedro Alves wrote: On 05/25/2015 07:41 PM, Jonathan Wakely wrote: Index: include/debug/functions.tcc === --- include/debug/functions.tcc (revision 0) +++ include/debug/functions.tcc (working copy) @@ -0,0 +1,159 @@ +// Debugging support implementation -*- C++ -*- + +// Copyright (C) 2015 Free Software Foundation, Inc. It worries me when we move existing code to new headers and only give it the current year as the copyright date. The copyright applies to the code, which is older than 2015, not to the file itself. I think I've asked about this before and been told that only adding the current year is correct, but that seems wrong to me. I'll try to confirm that. You should retain the old copyright years in the new file. Here's FSF's response when inquired about that a while ago: https://sourceware.org/ml/gdb-patches/2012-04/msg00640.html Thanks, Pedro. Glad to hear I've been doing it right! François, therefore please ensure you keep the right copyright years when moving code to new files (I'm still not convinced a new file is useful here though).
debug mode maintenance patch
Hi
This is a patch to clean the debug mode code.
I have introduced a new debug header, assertions.h, so that headers
that only need _GLIBCXX_DEBUG_ASSERT do not have to include the big
debug.h. I also introduce functions.tcc to isolate implementation of
__foreign_iterator which require a number of other headers.
I reorganized _Error_formatter. Nested _Parameter is now public
like several format methods so that I am able to move some code to
debug.cc. I would have needed to export new symbols otherwise. I
introduced 2 new type of debug parameters, __instance for any functor,
__iterator_value_type to report iterator value_type. I will use those in
new debug checks to come.
Note that I try to regenerate include/Makefile.in with autoreconf
2.64 but it was producing a lot of modifications in all Makefile.in. Can
someone check if those files are up to date ?
2015-02-19 François Dumont fdum...@gcc.gnu.org
* include/debug/debug.h ([_GLIBCXX_DEBUG_ASSERT,
_GLIBCXX_DEBUG_PEDASSERT, _GLIBCXX_DEBUG_ONLY]): Move definition...
* include/debug/assertions.h: ...here, new.
* include/debug/formatter.h
(struct _Error_formatter::_Is_iterator_value_type): New.
(struct _Error_formatter::_Is_instance): New.
(struct _Error_formatter::_Parameter): Make public and not friend
anymore.
(_Error_formatter::_Parameter::__instance): New _M_kind enum entry.
(_Error_formatter::_Parameter::__iterator_value_type): New _M_kind enum
entry.
(struct _Error_formatter::_Parameter::_Type): New.
(struct _Error_formatter::_Parameter::_Instance): New, inherit from
latter.
(union _Error_formatter::_Parameter::_M_variant): Reorganize.
(_Parameter(_Iterator const, const char*, _Is_iterator)): Make all
overloads take iterator through a const reference.
(_Parameter(const _Iterator, const char*, _Is_iterator_value_type)):
New.
(_Parameter(const _Type, const char*, _Is_instance)): New.
(_Error_formatter::_M_print_type): Delete.
(_Error_formatter::_M_iterator_value_type): New.
(_Error_formatter::_M_instance): New.
* include/debug/functions.h (__foreign_iterator): Move definition...
* include/debug/functions.tcc: ...here, new.
* include/Makefile.am: Add new above debug files.
* include/Makefile.in: Regenerate.
* include/debug/safe_iterator.h: Replace debug.h include with
assertions.h.
(__check_dereferenceable, __valid_range): Move here overload for
_Safe_iterator.
(struct __is_safe_random_iterator): Move here partial specialization
for _Safe_iterator.
(__check_singular_aux): Move...
* include/debug/safe_base.h (__check_singular_aux): ... here.
* include/debug/safe_local_iterator.h (__check_dereferenceable)
(__valid_range): Move here overload for _Safe_local_iterator.
* include/debug/safe_sequence.h: Replace debug.h with assertions.h.
Remove _Safe_iterator declaration.
* include/debug/safe_unordered_container.h: Replace debug.h with
assertions.h.
* include/debug/array: Replace safe_sequence.h include with
formatter.h and macros.h.
* include/debug/deque: Include functions.tcc.
* include/debug/forward_list: Likewise.
* include/debug/list: Likewise.
* include/debug/string: Likewise.
* include/debug/vector: Likewise.
* include/bits/unique_ptr.h: Replace debug.h include with new
assertions.h.
* include/bits/stl_iterator_base_funcs.h: Likewise.
* testsuite/23_containers/array/tuple_interface/get_debug_neg.cc:
Adjust dg-error line number.
* testsuite/23_containers/array/tuple_interface/
tuple_element_debug_neg.cc: Likewise.
* src/c++11/debug.cc: Adapt.
Tested under Linux x86_64 debug mode.
Will commit in a couple of days if no complain.
François
Index: include/Makefile.am
===
--- include/Makefile.am (revision 223630)
+++ include/Makefile.am (working copy)
@@ -759,6 +759,7 @@
debug_builddir = ./debug
debug_headers = \
${debug_srcdir}/array \
+ ${debug_srcdir}/assertions.h \
${debug_srcdir}/bitset \
${debug_srcdir}/debug.h \
${debug_srcdir}/deque \
@@ -765,6 +766,7 @@
${debug_srcdir}/formatter.h \
${debug_srcdir}/forward_list \
${debug_srcdir}/functions.h \
+ ${debug_srcdir}/functions.tcc \
${debug_srcdir}/list \
${debug_srcdir}/map \
${debug_srcdir}/macros.h \
Index: include/Makefile.in
===
--- include/Makefile.in (revision 223630)
+++ include/Makefile.in (working copy)
@@ -1040,6 +1040,7 @@
debug_builddir = ./debug
debug_headers = \
${debug_srcdir}/array \
+ ${debug_srcdir}/assertions.h \
${debug_srcdir}/bitset \
${debug_srcdir}/debug.h \
${debug_srcdir}/deque \
@@ -1046,6 +1047,7 @@
${debug_srcdir}/formatter.h \
${debug_srcdir}/forward_list \
${debug_srcdir}/functions.h \
+ ${debug_srcdir}/functions.tcc \
${debug_srcdir}/list \
Re: debug mode maintenance patch
On 25/05/15 15:31 +0200, François Dumont wrote: Hi This is a patch to clean the debug mode code. I have introduced a new debug header, assertions.h, so that headers that only need _GLIBCXX_DEBUG_ASSERT do not have to include the big debug.h. I also introduce functions.tcc to isolate implementation of __foreign_iterator which require a number of other headers. All other uses of .tcc extensions are headers included automatically at the bottom of the corresponding .h header, so other headers never need to do #include foo.tcc because that appears at the end of foo.h (or foo for standard headers). Also, .tcc is meant to be for definitions of non-inline templates that are declared (but not defined) in the header that includes the .tcc file, but all the functions you're moving to debug/functions.tcc are still small and inline. If the point is just to move some functions to a different header because not all consumers of functions.h need those functions then I think it should have a different name, not pretend to be related to functions.h I'm not convinced moving them to a separate header is even a good idea. Surely most headers that include debug/functions.h already end up including bits/move.h and type_traits anyway? * include/debug/debug.h ([_GLIBCXX_DEBUG_ASSERT, _GLIBCXX_DEBUG_PEDASSERT, _GLIBCXX_DEBUG_ONLY]): Move definition... These names should not be in square brackets (square brackets are used to indicate conditional changes, see http://www.gnu.org/prep/standards/html_node/Conditional-Changes.html) (__check_dereferenceable, __valid_range): Move here overload for _Safe_iterator. (struct __is_safe_random_iterator): Move here partial specialization for _Safe_iterator. These should say Move X here not Move here X. Index: include/debug/functions.tcc === --- include/debug/functions.tcc (revision 0) +++ include/debug/functions.tcc (working copy) @@ -0,0 +1,159 @@ +// Debugging support implementation -*- C++ -*- + +// Copyright (C) 2015 Free Software Foundation, Inc. It worries me when we move existing code to new headers and only give it the current year as the copyright date. The copyright applies to the code, which is older than 2015, not to the file itself. I think I've asked about this before and been told that only adding the current year is correct, but that seems wrong to me. I'll try to confirm that.
