refactor: Clean up the Room::compute_display_name() method#4306
Conversation
jmartinesp
left a comment
jmartinesp
left a comment
There was a problem hiding this comment.
LGTM, thanks. Just a question about the weird drop issue you mentioned.
| // We can't directly compute the display name from the summary here because Rust | ||
| // thinks that the `inner` lock is still held even if we explicitly call `drop()` | ||
| // on it. So we introduced the DisplayNameOrSummary type and do the computation in | ||
| // two steps. |
There was a problem hiding this comment.
Weird, but OK. Is this some kind of bug in the compiler?
There was a problem hiding this comment.
🤷 I have not investigated further, we had the same situation previously as well, but with some early returns.
I have somewhere seen that it's about thread local storage the lock uses, but no idea realistically.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4306 +/- ##
==========================================
- Coverage 85.11% 85.08% -0.04%
==========================================
Files 274 274
Lines 30184 30191 +7
==========================================
- Hits 25692 25688 -4
- Misses 4492 4503 +11 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
2 participants
This method has grown organically into a bit of a beast. Let's try to make it a bit easier to understand in preparation for MSC4171 🫡.