Skip to content

Conversation

@droberts195
Copy link

In C++17 there is a std::apply function that gets chosen in
preference to the apply function in the anonymous namespace
in the metric and event rate bucket gatherers.

This change renames our function so the compiler does not
have to resolve an ambiguity when it's called.

In C++17 there is a std::apply function that gets chosen in
preference to the apply function in the anonymous namespace
in the metric and event rate bucket gatherers.

This change renames our function so the compiler does not
have to resolve an ambiguity when it's called.
Copy link
Contributor

@edsavage edsavage left a comment

Choose a reason for hiding this comment

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

LGTM

@droberts195
Copy link
Author

Exactly the same clash was observed on Windows in #834 and on macOS in #867, so this must be a C++17 thing rather than a quirk of one of the compilers.

Even though C++17 changes are only essential in master, it doesn't hurt to backport to 7.x too, which will reduce the chance of future backport clashes.

@droberts195 droberts195 merged commit fb53fc7 into elastic:master Dec 2, 2019
@droberts195 droberts195 deleted the rename_apply_function branch December 2, 2019 15:02
droberts195 pushed a commit to droberts195/ml-cpp that referenced this pull request Dec 2, 2019
In C++17 there is a std::apply function that gets chosen in
preference to the apply function in the anonymous namespace
in the metric and event rate bucket gatherers.

This change renames our function so the compiler does not
have to resolve an ambiguity when it's called.

Backport of elastic#871
droberts195 pushed a commit that referenced this pull request Dec 2, 2019
In C++17 there is a std::apply function that gets chosen in
preference to the apply function in the anonymous namespace
in the metric and event rate bucket gatherers.

This change renames our function so the compiler does not
have to resolve an ambiguity when it's called.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants