910829 - Various cleanups

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Dan Gohman [:sunfish]]

Attachment: redundant-null-checks.patch

The following is a series of miscellaneous cleanups.

Comment 1

[Dan Gohman [:sunfish]]

Attachment: anonymous-namespaces.patch

Comment 2

[Dan Gohman [:sunfish]]

Attachment: ra-cleanup.patch

Comment 3

[Dan Gohman [:sunfish]]

Attachment: ra-debug-stderr.patch

Comment 4

[Dan Gohman [:sunfish]]

Attachment: ra-debug-phi-position.patch

Comment 5

[Dan Gohman [:sunfish]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ef553af7c130
https://hg.mozilla.org/integration/mozilla-inbound/rev/c0104d0c88a1

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ef553af7c130
https://hg.mozilla.org/mozilla-central/rev/c0104d0c88a1

Comment 7

[Nicholas Nethercote [inactive]]

> anonymous-namespaces.patch

What's the benefit of this?

Comment 8

[Dan Gohman [:sunfish]]

(In reply to Nicholas Nethercote [:njn] from comment #7)
> > anonymous-namespaces.patch
> 
> What's the benefit of this?

Just general code cleanliness. The main measurable effect of this patch is it reduces the number of global symbols among the .o files of the js engine by 3.8% (the number of global symbols in libmozjs-* is unchanged, due to the use of symbol visibilities). It's not important, so I don't mind if such patches aren't accepted.

Comment 9

[Nicolas B. Pierron [:nbp]]

(In reply to Dan Gohman [:sunfish] from comment #8)
> (In reply to Nicholas Nethercote [:njn] from comment #7)
> > > anonymous-namespaces.patch
> > 
> > What's the benefit of this?
> 
> Just general code cleanliness. The main measurable effect of this patch is
> it reduces the number of global symbols among the .o files of the js engine
> by 3.8% (the number of global symbols in libmozjs-* is unchanged, due to the
> use of symbol visibilities). It's not important, so I don't mind if such
> patches aren't accepted.

I think it would be good to use an empty macro as the name of the anonymous namespaces, such as people who complained about the ability to debug these functions can still set the macro to get a name in their debuggers.

Comment 10

[Jeff Walden [:Waldo]]

Interesting idea.  But (assuming I understand it) wouldn't it founder when code outside the "anonymous" namespace needs to call a method defined inside it?

namespace ANONYMOUS {

static bool
InitFoo()
{
  return true;
}

}

namespace other_thing {

bool
Exported()
{
  return InitFoo(); // doesn't work if ANONYMOUS expands to anything
}

}

I'm not sure how common this is, but there's some Mozilla code fitting this pattern now.

Comment 11

[Nicolas B. Pierron [:nbp]]

(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #10)
> bool
> Exported()
> {
>   return InitFoo(); // doesn't work if ANONYMOUS expands to anything
> }

#define USING_NS_ANONYMOUS using namespace anonymous

Comment 12

[Nicholas Nethercote [inactive]]

> #define USING_NS_ANONYMOUS using namespace anonymous

Please, please, no.

Comment 13

[Nicolas B. Pierron [:nbp]]

Another option would be:

#define ANONYMOUS            \
  anonymous {}               \
  using namespace anonymous; \
  namespace anonymous

in which case one can transparently use the named-anonymous namespace as if it was a real anonymous namespace.

(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #10)
> namespace ANONYMOUS {
> 
> static bool
> InitFoo()
> {
>   return true;
> }
> 
> }

Comment 14

[Brian Hackett [Laid off!]]

Comment on attachment 797398 [details] [diff] [review]
ra-cleanup.patch

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

Sorry for the delay.

Comment 15

[Dan Gohman [:sunfish]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f2d3df7c6056
https://hg.mozilla.org/integration/mozilla-inbound/rev/81be63a2e9b1
https://hg.mozilla.org/integration/mozilla-inbound/rev/f33f92d540df

Comment 16

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/f2d3df7c6056
https://hg.mozilla.org/mozilla-central/rev/81be63a2e9b1
https://hg.mozilla.org/mozilla-central/rev/f33f92d540df