1235261 - Remove AutoFallibleTArray

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Comment 1

[Biru [:poiru]]

Attachment: Part 1: Rename nsAutoTArray to AutoTArray

Comment 2

[Biru [:poiru]]

Attachment: Part 2: Switch some uses of AutoFallibleTArray to AutoTArray

This changes some function signatures to take a nsTArray<T>& instead of a
FallibleTArray<T>& because AutoTArray does not inherit from FallibleTArray.

This is effectively a no-op because the affected array operations already use
`mozilla::fallible`.

Comment 3

[Biru [:poiru]]

Attachment: Part 3: Switch remaining uses of AutoFallibleTArray to AutoTArray

This is effectively a no-op because the affected array operations already use
`mozilla::fallible`.

Comment 4

[Biru [:poiru]]

Attachment: Part 4: Remove AutoFallibleTArray

Comment 5

[Biru [:poiru]]

Attachment: Part 5: Merge nsAutoArrayBase into AutoTArray

nsAutoArrayBase is no longer needed because AutoTArray is its only subclass.

Comment 6

[Biru [:poiru]]

Attachment: Part 6: Rename AutoInfallibleTArray to AutoTArray

While we're here, lets get rid the useless AutoInfallibleTArray alias of AutoTArray.

Comment 7

[Biru [:poiru]]

Attachment: Part 7: Remove AutoInfallibleTArray

Comment 8

[Nathan Froyd [:froydnj]]

Comment on attachment 8702132 [details] [diff] [review]
Part 1: Rename nsAutoTArray to AutoTArray

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

r=me assuming this was done with the moral equivalent of |perl -pi -e 's/nsAutoTArray/AutoTArray/g'|

Comment 9

[Nathan Froyd [:froydnj]]

Comment on attachment 8702134 [details] [diff] [review]
Part 3: Switch remaining uses of AutoFallibleTArray to AutoTArray

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

::: dom/bindings/BindingUtils.h
@@ +2087,5 @@
>  // Class for simple sequence arguments, only used internally by codegen.
>  namespace binding_detail {
>  
>  template<typename T>
> +class AutoSequence : public AutoTArray<T, 16>

This is the only bit that worries me, as keeping a fallible-methods-only class around makes life simpler for the code generator (fewer ways to screw up the code generator for infallible stuff--and for whatever uses mozilla::dom::Sequence).

Can you file a followup bug, cc bz and ehsan, and ni? bz about keeping something like that around?  Ehsan has said that he's not concerned about this, but bz might have a separate opinion as the bindings expert.

Comment 10

[Nathan Froyd [:froydnj]]

Comment on attachment 8702137 [details] [diff] [review]
Part 6: Rename AutoInfallibleTArray to AutoTArray

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

r=me assuming this was done with the moral equivalent of |perl -pi -e 's/AutoInfallibleTArray/AutoTArray/g'|.

Comment 11

[Nathan Froyd [:froydnj]]

(In reply to Nathan Froyd [:froydnj] from comment #9)
> Can you file a followup bug, cc bz and ehsan, and ni? bz about keeping
> something like that around?  Ehsan has said that he's not concerned about
> this, but bz might have a separate opinion as the bindings expert.

Or do the ni? here...either way, the landing of these patches should wait until that discussion is resolved.

Comment 12

[Biru [:poiru]]

Right now, AutoSequence inherits from AutoFallibleTArray, which means that all allocating member functions must be used together with `mozilla:fallible`. If the `mozilla:fallible` parameter is not used, the compiler will emit an error.

This bug wants to remove AutoFallibleTArray and change AutoSequence to inherit from nsAutoTArray instead. This means that calls that already use `mozilla:fallible` will continue to work the same, but the compiler will no longer enforce the presence of `mozilla:fallible`. Calls that omit it will be infallible. Note that the few uses of AutoSequence in Codegen.py already use `mozilla::fallible` where appropriate.

bz, do you think this is an issue? (I can't ni? bz at the moment so just CCing him.)

Comment 13

[Boris Zbarsky [:bzbarsky]]

So the situation is that all existing dom::binding_detail::AutoSequence consumers (not just in codegen but everywhere, though I kinda hope nothing outside codegen uses it) currently pass mozilla::fallible, right?  So we would only need to worry about new consumers, which should be carefully thinking about their stuff anyway?

If so, I'm probably OK with this.

Comment 14

[Biru [:poiru]]

(In reply to Boris Zbarsky [:bz] (Vacation until Jan 4) from comment #13)
> So the situation is that all existing dom::binding_detail::AutoSequence
> consumers (not just in codegen but everywhere, though I kinda hope nothing
> outside codegen uses it) currently pass mozilla::fallible, right?  So we
> would only need to worry about new consumers, which should be carefully
> thinking about their stuff anyway?

Yes and yes.

Comment 15

[Biru [:poiru]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a2dab8605ea3

Comment 16

[Biru [:poiru]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/064969357fc94da487ada47c08249995b28cc560
Bug 1235261 - Part 1: Rename nsAutoTArray to AutoTArray. r=froydnj

https://hg.mozilla.org/integration/mozilla-inbound/rev/df70e89669da3970f0634fcf695d7ee8931024c2
Bug 1235261 - Part 2: Switch some uses of AutoFallibleTArray to AutoTArray. r=froydnj

https://hg.mozilla.org/integration/mozilla-inbound/rev/0ff0fa6fe81fd3cbed9b5d63d9f5326f844efcc8
Bug 1235261 - Part 3: Switch remaining uses of AutoFallibleTArray to AutoTArray. r=froydnj

https://hg.mozilla.org/integration/mozilla-inbound/rev/c50bb8ed41966cb782d9f56a90544b97ffccbcf8
Bug 1235261 - Part 4: Remove AutoFallibleTArray. r=froydnj

https://hg.mozilla.org/integration/mozilla-inbound/rev/32b61df131426410d33928fd8e425c2c59d09661
Bug 1235261 - Part 5: Merge nsAutoArrayBase into AutoTArray. r=froydnj

https://hg.mozilla.org/integration/mozilla-inbound/rev/467d945426bb58d16648f25002049d16af8a5569
Bug 1235261 - Part 6: Rename AutoInfallibleTArray to AutoTArray. r=froydnj

https://hg.mozilla.org/integration/mozilla-inbound/rev/d66c3f19a2104e88e6a1337277819e51c16d4d98
Bug 1235261 - Part 7: Remove AutoInfallibleTArray. r=froydnj

Comment 17

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/d77374bc1315 for cpptest bustage, https://treeherder.mozilla.org/logviewer.html#?job_id=20789235&repo=mozilla-inbound

Comment 18

[Biru [:poiru]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/75dfe10ec44a88bda2d9721d269ddf2429ac5426
Bug 1235261 - Part 1: Rename nsAutoTArray to AutoTArray. r=froydnj

https://hg.mozilla.org/integration/mozilla-inbound/rev/84aba71492334d548490820649fb8828ba38161e
Bug 1235261 - Part 2: Switch some uses of AutoFallibleTArray to AutoTArray. r=froydnj

https://hg.mozilla.org/integration/mozilla-inbound/rev/0feb7b5268a81bd2bc474baad697e1c627f0a9f7
Bug 1235261 - Part 3: Switch remaining uses of AutoFallibleTArray to AutoTArray. r=froydnj

https://hg.mozilla.org/integration/mozilla-inbound/rev/c824053f838d473380a6bf2d39a0922785ed16aa
Bug 1235261 - Part 4: Remove AutoFallibleTArray. r=froydnj

https://hg.mozilla.org/integration/mozilla-inbound/rev/0862175edb88673e1207e43e166bc3ed9fd9ed54
Bug 1235261 - Part 5: Merge nsAutoArrayBase into AutoTArray. r=froydnj

https://hg.mozilla.org/integration/mozilla-inbound/rev/e9a8abd870b906dd4e1b63784ea96d07579122b3
Bug 1235261 - Part 6: Rename AutoInfallibleTArray to AutoTArray. r=froydnj

https://hg.mozilla.org/integration/mozilla-inbound/rev/32bacd86298aa5ceca6675fd3b40cff2394124e7
Bug 1235261 - Part 7: Remove AutoInfallibleTArray. r=froydnj

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/75dfe10ec44a
https://hg.mozilla.org/mozilla-central/rev/84aba7149233
https://hg.mozilla.org/mozilla-central/rev/0feb7b5268a8
https://hg.mozilla.org/mozilla-central/rev/c824053f838d
https://hg.mozilla.org/mozilla-central/rev/0862175edb88
https://hg.mozilla.org/mozilla-central/rev/e9a8abd870b9
https://hg.mozilla.org/mozilla-central/rev/32bacd86298a