Bug 211608 – WTF headers should compile independently

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Don Olmstead]

Some WTF headers do not compile on their own.

These issues were found when creating a file that just includes the header file directly.

[Don Olmstead]

Created attachment 398829 [details]
Patch

[Don Olmstead]

Created attachment 398834 [details]
Patch

[Don Olmstead]

Created attachment 398836 [details]
Patch

[Ross Kirsling]

Comment on attachment 398836 [details]
Patch

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

> Source/WTF/wtf/LoggingHashMap.h:116
>      auto keys() { return m_map.keys(); }
> -    const auto keys() const { return m_map.keys(); }
> +    auto keys() const { return m_map.keys(); }
>      auto values() { return m_map.values(); }
> -    const auto values() const { return m_map.values(); }
> +    auto values() const { return m_map.values(); }

Whoops; it's good to mark these as const methods but these really are two different return types for each.

[Don Olmstead]

Comment on attachment 398836 [details]
Patch

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

>> Source/WTF/wtf/LoggingHashMap.h:116
>> +    auto values() const { return m_map.values(); }
> 
> Whoops; it's good to mark these as const methods but these really are two different return types for each.

In file included from WTF\DerivedSources\LoggingHashMapInclude.cpp:2:
..\..\Source\WTF\wtf\LoggingHashMap.h(114,5): warning: 'const' type qualifier on return type has no effect [-Wignored-qualifiers]
    const auto keys() const { return m_map.keys(); }
    ^~~~~~
..\..\Source\WTF\wtf\LoggingHashMap.h(116,5): warning: 'const' type qualifier on return type has no effect [-Wignored-qualifiers]
    const auto values() const { return m_map.values(); }
    ^~~~~~

[Yusuke Suzuki]

Comment on attachment 398836 [details]
Patch

r=me

[EWS]

Committed r261384: <https://trac.webkit.org/changeset/261384>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 398836 [details].

[Radar WebKit Bug Importer]

<rdar://problem/63022258>

[Darin Adler]

Comment on attachment 398836 [details]
Patch

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

> Source/WTF/wtf/IndexSparseSet.h:33
> +template<typename KeyTypeArg, typename ValueTypeArg>
> +struct KeyValuePair;

I would have written:

template<typename, typename> struct KeyValuePair;

I try to go minimal in forward declarations, repeating the minimum.

> Source/WTF/wtf/LoggingHashMap.h:56
> +    typedef typename HashMap::MappedTraits::PeekType MappedPeekType;

Would be nice to do "using" here. Also, can this live in the private part that already exists below instead of dropping into private and then back to public.

> Source/WTF/wtf/RunLoopTimer.h:36
> +#if USE(CF)
>  #include <wtf/RetainPtr.h>
> +#endif

Surprised we don’t get a warning about "private:" at the end of a class definition with nothing after it before the closing brace.

> Source/WTF/wtf/persistence/PersistentCoder.h:31
> +template <class T>
> +class Optional;

template<typename> class Optional;

But also, maybe use <wtf/Forward.h> instead?

> Source/WTF/wtf/text/NullTextBreakIterator.h:23
> +#include <wtf/text/StringView.h>

Can we include less, maybe just Forward.h? Or is that not enough?

> Source/WTF/wtf/text/StringOperators.h:24
> +#include <wtf/text/WTFString.h>

Can we include less, maybe just Forward.h? I’m really concerned about include cycles here -- this seems like a bad change!

> Source/WTF/wtf/text/StringToIntegerConversion.h:29
> +#include <unicode/utypes.h>
> +#include <wtf/ASCIICType.h>

Seems like ASCIICType.h should have been sufficient on its own; didn't need  to add both.