Skip to content

: reduce header dependency - #3128

Merged
Stephan T. Lavavej (StephanTLavavej) merged 1 commit into
microsoft:mainfrom
cpplearner:patch-1
Oct 12, 2022
Merged

: reduce header dependency#3128
Stephan T. Lavavej (StephanTLavavej) merged 1 commit into
microsoft:mainfrom
cpplearner:patch-1

Conversation

@cpplearner

@cpplearner S. B. Tam (cpplearner) commented Sep 25, 2022

Copy link
Copy Markdown
Contributor
  • Remove #include after changing uses of _RANGES transform to a simple for-loop.
    • I considered using std::copy when _CharT is same as char, but since _OutputIt is usually std::back_insert_iterator<_Fmt_buffer<_CharT>>, I believe that std::copy is not more optimized than a simple loop.
  • Remove #include . AFAIK does not directly use anything in it (and already includes it).
  • Replace #include and #include with #include . AFAIK does not need getline, stoi or to_string defined in , so should suffice.

@cpplearner
S. B. Tam (cpplearner) requested a review from a team as a code owner September 25, 2022 00:18
@StephanTLavavej Stephan T. Lavavej (StephanTLavavej) added throughput Must compile faster format C++20/23 format labels Sep 25, 2022
strega-nil-ms
nicole mazzuca (strega-nil-ms) approved these changes Sep 26, 2022
View reviewed changes

@strega-nil-ms nicole mazzuca (strega-nil-ms) left a comment

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.

This seems completely reasonable to me; thanks!

barcharcraz
Charlie Barto (barcharcraz) approved these changes Sep 28, 2022
View reviewed changes

@barcharcraz Charlie Barto (barcharcraz) left a comment

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.

This is good, I actually prefer the _Widen_and_copy function to the transform

@StephanTLavavej

Stephan T. Lavavej (StephanTLavavej) commented Oct 11, 2022

Copy link
Copy Markdown
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej
Stephan T. Lavavej (StephanTLavavej) merged commit bbb07df into microsoft:main Oct 12, 2022
@StephanTLavavej

Stephan T. Lavavej (StephanTLavavej) commented Oct 12, 2022

Copy link
Copy Markdown
Member

Thanks for improving throughput! 🚀 🐇 🐆

@cpplearner
S. B. Tam (cpplearner) deleted the patch-1 branch May 18, 2023 23:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@StephanTLavavej Stephan T. Lavavej (StephanTLavavej) StephanTLavavej approved these changes
+2 more reviewers
@barcharcraz Charlie Barto (barcharcraz) barcharcraz approved these changes
@strega-nil-ms nicole mazzuca (strega-nil-ms) strega-nil-ms approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@StephanTLavavej Stephan T. Lavavej (StephanTLavavej)

Labels

format C++20/23 format throughput Must compile faster

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@cpplearner @StephanTLavavej @barcharcraz @strega-nil-ms