Skip to content

ARROW-17814: [C++] Remove make_unique reimplementation - #14204

Merged
pitrou merged 2 commits into
apache:masterfrom
wgtmac:ARROW-17814
Sep 22, 2022
Merged

ARROW-17814: [C++] Remove make_unique reimplementation#14204
pitrou merged 2 commits into
apache:masterfrom
wgtmac:ARROW-17814

Conversation

@wgtmac

Copy link
Copy Markdown
Member

Use std::make_unique instead.

@pitroupitrou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks! Is it also possible to remove occurrences of using std::make_unique and instead write std::make_unique in full?

@github-actions

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

@wgtmac

Copy link
Copy Markdown
MemberAuthor

Thanks! Is it also possible to remove occurrences of using std::make_unique and instead write std::make_unique in full?

Thanks for the review! Please check it again. @pitrou

@pitroupitrou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM.

@pitrou

Copy link
Copy Markdown
Member

I'll just wait for CI now.

@pitrou

Copy link
Copy Markdown
Member

I rebased and fixed conflicts.

@wgtmac

Copy link
Copy Markdown
MemberAuthor

I rebased and fixed conflicts.

I have done the same thing. Sorry for missing your commit. @pitrou

@pitrou
pitrou merged commit 4cb7b50 into apache:masterSep 22, 2022
@ursabot

Copy link
Copy Markdown

Benchmark runs are scheduled for baseline = 0310add and contender = 4cb7b50. 4cb7b50 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed ⬇️0.55% ⬆️0.24%] test-mac-arm
[Failed ⬇️1.1% ⬆️0.27%] ursa-i9-9960x
[Finished ⬇️0.46% ⬆️0.04%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] 4cb7b500 ec2-t3-xlarge-us-east-2
[Failed] 4cb7b500 test-mac-arm
[Failed] 4cb7b500 ursa-i9-9960x
[Finished] 4cb7b500 ursa-thinkcentre-m75q
[Finished] 0310add1 ec2-t3-xlarge-us-east-2
[Finished] 0310add1 test-mac-arm
[Failed] 0310add1 ursa-i9-9960x
[Finished] 0310add1 ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

@ursabot

Copy link
Copy Markdown

['Python', 'R'] benchmarks have high level of regressions.
ursa-i9-9960x

@kou

kou commented Sep 23, 2022

Copy link
Copy Markdown
Member

@wgtmac This breaks our lint: https://github.com/apache/arrow/actions/runs/3105831802/jobs/5031935208#step:5:866

--- /arrow/cpp/src/arrow/type.cc+++ /arrow/cpp/src/arrow/type.cc (after clang format)@@ -1723,8 +1723,7 @@
SchemaBuilder::SchemaBuilder(std::vector<std::shared_ptr<Field>> fields,
ConflictPolicy policy,
Field::MergeOptions field_merge_options) {
- impl_ = std::make_unique<Impl>(std::move(fields), nullptr, policy,- field_merge_options);+ impl_ = std::make_unique<Impl>(std::move(fields), nullptr, policy, field_merge_options);
}
SchemaBuilder::SchemaBuilder(const std::shared_ptr<Schema>& schema, ConflictPolicy policy,
...

Could you fix this as a follow-up pull request? Or do you want me to fix this?

@kou

kou commented Sep 23, 2022

Copy link
Copy Markdown
Member

@pitrou

Copy link
Copy Markdown
Member

Oops, I don't know how I missed that. @kou I'd be glad if you could do it :-)

@kou

kou commented Sep 23, 2022

Copy link
Copy Markdown
Member

OK. I'll do it.

@koukou mentioned this pull request Sep 23, 2022
@kou

kou commented Sep 23, 2022

Copy link
Copy Markdown
Member

#14218

lidavidm pushed a commit that referenced this pull request Sep 23, 2022
This is a follow-up of #14204.
Authored-by: Sutou Kouhei <kou@clear-code.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
zagto pushed a commit to zagto/arrow that referenced this pull request Oct 7, 2022
Use std::make_unique instead.
Authored-by: Gang Wu <ustcwg@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
zagto pushed a commit to zagto/arrow that referenced this pull request Oct 7, 2022
This is a follow-up of apache#14204.
Authored-by: Sutou Kouhei <kou@clear-code.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
Sign up for freeto 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.

4 participants

@wgtmac@pitrou@ursabot@kou