Re: [boost] Formal Review: any

From: Jeremy Siek (jsiek_at_[hidden])
Date: 2000-11-02 11:54:53

Conclusion: I suggest the acceptance of library "any" to Boost.
 Its solve an important problem, the code looks good,
 and the documentation is thorough.

I put about 1 hour into evaluating the library. Here's
my comments.

Design
======

  Looks great!
  
  One question: what's the relation between
  class any and the class poly described in Dietmar's
  paper? http://www.oonumerics.org/tmpw00/kuehl.html
  I think the difference is that Dietmar used pointer syntax,
  operator* and operator-> to provide access to the underlying
  object... I'm not so sure that is a good idea.

Documentation
=============

  The predicate examples are confusing when you get to copy_to() and
  to_ptr(). This is because these two functions have not yet been
  introduced, and the predicate examples are basically using these
  functions for their secondary purpose, not their primary purpose. It
  would be better to first have examples that demonstrate the primary
  purpose of these functions.

Code
====

  From what I can see, the MSVC_INCLUDE/MSVC_EXCLUDE is used to
  workaround several different compiler bugs. It would be better to use
  the bug-specific flag for each case instead of a compiler-specific
  flag. This would make it easier to port to more compilers such as
  Sun's C++.

  In test.hpp line 11, use of "string" as a template parameter a but
  confusing. How about "string_type" instead.

Compiling for KAI C++ 3.4f
==========================

  I got several errors of the following kind:

  "test.hpp", line 40: error: exception specification for virtual function
            "test::failure::what" is incompatible with that of overridden
            function "std::exception::what"
            virtual const char *what() const

  The errors could be fixed by adding a throw():

          virtual const char *what() const throw()

  ------

  I also got the following warning:

  "test.hpp", line 29: warning: exception specification for implicitly declared
        virtual function "test::failure::~failure" is incompatible with that
            of overridden function "std::exception::~exception"
        class failure : public std::exception
              ^

  This could be fixed by adding this destructor:

          virtual ~failure() throw() { }

Compiling for g++ 2.95.2
========================
  g++ doesn't provide <sstream> so I couldn't compile the test
     using g++'s native headers. Would it be possible to
     remove the use of <sstream> from the test?

Compiling for Sun WorkShop 6 2000/04/07 C++ 5.1
===============================================

  Lots of errors. I'd be willing to help port this once the
  MSVC_INCLUDE macros get changed to bug-specific macros.

----------------------------------------------------------------------
 Jeremy Siek                        www: http://www.lsc.nd.edu/~jsiek/
 Ph.D. Candidate                  email: jsiek_at_[hidden]
 Univ. of Notre Dame         work phone: [phone]
----------------------------------------------------------------------