Bug 226432 - Drop UncheckedCondition / UncheckedLock
Summary: Drop UncheckedCondition / UncheckedLock
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on: 226427 226428 226431
Blocks:
  Show dependency treegraph
 
Reported: 2021-05-29 20:17 PDT by Chris Dumez
Modified: 2021-05-30 13:37 PDT (History)
14 users (show)

See Also:


Attachments
Patch (18.66 KB, patch)
2021-05-30 11:12 PDT, Chris Dumez
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (18.21 KB, patch)
2021-05-30 11:56 PDT, Chris Dumez
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2021-05-29 20:17:59 PDT
Drop UncheckedCondition / UncheckedLock now that they are unused.
Comment 1 Chris Dumez 2021-05-30 11:12:18 PDT
Created attachment 430137 [details]
Patch
Comment 2 Darin Adler 2021-05-30 11:40:13 PDT
Comment on attachment 430137 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=430137&action=review

> Source/WTF/wtf/Lock.h:146
> +template<typename LockType> inline void assertIsHeld(const LockType& lock) { ASSERT_UNUSED(lock, lock.isLocked()); }

Why is this template needed? What classes is this used on besides Lock? Is a template the best way to do this, or would simple overloading be better?
Comment 3 Chris Dumez 2021-05-30 11:43:43 PDT
Comment on attachment 430137 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=430137&action=review

> Source/WTF/wtf/Condition.h:201
> +                    assertIsHeld(lock);

I had to add an assertIsHeld() here to make clang happy (even though I already marked the whole function as WTF_IGNORES_THREAD_SAFETY_ANALYSIS). I am not sure why but anyway. WTF::Condition is templated and supposed to work with all kind of locks (RecursiveLock comes to mind but this is not limited AFAIK).
As a result, I had to add a templated assertIsHeld() so that this would build no matter what lock type the condition is used with.
Comment 4 Chris Dumez 2021-05-30 11:53:07 PDT
(In reply to Chris Dumez from comment #3)
> Comment on attachment 430137 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=430137&action=review
> 
> > Source/WTF/wtf/Condition.h:201
> > +                    assertIsHeld(lock);
> 
> I had to add an assertIsHeld() here to make clang happy (even though I
> already marked the whole function as WTF_IGNORES_THREAD_SAFETY_ANALYSIS). I
> am not sure why but anyway. WTF::Condition is templated and supposed to work
> with all kind of locks (RecursiveLock comes to mind but this is not limited
> AFAIK).
> As a result, I had to add a templated assertIsHeld() so that this would
> build no matter what lock type the condition is used with.

Oh, based on something I just saw online, I may be able to add WTF_IGNORES_THREAD_SAFETY_ANALYSIS on the lambda. Let me try that. If that works, then I wouldn't need this new templated assertIsHeld at all.
Comment 5 Chris Dumez 2021-05-30 11:56:12 PDT
Created attachment 430140 [details]
Patch
Comment 6 EWS 2021-05-30 13:36:06 PDT
Committed r278257 (238294@main): <https://commits.webkit.org/238294@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 430140 [details].
Comment 7 Radar WebKit Bug Importer 2021-05-30 13:37:17 PDT
<rdar://problem/78667488>