"Robert Ramey" <ramey@rrsd.com> writes:
David Abrahams wrote:
At this point, if you make any change I think you should add a note for those of us who are too clever by half, saying that, yes, it's really okay to overload in boost::serialization, and describing the trick used.
no problem with that.
I've recently checked in changes in the documentation in accordance with this discussion. I believe that they will be satisfactory.
BUT there is a problem.
I recast the section Archive Concept / Saving in the table format used by SGI. I'm very dissatisfied with this for esthetic reasons. I may be more sensitive to this than other people because I use a monitor than can display in portrait mode and that is the way I always use it. (Its in explicable to me that everyone doesn't do this!).
In principle I agree. However, I like having two portrait-sized images side-by-side :) I think the aesthetics of the presentation are mostly OK; your biggest problem is with the long code in the table cells. For the MPL book we dealt with this by inserting line breaks in the cell, e.g.: ar.template register_type< T >() That's an extreme example -- I probably would only use one newline in that one. Oh, part of the problem is that the "Expression" column is full of things that aren't valid expressions, and are therefore much longer than they should be! Okay, it looks like I'm going to have to go through the whole thing and add my comments :(
An Archive object is a sequence of bytes created from an arbitrary nested set of C++ data structures. Here we describe the Archive concept. This is a list of requirements that a class must fullfill in order for a class to be used to save and load and arbitary nest set of C++ structures. Strictly speaking, there are two Archive concepts. One for saving data and one for loading it back. In this document we use the term Archive concepts for either one. Which one we mean should be clear from the context.
That's not really acceptable. The proper thing to do is define the Archive concept of which LoadingArchive and SavingArchive are refinements (choose better names; I picked those to be consistent
Saving
Associated types
+-----------------+-------------------+-------------------------+ |boost::mpl::bool_|Archive::is_saving |boost::mpl::true if this | missing "_ "---------------^ should be "Archive"--^^^^
rewrite: boost::mpl::true_ if data can be saved to an object of type Archive.
| | |is an Archive to which | | | |data can be | | | |saved. boost::mpl::false | | | |otherwise. For an | | | |example showing how this | | | |can beused, see the | | | |implementation of | | | |split_free.hpp. |
The last sentence should be in a footnote or in the main body of the text below.
+-----------------+-------------------+-------------------------+ |boost::mpl::bool_|Archive::is_loading|boost::mpl::true if this | | | |is an Archive from which | | | |data can be | | | |loaded. boost::mpl::false| | | |otherwise. | +-----------------+-------------------+-------------------------+
All table columns need column headers. The first column doesn't make any sense. It's the name of a template. Strike it. It seems like the second column is a Valid Expression just like any other. [is_saving/is_loading are poorly chosen names for compile-time constants: they strongly imply state. I would use can_save and can_load]
Notation
Archive A type that is a model of the Archive concept. T A type that is a model of the Serializable Concept. ar Object of type Archive t Object of type T
You should use something like x instead of t. Having T and t in the same context is slightly confusing and it makes it very difficult to verbally discuss what's in the table.
Valid expressions The following expressions must be valid. \
Kill the backslash-------------------------^
+----------+---------------------+---------------------------------+ |Name |Expression |Return type | | | | |
Note: the "Name" column is a luxury. If you find it helps the aesthetics it's okay to leave it out. I suggest you also drop the word "return" from "return type"
+==========+=====================+=================================+ |Save |ar.save_binary(const |void | |Binary |void *, std::size_t) | |
Okay, that's not a valid expression. You should write something like: ar.save_binary( p, n ) and then add a Notation section that includes something like n a non-negative value of integral type p a pointer to n bytes of storage containing POD data
+----------+---------------------+---------------------------------+ |Save |ar << t |Archive & | | | | | +----------+---------------------+---------------------------------+ |Save |ar & t |Archive & | | | | | +----------+---------------------+---------------------------------+ |Register |ar.template |void | |type |register_type<T>() | | +----------+---------------------+---------------------------------+ |Library |ar.library_version() |unsigned int | |Version | | | +----------+---------------------+---------------------------------+
Expression Semantics +----------+---------------------+---------------------------------+ |Name |Expression |Semantics | +----------+---------------------+---------------------------------+ |Save |ar.save_binary(const |Appends count sequence of bytes | |Binary |void * address, |starting at address to the | | |std::size_t count) |archive. |
Again, not a valid expression. Apply the same correction used earlier here. I don't see why you're not using a single table with 4 columns including Semantics and Return type. Cross-referencing two tables is more work for the user than it should be.
+----------+---------------------+---------------------------------+ |Save |ar << t |Appends a sequence of bytes | | | |corresponding to the value of | | | |type T to the archive. | +----------+---------------------+---------------------------------+ |Save |ar & t |Appends a sequence of bytes | | | |corresponding to the value of | | | |type T to the archive. |
That description of the semantics doesn't seem detailed enough to me. Doesn't it need to call serialize(ar, t, some-version) at some point? I bet it's not enough to append any arbitrary sequence of bytes that I determine corresponds to the value of type T. Remember, this table has to tell me what I need to do in my archive to make it work with the library. Concept tables are not primarily for users of the concept's models (who should have specific documentation for each model), they're for implementors of the models. Don't use the container requirements tables as examples, because the container concepts are almost useless for generic programming. Look at the iterator requirements.
+----------+---------------------+---------------------------------+ |Register |ar.register_type<T>(T|Appends a sequential integer to | |Type |* t = NULL) |the archive. This integer becomes|
What is a "sequential integer?" The expository writing there really should go in a footnote or in the main body of the text below. These tables should be both minimal and complete.
| | |the "key" used to look up the | | | |class type when the archive is | | | |later loaded. This process is | | | |referred to as "class | | | |registration". It is only | | | |necessary to invoke this function| | | |for objects of derived classes | | | |which are serialized through a | | | |base class pointer. This is | | | |explained in detail in Special | | | |Considerations - Derived | | | |Pointers. |
Again, not a valid expression. Maybe you mean ar.register_type((T*)0) ?? The semantics column once again seems far too much geared to users of models of the concept and gives little information to the potential archive implementor.
+----------+---------------------+---------------------------------+ |Library |ar.library_version() |Returns the version number of the| |Version | |serialization library that | | | |created the archive. This number |
So, I'm implementing an archive. How am I supposed to acquire the correct version number? Take the following expository stuff out of the table.
| | |will be incremented each time the| | | |library is altered in such a way | | | |that serialization could be | | | |altered for some type. For | | | |example, suppose the type used | | | |for a count of collection members| | | |is changed. The code that loads | | | |collections might be conditioned | | | |on the library version to make | | | |sure that libraries created by | | | |previous versions of the library | | | |can still be read. | +----------+---------------------+---------------------------------+
Loading
A type that models the Archive concept is a class with the following members:
The template parameter T must correspond to a type which models the
"correspond to" or "be?" AFAICT, you mean: T must be a model of Serializable.
Serializable concept.
template<class T> iarchive & operator>>(T & t);
What is this? operator>> is a binary function. Perhaps you mean this to be a member of something, but if so, it's unclear of what. What is iarchive? Is this documentation of the Archive concept or of some specific model? If the former, is operator>> really required to return iarchive&? If the latter, it doesn't belong on a page titled "Archive Concept." I just snipped out the rest of the documentation because all of the questions I am asking in this break between quotations apply to most of it. I can't tell what you're trying to do. It seems like you don't quite have a clear understanding of what a "Concept" is yet.
I know I didn't choose the orginal form without thinking about it. Looking back I realize now I was strongly influenced by the presentation in "STL Tutorial and Reference Guide" which lays out the requirements in a form similar to the one I chose.
I haven't seen it, but it would shock me if Dave Musser did it wrong.
Aside from any errors that remain which I will be happy to fix in any case, I would like to leave the requirements in the dictionary text form as it was originally.
?? Dictionary text form??
If I move to the table form for the Archive Concept, then in the interest of consistency I would also have to do that with the section Serializable Concept which would be a much larger task. So what I would like to do is revert to the dictionary format for the saving archive concept. I've just uploaded updated document files. It contains one format for the save archive concept and the other for the load archive concept. Interested parties are invited to comment before I make a final change in one or the other.
Unless I'm missing something huge (happened once this week already), the format is the least of your problems. The content is wrong; it doesn't do what a concept description is supposed to do. -- Dave Abrahams Boost Consulting www.boost-consulting.com