Page MenuHomePhabricator

[coroutines] Fix fallthrough diagnostics for coroutines
ClosedPublic

Authored by EricWF on May 24 2017, 6:34 PM.

Details

Summary

This patch fixes a number of issues with the analysis warnings emitted when a coroutine may reach the end of the function w/o returning.

  • Fix bug where coroutines with return_value are incorrectly diagnosed as missing co_return's.
  • Rework diagnostic message to no longer say "non-void coroutine", because that implies the coroutine doesn't have a void return type, which it might. In this case a non-void coroutine is one who's promise type does not contain return_void()

As a side-effect of this patch, coroutine bodies that contain an invalid coroutine promise objects are marked as invalid.

Diff Detail

Event Timeline

EricWF created this revision.May 24 2017, 6:34 PM
EricWF edited the summary of this revision. (Show Details)
GorNishanov added inline comments.May 24 2017, 6:40 PM
lib/Sema/AnalysisBasedWarnings.cpp
378

Is this check no longer needed because of the changes to AnalysisDeclContext.cpp?

At some point it was needed because we were emitting "return gro" to produce the immediate return value from the coroutine, but, we wanted to flag the case when the user forgot to add "co_return"

EricWF added inline comments.May 24 2017, 6:43 PM
lib/Sema/AnalysisBasedWarnings.cpp
378

I couldn't exactly figure out why this was no longer needed, but it appears we don't traverse those implicitly created return statements.

I made sure to test this particular part of the change to ensure it didn't regress behavior. If you look at the changes for coreturn.cpp you should see that case exercised.

This revision is now accepted and ready to land.May 24 2017, 6:46 PM
EricWF closed this revision.May 24 2017, 7:17 PM