Modify

Ticket #2823 (new Bugs)

Opened 5 years ago

Last modified 3 years ago

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

Reported by: dkomisar@… Owned by: djowel
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

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

Change History

Changed 5 years ago by dkomisar@…

comment:1 Changed 5 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 3 years ago by mikhailberis

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 3 years ago by djowel

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 follow-up: ↓ 5 Changed 3 years ago by grafik

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 3 years ago by mikhailberis

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 3 years ago by mikhailberis

Patch as proposed by Rene, applies to r66834.

comment:6 Changed 3 years ago by marshall

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

comment:7 Changed 3 years ago by marshall

  • Owner changed from djowel to marshall

comment:8 Changed 3 years ago by marshall

  • Owner changed from marshall to djowel

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 3 years ago by mikhailberis

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

Changed 3 years ago by mikhailberis

Updated patch to take Steven's suggestion.

comment:10 Changed 3 years ago by marshall

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

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as new
Author


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

 
Note: See TracTickets for help on using tickets.