From: Søren Lassen (s.lassen_at_[hidden])
Date: 2005-04-28 09:57:23
On Wed, 27 Apr 2005 08:01:36 -0400, David Abrahams
<dave_at_[hidden]> wrote:
> Søren Lassen <s.lassen_at_[hidden]> writes:
>
>> On Tue, 26 Apr 2005 12:44:19 -0400, David Abrahams
>> <dave_at_[hidden]> wrote:
>>
>>>
>>> It was discussed in the past IIRC and Philippe's rationale basically
>>> captures the consensus here.
>>>
>>
>> Very well, it seems that I am outnumbered. Non-const return values from
>> binary operators are not a bug, but a feature. However, it remains an
>> undocumented feature.
>
> ?? The return type of the generated operators is perfectly well
> visible in the documentation.
The documentation (http://www.boost.org/libs/utility/operators.htm) says
that the operators return "convertible to T". In the broadest sense, this
can mean anything. In the context, it can be read as either a T (or
something implicitly convertible, e.g. "&T"), or as something that can be
used in an assignment/construction (e.g. also "const T", "const &T"). The
required non-const-ness of the return values is not obvious, and no
rationale is given for it in the documentation, nor in the program
comments.
Quoting from http://www.boost.org/more/lib_guide.htm#Rationale_rationale:
"Beman Dawes comments: Failure to supply contemporaneous rationale for
design decisions is a major defect in many software projects. Lack of
accurate rationale causes issues to be revisited endlessly, causes
maintenance bugs when a maintainer changes something without realizing it
was done a certain way for some purpose, and shortens the useful lifetime
of software."
>
>> It would be easier to contribute to boost if the programming
>> guidelines on the homepage
>
> What are you referring to? There are no programming guidelines in
> http://www.boost.org/index.htm.
There is a link on the left (section "About") that says "Guidelines" it
takes you to
http://www.boost.org/more/lib_guide.htm#Guidelines
The section contains, among other things, programming guidelines. They
are, in some cases, quite detailed.
But you are right. The Guidelines are not on the actual home page.
>
>> corresponded to the actual programming guidelines that have been
>> decided.
>
>
>> At the moment the homepage has an explicit reference to a book
>> ("Effective C++", second edition) that recommends the opposite
>> policy, the real guidelines are not mentioned.
>
> There are no "real guidelines." Decisions are taken on a case-by-case
> basis.
Then why does the homepage have a link called "Guidelines"? And why do you
refer to a past discussion (what is IIRC, by the way? there are no
references on the www.boost.org website), if decisions are taken on a
case-by-case basis?
>
>> I have reintroduced the non-const return values for the operators,
>> the new version has been uploaded to the sandbox vault.
>
> What is this code supposed to fix? The first things you should submit
> are test programs that fail to work because of the things you are
> saying are broken.
>
I tried to fix the following errors:
1) An apparently worthless speed optimization for NRVO, which, for
compilers that only have RVO, is probably worse than no optimization
(because the call-by-value method introduces a named value that is harder
to optimize away). My fix also makes the program easier to read and
maintain.
I do not have examples of code that fails, it does not (were there any
examples of failing programs that were fixed by the NRVO optimization? I
think not). The biggest problem is the unnecessary complicatedness of the
program, and the fact that "operators.hpp", when used as an example, may
entice other programmers to write similarly over-convoluted code. If you
really want Boost to be used widely and to be seen as an example of good
programming, "if it ain't broke, don't fix it" is not the right attitude,
continous improvement is.
By the way, the documentation of the NRVO code is very good, the rationale
is well-explained and well-documented. That is exactly why I am pretty
sure that
a) the rationale unfortunately is a misunderstanding,
b) the optimization can safely be removed.
2) A pollution of the global namespace, which creates a potential
ambiguity. The following examples will fail with compilers that do not
support inline friend functions in namespaces:
#include <boost/operators.hpp>
using namespace boost;
class a:addable1<a>{...}
// Error: ambiguity between boost::addable1 and ::addable1
#include <boost/rational.hpp> // includes operators.hpp
bool addable1;
// Error: addable1 is already defined in global namespace
The problem is not only that the above code fails on these (this?)
platforms, but also that it succeeds on other platforms. Thus, programmers
very easily end up writing non-portable code that has to be rewritten at a
later stage.
My fix for this bug also makes the code easier to read and maintain.
-- Søren