Skip to content

Update json_value.cpp, fixing up spelling mistakes (boundry to boundary) #1277

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

Update json_value.cpp, fixing up spelling mistakes (boundry to boundary) #1277

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0707eef
Update json_value.cpp
estrm Mar 17, 2021
3345a5c
Update json_value.cpp
estrm Mar 18, 2021
8780fb0
Update json_value.cpp
estrm Mar 18, 2021
ea66d98
Update json_value.cpp
estrm Mar 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/lib_json/json_value.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1398,11 +1398,11 @@ String Value::Comments::get(CommentPlacement slot) const {
}

void Value::Comments::set(CommentPlacement slot, String comment) {
if (slot >= CommentPlacement::numberOfCommentPlacement) {
Copy link
Contributor

@BillyDonahue BillyDonahue Mar 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This is not the code I suggested. Can you do it again?
The braces are not needed for the early return.
And the ptr_ assignment needs to happen whether we allocated it in this call or not.

Copy link
Author

Choose a reason for hiding this comment

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

Yes. If return directly while slot >= CommentPlacement::numberOfCommentPlacement, the CI told that Some checks were not successful.

Copy link
Contributor

@BillyDonahue BillyDonahue Mar 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I'd be interested in the CI failure.
The problem with this iteration of the code is that the assignment to (*ptr_)[slot] doesn't happen if there wasn't already a ptr_ when the function started. That's not going to work.
But it isn't because of the slot range check. It's because line 1406 should be 2 lines lower, outside the if block.

I'd like to see what happens if you literally take the code I gave earlier, token for token, and paste it in.

return;
}
if (!ptr_) {
ptr_ = std::unique_ptr<Array>(new Array());
}
// check comments array boundry.
if (slot < CommentPlacement::numberOfCommentPlacement) {
(*ptr_)[slot] = std::move(comment);
}
}
Expand Down