Modify

Opened 9 years ago

Closed 2 years ago

#2823 closed Bugs (fixed)

[fusion] vector copy constructor copies sequence members in different order on different platforms

Reported by: dkomisar@… Owned by: Joel de Guzman
Milestone: Boost 1.39.0 Component: fusion
Version: Boost 1.38.0 Severity: Problem
Keywords: Cc:

Description

The generalized fusion::vector copy constructor for constructing from arbitrary fusion sequences does its copying in reverse order on one of my machines. This does not occur with vector copy assignment, and does not occur at all with fusion::list (see attached code).

In all my real world use this has not made a difference; I uncovered it in a unit test. The attached code shows what happens when you use transform_view to transform a fusion sequence with a stateful fusion functor.

Correct order: Mac PPC OS 10.5.6 Darwin Kernel 9.6.0 gcc 4.2.1

Reverse order: Linux 2.6.9-42.0.3.ELsmp i686 i686 i386 gcc 4.2.4

Attachments (3)

test.cpp (1.3 KB) - added by dkomisar@… 9 years ago.
boost-fusion-issue2823.diff (550 bytes) - added by Dean Michael Berris 7 years ago.
Patch as proposed by Rene, applies to r66834.
boost-fusion-issue2823-2.diff (535 bytes) - added by Dean Michael Berris 7 years ago.
Updated patch to take Steven's suggestion.

Download all attachments as: .zip

Change History (14)

Changed 9 years ago by dkomisar@…

Attachment: test.cpp added

comment:1 Changed 9 years ago by dkomisar@…

Took a look at the code and it looks like this is due to function call arguments being called in different orders on different platforms, so there's not much that can be done. I think that this really needs to be highlighted (at least mentioned) in the docs somewhere.

I could be wrong though, I'm not very good at reading PP-intensive code.

comment:2 Changed 7 years ago by Dean Michael Berris

It looks like a compiler bug, should this be marked as wontfix appropriately, and have a different issue for the documentation changes? Joel?

comment:3 Changed 7 years ago by Joel de Guzman

Dean, it's not a compiler bug. This is a known problem. The order of evaluation of function call arguments are undefined accd' to standard. Need further to look into this.

comment:4 Changed 7 years ago by René Rivera

I've run into this problem recently, and bugged Hartmut about it on IRC. My solution to the implementation defined behavior that makes all fusion sequence operations have an unpredictable execution ordering. My solution was to create my own version of fusion::as_list that has a guaranteed execution order. The key being to make copies of the sequence values in the correct ordering. Which boils down to changing fusion::detail::build_cons<First,Last,false>::call(...) from:

static type
call(First const& f, Last const& l)
{
    return type(*f, next_build_cons::call(fusion::next(f), l));
}

To:

static type
call(First const& f, Last const& l)
{
    typename result_of::value_of<First>::type v = *f;
    return type(v, next_build_cons::call(fusion::next(f), l));
}

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

Replying to grafik:

The key being to make copies of the sequence values in the correct ordering. Which boils down to changing fusion::detail::build_cons<First,Last,false>::call(...) from:

...

To:

...

Interesting. Let me try that locally and run the tests -- unfortunately I cannot test on multiple platforms. I'll attach the patch for testing.

Changed 7 years ago by Dean Michael Berris

Attachment: boost-fusion-issue2823.diff added

Patch as proposed by Rene, applies to r66834.

comment:6 Changed 7 years ago by Marshall Clow

(In [67745]) Applied patch; refs #2823; will merge to release once tests cycle

comment:7 Changed 7 years ago by Marshall Clow

Owner: changed from Joel de Guzman to Marshall Clow

comment:8 Changed 7 years ago by Marshall Clow

Owner: changed from Marshall Clow to Joel de Guzman

On-list, Steven Watanabe wrote:

> result_of::deref would be better. There's no reason to make an extra copy. (Although this code looks pretty copy-happy to begin with).

Joel says that he will take it from here; assigning back to him. However, the patch that I applied does fix the bug.

comment:9 Changed 7 years ago by Dean Michael Berris

I've updated the patch (made against the latest in Trunk, 67745) which implements the suggestion. Please see the new attachment.

Changed 7 years ago by Dean Michael Berris

Updated patch to take Steven's suggestion.

comment:10 Changed 7 years ago by Marshall Clow

Fix merged to release branch in [67792]; Joel will optimize more.

comment:11 Changed 2 years ago by Joel de Guzman

Resolution: fixed
Status: newclosed

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Joel de Guzman.
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.