|
|
Boost : |
From: John Torjo (john_at_[hidden])
Date: 2003-06-02 03:50:52
> Hello John,
>
> I tried assert lib under Borland C++ Builder and Intel C++ and bellow are
> few notes so far.
>
> There seems to be recursive loop bug (item 10).
>
> /Pavel
>
> PS: it is sooo big: 20 headers and > 100kB of source. I would never
imagine
> old
> #define assert(x) if (!(x)) abort()
> can grow so much :-O
;-)
True. But it's got sooo many features ;-)
>
> -----------------------------------------------------------
>
> 1. file smart_assert_persist.hpp:
>
> constructor persistence_context( const char_type * strFile = "", int nLine
=
> 0)
>
> could it be
> persistence_context( const char_type * strFile = 0, int nLine = 0)
> ?
True. Done.
>
> Borland C++ Builder doesn't allow the [= ""] expressions for precompiled
> headers (this is limitation of BC++B). I think
>
> semantic remains the same.
>
> 2. smart_assert_defs.hpp:
>
> typedef enum ignore_type
> declaration needs to end with a name (BC++B).
>
> like:
> typedef enun ignore_type {
> ...
> } ignore_type;
>
> The same goes for smart_assert_h_ext.hpp and typedef enum
show_details_type.
Done. Thanks.
>
> 3. smart_assert_h_basic.hpp:
> needs
> #include <ctype.h>
> (or #include <cctype>)
>
> Problem for BC++B: isspace() not recognized.
>
Done.
>
> 4. warnings on BC++B:
>
> I put into boost/smart_assert.hpp:
>
> -------------
> #if _MSC_VER >= 1020
> #pragma once
> #endif
>
> #ifdef __BORLANDC__
> #pragma warn -8027 // functions not expanded inline
> #pragma warn -8026 // functions with exception spec not expanded inline
> #endif
>
> #include "boost/smart_assert/smart_assert.hpp"
> #include "boost/smart_assert/smart_enforce.hpp"
> #include "boost/smart_assert/smart_verify.hpp"
>
> #include "boost/smart_assert/smart_assert_settings.hpp"
>
> #ifdef __BORLANDC__
> #pragma warn +8027
> #pragma warn -8026
> #endif
>
> #endif
Thanks!
Did it.
At the last #pragma, didn't you mean
// + instead of -
#pragma warn +8026 ?
> ----------------
>
> This got rid of ~20 warnings in Release mode.
>
> 5. smart_assert_alloc.hpp contains tab(s). Tabs are not allowed in Boost
> sources.
>
> line with : void construct(pointer p, const type & v) {
> and line after closing bracket of this function.
Thanks! removed them (it was because I pasted some code from somewhere else)
>
> 6. Maybe there can be some more subdirectories in smart_assert/, like
> handlers/, loggers/, utils/. Right now there are 18
>
> files with the same long prefixes.
Hmm. Yes, I could do this - I'll think about it.
>
> 7. smart_asert_keeper.hpp: please put newline at the end of file. It is
> (IMHO stupid) standard requirement.
Done. I saw it afterwards ;-)
I rechecked all files for this anyhow.
>
> 8. smart_assert_l_ext.hpp, function void erase_logger(int) gives warning
on
> signed/unsigned comparison: can the parameter be
>
> std::size_t?
Done. Thanks.
>
> 9. smart_assert_defs.hpp: the derived exception may conflict with
different
> definitions over different compilers (it happened
>
> often to me).
>
> For Borland C++ Builder I needed to do:
>
> const char *
> #ifdef __BORLANDC__
> // if we use different default calling convention
> __cdecl
> #endif
> what() const throw() {
> return m_val.c_str();
> }
>
>
> and
>
> #ifdef __BORLANDC__
> // if we use different default calling convention
> __cdecl
> #endif
> ~smart_assert_error() throw() {
> }
Done. Thanks!
>
> 10. in smart_assert_keeper.hpp:
>
> the function
> assert_context_func( const assert_context_func & val)
> : m_pimpl( get_clone( val.m_pimpl) ) {
> }
>
> causes infinite loop on BC+B. I think the clone() function recursively
calls
> the constructor. I may dig into it deeper if you
>
> want.
>
> Intel C++ throws access violation on line
> return pimpl ? pimpl->clone() : 0;
Oops :-(
I redefined the whole assert_context_func - I never liked it the way it used
to be. Please take a look when you have the time - I'll post the new code
soon.
>
>
> 11. smart_assert_ts.hpp can have:
>
> #include <boost/config.hpp>
> #ifndef BOOST_HAS_THREADS
> # undef BOOST_SMART_ASSERT_THREAD_SAFE
> #endif
>
> in the beginning.
>
> IMO recursive_mutex should be used.
True. I thought about it as well, and that's how it is now.
>
> 12. When I define BOOST_SMART_ASSERT_NO_CUSTOM_ALLOC source cannot be
> compiled (BC++B).
>
> I think it is because of
> typedef std::vector< val_and_str, Private::smart_assert_alloc<
> val_and_str> > vals_array;
>
> in smart_assert_context.hpp.
>
> Error listing:
> --------------------------------------------------------------------------
--
> ----
> [...]
O God! What an error :-(
I think I got it. Please recheck if it appears again.
>
>
> 13. Suggestion: maybe function like assert_settings() can be named like:
> assert_settings_singleton()
>
> It is more expressive.
True. But a lot more to type ;-)
>
> 14. Style suggestion:
>
> in templates like
> template< class type>
> type ret_on_fail( const type & val) {
> return val;
> }
>
> the
> template< class T>
> T ret_on_fail(const T& val) {
> return val;
> }
> would be more common. When I saw this first time I looked for 'type'
typedef
> somewhere.
I have to think about that. I have used 'type' a lot of times.
>
> 15. new/delete usage:
>
> maybe allocations like:
> m_pimpl = new Private::assert_context_func_impl< T>(val);
> can be expressed using assert allocator. (I am not really sure if this
makes
> sense.)
I guess you're right. I will most likely do it.
>
> 16. Maybe when someting is delete like
>
> template< class T>
> assert_context_func & assign( const T & val) {
>
> delete m_pimpl;
> m_pimpl = new Private::assert_context_func_impl< T>(val);
> return *this;
> }
> it should be zeroed:
>
> template< class T>
> assert_context_func & assign( const T & val) {
>
> delete m_pimpl;
> m_pimpl = 0;
> m_pimpl = new Private::assert_context_func_impl< T>(val);
> return *this;
> }
True. Did it.
Thanks so much for your comments.
Best,
John
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk