This change avoids lots of false positives for Coverity's CHECKED_RETURN
warning, caused by NS_WARN_IF's current use in both statement-style and
expression-style.
In the case where the code within the NS_WARN_IF has side-effects, I made the
following change.
> NS_WARN_IF(NS_FAILED(FunctionWithSideEffects()));
> -->
> Unused << NS_WARN_IF(NS_FAILED(FunctionWithSideEffects()));
In the case where the code within the NS_WARN_IF lacks side-effects, I made the
following change.
> NS_WARN_IF(!condWithoutSideEffects);
> -->
> NS_WARNING_ASSERTION(condWithoutSideEffects, "msg");
This has two improvements.
- The condition is not evaluated in non-debug builds.
- The sense of the condition is inverted to the familiar "this condition should
be true" sense used in assertions.
A common variation on the side-effect-free case is the following.
> nsresult rv = Fn();
> NS_WARN_IF_(NS_FAILED(rv));
> -->
> DebugOnly<nsresult rv> = Fn();
> NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "Fn failed");
--HG--
extra : rebase_source : 58788245021096efa8372a9dc1d597a611d45611
The new name makes the sense of the condition much clearer. E.g. compare:
NS_WARN_IF_FALSE(!rv.Failed());
with:
NS_WARNING_ASSERTION(!rv.Failed());
The new name also makes it clearer that it only has effect in debug builds,
because that's standard for assertions.
--HG--
extra : rebase_source : 886e57a9e433e0cb6ed635cc075b34b7ebf81853
nsThreadManager::get() can return a reference. This lets us remove some
redundant assertions.
nsThreadArray elements can be NotNull<>s.
--HG--
extra : rebase_source : fd49010167101bc15f7f6d01bf95fd63b81d60fb
Removed '#ifndef XPCOM_GLUE' that blocked usage of thread-safe ref-counting
from XPCOM_GLUE code.
MozReview-Commit-ID: Hm0rdTKK46l
--HG--
extra : rebase_source : cd2779d0b39c5319cfdb5d12e8f4ac6f38945e7b
The patch is generated from following command:
rgrep -l unused.h|xargs sed -i -e s,mozilla/unused.h,mozilla/Unused.h,
MozReview-Commit-ID: AtLcWApZfES
--HG--
rename : mfbt/unused.h => mfbt/Unused.h
This is the same basic idea as NS_IMPL_RELEASE_WITH_DESTROY. I need
this because I am making XPCNativeInterface refcounted, and it uses
some weird placement new stuff requiring a special function to
deallocate the object. (It does this to store an array of arbitrary
length inline, presumably for some sort of time or space reason.)
MozReview-Commit-ID: 5I7BgY6YlLl
We have a number of nsTHashtable<nsPtrHashKey<T>> instantiations, mostly
from IPDL-generated code. There's no reason in principle that all of
these instantiations couldn't share code, since they're all storing POD
entries of the same size. Let's specialize nsTHashtable for such types,
providing a thin layer over a hashtable that stores void pointers. This
change saves about 90K of space (!) on x86-64 Linux.
The standard placement new function is declared to not throw, which
means that, per spec, a null check on its result is required. There are
a number of places throughout xpcom/ where we know that we are passing
non-null pointers to placement new (and receiving them as a return
value), and we are therefore doing useless work performing these null
checks.
Therefore, we should be using an operator new overload that doesn't
require the null check. MFBT has just such an overload, so use that.
This is the same basic idea as NS_IMPL_RELEASE_WITH_DESTROY. I need
this because I am making XPCNativeInterface refcounted, and it uses
some weird placement new stuff requiring a special function to
deallocate the object. (It does this to store an array of arbitrary
length inline, presumably for some sort of time or space reason.)
MozReview-Commit-ID: 5I7BgY6YlLl
mozilla::function involves an allocation of the FunctionImpl type, that we can
avoid rather easily in this case.
This commit is completely optional, I guess, though the code we're using
RemoveElementsBy with the previous patch is sort of hot.
MozReview-Commit-ID: 2LoQs4cB28X
The standard placement new function is declared to not throw, which
means that, per spec, a null check on its result is required. There are
a number of places throughout xpcom/ where we know that we are passing
non-null pointers to placement new (and receiving them as a return
value), and we are therefore doing useless work performing these null
checks.
Therefore, we should be using an operator new overload that doesn't
require the null check. MFBT has just such an overload, so use that.
This patch makes most Run() declarations in subclasses of nsIRunnable have the
same form: |NS_IMETHOD Run() override|.
As a result of these changes, I had to add |override| to a couple of other
functions to satisfy clang's -Winconsistent-missing-override warning.
--HG--
extra : rebase_source : 815d0018b0b13329bb5698c410f500dddcc3ee12
This removes the unnecessary setting of c-basic-offset from all
python-mode files.
This was automatically generated using
perl -pi -e 's/; *c-basic-offset: *[0-9]+//'
... on the affected files.
The bulk of these files are moz.build files but there a few others as
well.
MozReview-Commit-ID: 2pPf3DEiZqx
--HG--
extra : rebase_source : 0a7dcac80b924174a2c429b093791148ea6ac204
The backwards copying case in MoveOverlappingRegion had a bug: rather
than destroying each element from the source range as we moved it, we
would always destroy the element at the beginning of the source range.
Fortunately, none of the existing types that were copied via
constructors seem to trigger the problematic code.
In all of the calls to CopyNonOverlappingRegion from within nsTArray, we
don't care about the contents of the source afterwards. So we can use
moves instead of copies to potentially make things more efficient.
Whenever we're copying the header, we can be guaranteed that we're never
going to use the elements from the old array afterward, so can move (in
the C++ sense) the elements rather than copying them.
We'll need these for future patches as we transition nsTArray to use
moves for most of its operations rather than copies. The implementation
of these functions are essentially cut-and-paste versions of the Copy*
functions, but using moves.
The names {Copy,Move}Elements are based on the use of mem{cpy,move},
respectively. However, I submit that we really want the names to
reflect the C++ operations being done, rather than the underlying
implementation details. So let's rename these to reflect that we are
always copying the elements, and discriminate between the two cases
based on whether the regions being copied overlap or not.
Bring CopyHeaderAndElements along for the ride, as well.
This change enables some of the methods in nsTArray to be lazily
instantiated, particularly the ones that care about whether the element
type is copyable. Since we have a number of places where nsTArray is
used with move-only types, we need to ensure that unless methods
requiring copyability are actually called, those methods are not
instantiated.
NS_ProxyRelease's current implementation requires a lot of code. We can
reduce the impact of this by providing an out-of-line implementation for
classes based on nsISupports. This change reduces codesize by ~60K on
a Linux x86-64 build.
Less ns-prefixing is more better. Also, this renaming makes clearer
that these classes are private implementation details, which is good,
because we're going to take advantage of that fact in a bit.
In all of the places touched by this patch, the smart pointer we're
appending is about to become unused, so simply .forget()'ing its
reference into the appropriate nsCOMArray works just fine.
By perfect-forwarding its argument, we can automatically gain move semantics
optimization when storing the given function object into nsRunnableFunction.
Also it allows movable-only function objects.
Note that any reference is removed from the type to be stored, so that the
runnable always contain a concrete function object.
MozReview-Commit-ID: 9EZK84ZhMvR
--HG--
extra : rebase_source : e1f87c3284fda4df6d13839ea6a0b0c2ce196833
These are generally good practice for reference-counted objects; they
catch cases where these operations are used by accident, breaking
reference-counting.
This doesn't show any existing problems, though.
MozReview-Commit-ID: EvRkNCymOqT
It's an annotation that is used a lot, and should be used even more, so a
shorter name is better.
MozReview-Commit-ID: 1VS4Dney4WX
--HG--
extra : rebase_source : b26919c1b0fcb32e5339adeef5be5becae6032cf
Outer pointers for object aggregation never get used. Having these
always-null pointers around means extra space to store them and extra
instructions to deal with them. Let's just remove them.
js::Class op are often all null. And when they're not all null, they're often
duplicated among classes. By pulling them out into their own struct, and using a
(possibly null) pointer in js::Class, we can save 114 KiB per process on
64-bit, and half that on 32-bit.
* * *
imported patch separate-ClassOps-2
--HG--
extra : rebase_source : bd751bf247e9491c1966a123dbeffa573657dfb1
There are about 600 of these, at 16 bytes each, and this change makes all of
them shareable between processes
--HG--
extra : rebase_source : b1824f0a022389aeb02d925c4d788e5a671bf782
Using explicit iteration at measurement sites is much simpler and nicer than
using callbacks.
--HG--
extra : rebase_source : 8b3f7aa702743b665383766b66a866a2c3d17240
FindFreeEntry() has one caller, so using MOZ_ALWAYS_INLINE should be good
enough for it. As for SearchTable(), NS_FASTCALL is the same as
PL_DHASH_FASTCALL and so can be used instead.
--HG--
extra : rebase_source : 814f96d4751922785358e7a4f9d64fcf522364c1
They're trivial and very hot. This reduces binary size in a 64-bit Linux opt build by 20 KiB and avoiding the calls can only help performance.
--HG--
extra : rebase_source : 774e6ffff9c787fa5444f939d1236d994ac8cf5b
The JS engine does not export symbols outside of XUL, so having these
defined inside mozglue apparently causes linking errors on some
platforms with the patches in bug 1120016.
This patch moves enough methods outside of mozglue that the patch in
that other bug will still link on all platforms, without moving so
much out that there are other linking errors.
--HG--
rename : xpcom/glue/nsCycleCollectionParticipant.cpp => xpcom/base/nsCycleCollectorTraceJSHelpers.cpp
The patch changes all uses of SizeOfIncludingThisMustBeUnshared() to
SizeOfIncludingThisIfUnshared(). This incurs the (tiny) cost of an unnecessary
IsReadonly() check for guaranteed-unshared strings, but avoids the possible
assertion failures that would occur when MustBeUnshared() was used incorrectly
on shared strings, which is an easy mistake to make.
--HG--
extra : rebase_source : b1e91f1c19bcbe0521b0ce461d6c90512ca938ef
xpcom/glue/PLDHashTable.cpp:471:10 [-Wunreachable-code-return] 'return' will never be executed
xpcom/tests/TestAutoPtr.cpp:324:9 [-Wunreachable-code] code will never be executed
xpcom/tests/TestBlockingProcess.cpp:6:11 [-Wunreachable-code-return] 'return' will never be executed
The logging interface is moved to xpcom/base, a LogModule wrapper for PR_Log is
added, a thread-safe LogModuleManager is added, and a LazyLogModule class used
to lazily load log modules in a thread-safe manner is added.
--HG--
rename : xpcom/glue/Logging.h => xpcom/base/Logging.h
extra : rebase_source : 89b76664d9477e2c894448cdea4dae1c61f8ca24
The bulk of this commit was generated with a script, executed at the top
level of a typical source code checkout. The only non-machine-generated
part was modifying MFBT's moz.build to reflect the new naming.
CLOSED TREE makes big refactorings like this a piece of cake.
# The main substitution.
find . -name '*.cpp' -o -name '*.cc' -o -name '*.h' -o -name '*.mm' -o -name '*.idl'| \
xargs perl -p -i -e '
s/nsRefPtr\.h/RefPtr\.h/g; # handle includes
s/nsRefPtr ?</RefPtr</g; # handle declarations and variables
'
# Handle a special friend declaration in gfx/layers/AtomicRefCountedWithFinalize.h.
perl -p -i -e 's/::nsRefPtr;/::RefPtr;/' gfx/layers/AtomicRefCountedWithFinalize.h
# Handle nsRefPtr.h itself, a couple places that define constructors
# from nsRefPtr, and code generators specially. We do this here, rather
# than indiscriminantly s/nsRefPtr/RefPtr/, because that would rename
# things like nsRefPtrHashtable.
perl -p -i -e 's/nsRefPtr/RefPtr/g' \
mfbt/nsRefPtr.h \
xpcom/glue/nsCOMPtr.h \
xpcom/base/OwningNonNull.h \
ipc/ipdl/ipdl/lower.py \
ipc/ipdl/ipdl/builtin.py \
dom/bindings/Codegen.py \
python/lldbutils/lldbutils/utils.py
# In our indiscriminate substitution above, we renamed
# nsRefPtrGetterAddRefs, the class behind getter_AddRefs. Fix that up.
find . -name '*.cpp' -o -name '*.h' -o -name '*.idl' | \
xargs perl -p -i -e 's/nsRefPtrGetterAddRefs/RefPtrGetterAddRefs/g'
if [ -d .git ]; then
git mv mfbt/nsRefPtr.h mfbt/RefPtr.h
else
hg mv mfbt/nsRefPtr.h mfbt/RefPtr.h
fi
--HG--
rename : mfbt/nsRefPtr.h => mfbt/RefPtr.h
It's a little more convenient than checking Count(), and also gives
nsTHashtable the same interface as nsTArray (for this operation, at
least), which seems worthwhile.
Having a template parameter conflict with a global name is terribly
inconvenient, so let's try to avoid that by renaming the 'RefPtr'
template parameter to something else.
This class can be used instead of raw pointer for a sound leaking-by-default
behavior. Also it could take advantage of move semantic check in the future.
--HG--
extra : source : 6bf72b4eaa92b13d42b547d8aeee677489a4d3e2
We don't need to reinterpret_cast when casting from void*, and we don't
need to reinterpret_cast when we're casting up and down a class
hierarchy. static_cast takes care of those cases just fine, and doesn't
scare the reader into thinking that nsTHashtable is doing something
unusual.