836920 - Move Optional to BindingDeclarations.h

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Right now it's in BindingUtils.h, but we no longer include that in binding headers.  So if your interface name starts with a letter before 'P' (so that you come before PrimitiveConversions.h in the include list in the .cpp) and there's a dictionary involved (which uses Optional<> members in the header), you lose.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Move dom::Optional<> and dom::Sequence to BindingDeclarations.h so using dictionaries in interfaces whose name comes before 'PrimitiveConversions' alphabetically does not break.

Rik, give this a try?

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 708799 [details] [diff] [review]
Move dom::Optional<> and dom::Sequence to BindingDeclarations.h so using dictionaries in interfaces whose name comes before 'PrimitiveConversions' alphabetically does not break.

Actually, this doesn't build; need to move FakeDependentString too, and check whether that pulls in anything too evil.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Yeah, that doesn't work because xpcpublic.h includes BindingDeclarations.h and xpcshell.cpp includes xpcpublic.h.

Let me play with this stuff a bit more, I guess...

Comment 4

[Boris Zbarsky [:bzbarsky]]

Yeah, and various other external-string code (e.g. TestAppShellSteadyState.cpp) includes nsIDocument.h, which includes DocumentBinding.h, which includes BindingDeclarations.h.

And the problem there, of course, is that FakeDependentString relies on the internal version of nsDependentString...

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Move dom::Optional<> and dom::Sequence to BindingDeclarations.h so using dictionaries in interfaces whose name comes before 'PrimitiveConversions' alphabetically does not break. the code is just moving except the Optional<nsAString>::operator=

Well, this builds.  Peter, what do you think?

Comment 6

[Peter Van der Beken [:peterv]]

Comment on attachment 708883 [details] [diff] [review]
Move dom::Optional<> and dom::Sequence to BindingDeclarations.h so using dictionaries in interfaces whose name comes before 'PrimitiveConversions' alphabetically does not break.  the code is just moving except the Optional<nsAString>::operator=

Review of attachment 708883 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bindings/BindingDeclarations.h
@@ +214,5 @@
>  };
>  
> +// Class for representing optional arguments.
> +template<typename T>
> +class Optional {

While you're here, want to move {'s on their own line, following the style of this file?

Comment 7

[Boris Zbarsky [:bzbarsky]]

Done and https://hg.mozilla.org/integration/mozilla-inbound/rev/88601eca5f69

Then https://hg.mozilla.org/integration/mozilla-inbound/rev/830784a5e43e to fix warnings-as-errors bustage.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/88601eca5f69
https://hg.mozilla.org/mozilla-central/rev/830784a5e43e