fix new GCC warnings #34
Conversation
There was a problem hiding this comment.
@mingtaoy has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
fizz/record/RecordLayer.cpp
Outdated
| @@ -89,10 +89,10 @@ Param parse<ServerHello>(Buf handshakeMsg, Buf original) { | |||
| hrr.extensions = std::move(shlo.extensions); | |||
|
|
|||
| hrr.originalEncoding = std::move(original); | |||
| return std::move(hrr); | |||
| return hrr; | |||
There was a problem hiding this comment.
You cannot elide the move here. NRVO doesn't apply since there are two branches that return different types. We need the std::move() here to explicitly invoke the variant constructor.
There was a problem hiding this comment.
|
@neheb has updated the pull request. Re-import the pull request |
error: redundant move in return statement [-Werror=redundant-move]
|
@neheb has updated the pull request. Re-import the pull request |
|
Got rid of all the changes in fizz/record/RecordLayer.cpp . Travis seems happy. Not the case locally. I guess this is good enough. edit: I should clarify that this only fails when passing -Werror -Wall |
There was a problem hiding this comment.
@mingtaoy has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@neheb the issue with implicit constructors that we run into in RecordLayer.cpp is discussed in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87300. Even though it's correct with the new standard, I don't think we want to simply remove the |
|
Well, fizz does not pass -Werror, so I think it's fine. |
|
Merged. Thanks for your contribution, @neheb! |
error: redundant move in return statement [-Werror=redundant-move]