Modify

Opened 9 years ago

Closed 7 years ago

#2294 closed Bugs (fixed)

Please improve test annotation

Reported by: Dave Abrahams Owned by: Marshall Clow
Milestone: Boost 1.37.0 Component: optional
Version: Boost 1.35.0 Severity: Problem
Keywords: Cc:

Description

The explicit-failures-markup for the optional_test_ref test is not informative enough to be useful to me. OK, so there's a compiler bug. How does it affect the usability of the library? What actually *is* the behavior of the library when I create optional<T&>? Is it just the address-of operator that's broken, or does the optional not actually store the reference as it should?

I'd appreciate it if you could improve this text.

Attachments (1)

boost-optional-explicit-failures-markup-issue2294.diff (2.7 KB) - added by Dean Michael Berris 7 years ago.
Proposed patch including the proposed text in the issue.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 9 years ago by Dave Abrahams

I think I understand the bug now (though the text should still be improved) and I have a suggestion to make: on those compilers where the bug exists, make the constructor in question explicit so that you can't mistakenly end up with a dangling reference. I did that by brute force in my local copy by specializing optional<V const&> and adding "explicit" to the ctor.

comment:2 Changed 9 years ago by Dave Abrahams

Suggested new text:

This failure is caused by a compiler bug, and as far as we can tell, can't be worked around in the library, although we think the library might be made safer with respect to this bug.

Specifics: the following simple test fails when it should succeed.

 #include <cassert>

int const x = 0;
struct A
{
   A(int const& y)
   {
       assert(&x == &y);
   }
};

int main()
{
   A a(x);  // direct initialization works fine
   A b = x; // copy initialization causes x to be copied before it is bound
}

The possible safety enhancement would be to cause the constructor in question to be explicit for optional<T const&>; that would prevent copy initialization.

comment:3 Changed 7 years ago by Dean Michael Berris

Dave, can you be a little more explicit as to which file this text should go in? I'm prepared to make a patch but I can't see where the expected failure annotation should be changed.

comment:4 Changed 7 years ago by anonymous

It's at status/explicit-failures-markup.xml.

comment:5 in reply to:  4 Changed 7 years ago by Dean Michael Berris

Replying to anonymous:

It's at status/explicit-failures-markup.xml.

Thanks for the pointer. I see two expected failure markups for optional_test_ref, one entry for msvc-6.5 and msvc-7.1 and another for the GCC family of compilers.

I'll add the above text in both entries and whoever will apply should do the right thing.

Changed 7 years ago by Dean Michael Berris

Proposed patch including the proposed text in the issue.

comment:6 Changed 7 years ago by Marshall Clow

(In [67743]) Applied patch; refs #2294

comment:7 Changed 7 years ago by Marshall Clow

Owner: changed from Fernando Cacciola to Marshall Clow

comment:8 Changed 7 years ago by Marshall Clow

Resolution: fixed
Status: newclosed

Fix merged to release branch in [67792]

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Marshall Clow.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.