Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clang-tidy: performance-noexcept-move-constructor #7393

Conversation

derekargueta
Copy link
Member

Description: performance-noexcept-move-constructor checks for move constructors that aren't declared noexcept, the reason being that STL containers will use the copy constructor for operations like emplace_back if the move constructor is not declared noexcept due to exception safety guarantees.
http://www.hlsl.co.uk/blog/2017/12/1/c-noexcept-and-move-constructors-effect-on-performance-in-stl-containers
https://stackoverflow.com/a/32225460
Risk Level: low
Testing: existing
Docs Changes: N/A
Release Notes: N/A

Relates to #4863

Signed-off-by: Derek Argueta dereka@pinterest.com

Signed-off-by: Derek Argueta <dereka@pinterest.com>
Signed-off-by: Derek Argueta <dereka@pinterest.com>
@derekargueta
Copy link
Member Author

/retest

@repokitteh-read-only
Copy link

🔨 rebuilding ci/circleci: coverage (failed build)

🐱

Caused by: a #7393 (comment) was created by @derekargueta.

see: more, trace.

@htuch htuch self-assigned this Jun 25, 2019
@lambdai
Copy link
Contributor

lambdai commented Jun 25, 2019

This is subtle. TIL

@derekargueta
Copy link
Member Author

hmm coverage keeps core-dumping, seemingly on IpVersions/ProxyFilterIntegrationTest.RemoveHostViaTTL/IPv4

external/bazel_tools/tools/test/test-setup.sh: line 310: 2368 Aborted (core dumped) "${TEST_PATH}" "$@" 2>&1

Not seeing anything on master relating to a "fix" for this, gonna give it one more retest, and if that fails will merge master. BTW is there a way to access the error log file from Circle CI? I haven't figured that out yet for looking at the exact error log.

@derekargueta
Copy link
Member Author

/retest

@repokitteh-read-only
Copy link

🔨 rebuilding ci/circleci: coverage (failed build)

🐱

Caused by: a #7393 (comment) was created by @derekargueta.

see: more, trace.

@mattklein123
Copy link
Member

@derekargueta that test flake is fixed in #7361

Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

Nice, thanks!

@htuch htuch merged commit e4bdb73 into envoyproxy:master Jun 25, 2019
@derekargueta derekargueta deleted the dereka/clang-tidy-move-constructor-noexcept branch June 26, 2019 00:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants