[Webkit-unassigned] [Bug 65414] DequeIterator lacks default constructor

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Sep 7 00:57:59 PDT 2011


https://bugs.webkit.org/show_bug.cgi?id=65414





--- Comment #5 from Balazs Kelemen <kbalazs at webkit.org>  2011-09-07 00:57:59 PST ---
(In reply to comment #4)
> (From update of attachment 106405 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=106405&action=review
> 
> > Source/JavaScriptCore/wtf/Deque.h:598
> > +        , m_next(0)
> > +        , m_previous(0)
> 
> I’m not sure I understand the full rationale for setting these in a debug build but not a release build. Will this help us catch mistakes we would otherwise miss? Or perhaps it will hide mistakes we might otherwise catch.

This is just to pass the assertions in removeFromIteratorList with an empty iterator:

if (!m_deque) {
    ASSERT(!m_next);
    ASSERT(!m_previous);
}

Maybe ASSERT_DISABLED would be the appropriate condition.

> 
> > Source/JavaScriptCore/wtf/MainThread.cpp:-213
> > -        // We must redefine 'i' each pass, because the itererator's operator= 
> > -        // requires 'this' to be valid, and remove() invalidates all iterators
> 
> The substance of this comment still seems correct, even if the word redefine is no longer accurate, so it’s not clear to me that it’s good to remove it.

I think this comment is not valid since http://trac.webkit.org/changeset/92050 - my bad to not updated this call site - so we can remove it.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.


More information about the webkit-unassigned mailing list