Skip to content

Add RFC for tbb::optional#2040

Open
kboyarinov wants to merge 1 commit intomasterfrom
dev/kboyarinov/rfc-tbb-optional
Open

Add RFC for tbb::optional#2040
kboyarinov wants to merge 1 commit intomasterfrom
dev/kboyarinov/rfc-tbb-optional

Conversation

@kboyarinov
Copy link
Copy Markdown
Contributor

Description

Add a comprehensive description of proposed changes

Fixes # - issue number(s) if exists

Type of change

Choose one or multiple, leave empty if none of the other choices apply

Add a respective label(s) to PR if you have permissions

  • bug fix - change that fixes an issue
  • new feature - change that adds functionality
  • tests - change in tests
  • infrastructure - change in infrastructure and CI
  • documentation - documentation update

Tests

  • added - required for new features and some bug fixes
  • not needed

Documentation

  • updated in # - add PR number
  • needs to be updated
  • not needed

Breaks backward compatibility

  • Yes
  • No
  • Unknown

Notify the following users

List users with @ to send notifications

Other information

formally required) since a default constructed object is the intuitive choice for signaling end-of-input when
no sentinel is available. Non-default-constructible output types require contrived workarounds.

With ``std::optional``, this same body becomes self-documenting:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is worth mentioning, how would the body of one-filter pipeline look like. Something like optional<continue_msg> could be used in this case:

std::optional<tbb::flow::continue_msg> operator()() {
    if (end-of-input) {
        return {};
    }
    return tbb::flow::continue_msg{};
}

Perhaps moving continue_msg out of flow should be considered.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I get the point about continue_msg? Are you suggesting to move continue_msg out of the flow namespace or that we can drop continue_msg completely?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I understand, the point is that Optional-Like cannot be used for single pipeline filter (both input and output are void):

void single_filter_body() {
     // Return type is void, how to signal end-of-pipeline?
}

The idea is to use continue_msg as a special flag class in this case and return non-empty optional<continue_msg> if the pipeline should be continued and an empty optional if the pipeline should be stopped.

Since continue_msg is part of flow namespace now, the idea is to move it to tbb namespace. However, since we need to keep the old code that uses tbb::flow::continue_msg, I guess having it in both tbb and tbb::flow is a better option.

An alternative is to keep flow_control for single-filter (or even for entire parallel_pipeline) and to migrate to Optional-Like only for input_node.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I see now. For this use, the semantics of continue_msg is different. Usually, it means that the predecessor-successor dependence has been satisfied between two nodes. If used to signal the continue/stop condition for a single filter pipeline, it would be acting as-if there is an imaginary back edge from the single filter to itself. You call it a "special flag class". I wonder if any savings from eliminating an extra class is worth the possible sematic confusion?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants