1602425 - Remove xpidl [array] use in calIRecurrenceRule.idl

Status: RESOLVED FIXED

(Calendar :: General, task)

Comment 1

[Ben Campbell]

Attachment: 1602425-xpidl-array-removal-in-calIRecurrenceRule-1.patch

Mostly straightforward, but I took the opportunity to replace some serious preprocessor macro voodoo with a lookup table, for getComponent()/setComponent(). Hopefully it makes the code easier to read.

No try run yet (I'll do one for all the calendar stuff together), but a local run passes all the unit tests that use getComponent()/setComponent(), so I'm pretty confident it's sound.

Comment 2

[Geoff Lankow (:darktrojan)]

Comment on attachment 9117486 [details] [diff] [review]
1602425-xpidl-array-removal-in-calIRecurrenceRule-1.patch

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

Hooray for voodoo reduction. Although I'm sure it didn't look as bad before the formatter got hold of it.

::: calendar/base/backend/libical/calRecurrenceRule.cpp
@@ +261,5 @@
> +                                nsTArray<int16_t> &aValues) {
> +  aValues.ClearAndRetainStorage();
> +  // Look up the array for this component type.
> +  for (int i = 0; recurrenceTable[i].name; ++i) {
> +    auto const &r = recurrenceTable[i];

I'd prefer `r` to be called `row`, assuming that's what it's short for. Same below.

Comment 3

[Ben Campbell]

Attachment: 1602425-xpidl-array-removal-in-calIRecurrenceRule-2.patch

Renamed r => row.
Good point about the reformat changing the old macro voodoo. I wondered why it was arranged the way it was!

Anyway, a new try build which covers this patch (and the other two calendar ones):
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=5cb6d13351199bb8ebbf8a7c2befc2d5c997a16a

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/e71712f49a91
Remove xpidl [array] use in calIRecurrenceRule. r=darktrojan