Skip to content

fix(spanner): avoid evaluation-order issue in function arguments#9452

Merged
devbww merged 1 commit intogoogleapis:mainfrom
devbww:argument-evaluation-order
Jul 12, 2022
Merged

fix(spanner): avoid evaluation-order issue in function arguments#9452
devbww merged 1 commit intogoogleapis:mainfrom
devbww:argument-evaluation-order

Conversation

@devbww
Copy link
Copy Markdown
Contributor

@devbww devbww commented Jul 11, 2022 •
edited by coryan
Loading

The behavior of

  extern void f(std::unique_ptr<T>, T);
  std::unique_ptr<T> p = ...;
  f(std::move(p), *p);

is undefined, so introduce a sequence point between dereferencing
the unique_ptr and moving it.

This change is Reviewable

@devbww
fix(spanner): avoid evaluation-order issue in function arguments
bc6ad2b 
The behavior of
```
  extern void f(std::unique_ptr<T>, T);
  std::unique_ptr<T> p = ...;
  f(std::move(p), *p);
```
is undefined, so introduce a sequence point between dereferencing
the `unique_ptr` and moving it.
@product-auto-label product-auto-label Bot added the api: spanner Issues related to the Spanner API. label Jul 11, 2022
@google-cloud-cpp-bot
Copy link
Copy Markdown
Contributor

google-cloud-cpp-bot commented Jul 11, 2022

Google Cloud Build Logs
For commit: bc6ad2b8f9ddc5d851b9e8a7dd85291b676c415f

ℹ️ NOTE: Kokoro logs are linked from "Details" below.

coryan
coryan approved these changes Jul 11, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@coryan coryan left a comment

Choose a reason for hiding this comment

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

I wish there was clang-tidy check for these:

llvm/llvm-project#21193

@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 11, 2022 •
edited
Loading

Codecov Report

Merging #9452 (bc6ad2b) into main (5e15ecb) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #9452      +/-   ##
==========================================
- Coverage   94.63%   94.63%   -0.01%     
==========================================
  Files        1486     1486              
  Lines      136668   136670       +2     
==========================================
  Hits       129338   129338              
- Misses       7330     7332       +2
Impacted Files Coverage Δ
google/cloud/spanner/internal/connection_impl.cc 95.11% <100.00%> (+0.01%) ⬆️
...loud/bigtable/internal/connection_refresh_state.cc 97.22% <0.00%> (-2.78%) ⬇️
...le/cloud/internal/default_completion_queue_impl.cc 97.15% <0.00%> (-0.57%) ⬇️
...cloud/pubsub/internal/subscription_session_test.cc 97.98% <0.00%> (-0.17%) ⬇️
google/cloud/pubsub/subscriber_connection_test.cc 98.31% <0.00%> (+0.56%) ⬆️
...ud/spanner/integration_tests/client_stress_test.cc 86.18% <0.00%> (+0.65%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5e15ecb...bc6ad2b. Read the comment docs.

@devbww devbww marked this pull request as ready for review July 12, 2022 00:04
@devbww devbww requested a review from a team July 12, 2022 00:04
@devbww devbww merged commit ac964c9 into googleapis:main Jul 12, 2022
@devbww devbww deleted the argument-evaluation-order branch July 12, 2022 00:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coryan coryan coryan approved these changes

Assignees

No one assigned

Labels

api: spanner Issues related to the Spanner API.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@devbww @google-cloud-cpp-bot @coryan