1447914 - Change FunctionCall::getAtomName() to appendName()

Status: RESOLVED FIXED

(Core :: XSLT, enhancement, P2)

Description

[Nicholas Nethercote [inactive]]

getAtomName() is only used in one place -- FunctionCall::toString() -- where
the returned atom is appended to a string. This patch changes it to
appendName(), which does the appending itself. This avoids a bunch of manual
atom refcounting and the possibility of failure.

MozReview-Commit-ID: I75rO1Bv6Y8

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: Change FunctionCall::getAtomName() to appendName()

Comment 2

[Eric Rahm [:erahm]]

Comment on attachment 8961297 [details] [diff] [review]
Change FunctionCall::getAtomName() to appendName()

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

Nice cleanup, just a minor nit.

::: dom/xslt/xpath/txExpr.h
@@ +139,5 @@
>  
>  #ifdef TX_TO_STRING
>  #define TX_DECL_TOSTRING \
>      void toString(nsAString& aDest) override;
>  #define TX_DECL_GETNAMEATOM \

nit: Can you update the macro name?

@@ +140,5 @@
>  #ifdef TX_TO_STRING
>  #define TX_DECL_TOSTRING \
>      void toString(nsAString& aDest) override;
>  #define TX_DECL_GETNAMEATOM \
> +    void appendName(nsAString& aStr) override;

nit: Maybe name the param `aDest` to match `toString`.

Comment 3

[Nicholas Nethercote [inactive]]

> >  #define TX_DECL_GETNAMEATOM \
> 
> nit: Can you update the macro name?

Good catch.

> > +    void appendName(nsAString& aStr) override;
> 
> nit: Maybe name the param `aDest` to match `toString`.

Sure.

Comment 4

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3145ee5bb85c8cf09b65d88e86e8aba2949efac4
Bug 1447914 - Change FunctionCall::getAtomName() to appendName(). r=froydnj

Comment 5

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/3145ee5bb85c