Fix bug of using GenericParamList after it's std::move'd #62656
Conversation
| @@ -4504,6 +4504,17 @@ class DeclDeserializer { | |||
|
|
|||
| outerParams = genericParams; | |||
| } | |||
| #ifndef NDEBUG | |||
| // Use outerParams before it will be moved with std::move(outerParams) | |||
| unsigned paramCount = 0; | |||
There was a problem hiding this comment.
Is that crashing somewhere? I think that although this is makes when thinking about move semantics, this is not a problem in this case because outterParams is a pointer GenericParamList * and std::move of a pointer like trivial types such as int just defaults to copy so state of outerParams is not changed. The question would be why would we std::move a pointer, but I guess the answer is to match ctx.evaluator.cacheOutput signature with takes an r-value of request result value...
There was a problem hiding this comment.
Is that crashing somewhere?
No, it's not.
There was a problem hiding this comment.
The question would be why would we std::move a pointer, but I guess the answer is to match
ctx.evaluator.cacheOutputsignature with takes anr-valueof request result value...
That's common case why std::move is used.
What would you suggest? Should I close PR?
There was a problem hiding this comment.
Let's wait for @CodaFi feedback :)
| paramList != nullptr; | ||
| paramList = paramList->getOuterParameters()) { | ||
| paramCount += paramList->size(); | ||
| } | ||
| assert(paramCount == |
There was a problem hiding this comment.
This assert should be moved as well, since it's the reason we're computing paramCount here at all.
|
|
Should this PR be closed as irrelevant? |
No description provided.