Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
samples(storagecontrol): add storagecontrol folder samples #14332
samples(storagecontrol): add storagecontrol folder samples #14332
Changes from 8 commits
453c202
c889a9c
cdd2ad1
7e55a5f
28aaec7
33e33f5
90d2539
f1f7f92
d8e1007
4f48047
bd523a0
bdce3e2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling
std::move(client)
here invalidates the client and causes the crash.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... Normally
clang-tidy
should give you a warning about that. I wonder why it didn't... Ah, because generated libraries do not get aclang-tidy
build:google-cloud-cpp/ci/lib/shard.sh
Lines 89 to 90 in 54274c0
After we merge this PR, we should discuss with @scotthart and @dbolduc if they want to enable
clang-tidy
for any generated libraries with manual examples.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed. But out of curiosity why does this crash?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are using the move-constructor: note that
std::move()
by itself just says "please use the move constructor if available". There is a move constructor available: the compiler generated one, which performance member-by-member move construction. Oh, and because your lambda (now a function) consumes theclient
parameter by-value the constructor is called.Move constructors (as the name implies) move or steal the contents of a the source object into the destination object 1. That leaves the source object in an undetermined state 2. Calling member functions that have pre-conditions may result in UB (undefined behavior). In this case the contents are a
std::shared_ptr<>
, the shared pointer move constructor leaves the source with anullptr
and dereferencing anullptr
causes a crash.Footnotes
Well, move constructors can do whatever they want. They can behave like copy constructors. But by convention they move / steal the contents. ↩
Again, technically move constructors are required to leave the source object in a "consistent but undetermined state". It could be that the source object is left in a state where it is usable, but in general one should assume move constructors leave you with an object where only functions without preconditions (e.g. assigning to the object) work. ↩