1443428 - performance-unnecessary-copy-initialization - Fix the remaining issues

Status: RESOLVED WONTFIX

(Developer Infrastructure :: Source Code Analysis, defect)

Description

[Sylvestre Ledru [:Sylvestre]]

Already enabled at review phase. I am trying to remove them now (used the autofix)

Comment 1

[Sylvestre Ledru [:Sylvestre]]

Attachment: Bug 1443428 - Remove some unnecessary copies by declaring them as const

Review commit: https://reviewboard.mozilla.org/r/225328/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/225328/

Comment 2

[(no longer active)]

Comment on attachment 8956431 [details]
Bug 1443428 - Remove some unnecessary copies by declaring them as const

https://reviewboard.mozilla.org/r/225328/#review233330

::: dom/base/nsContentSink.cpp:1268
(Diff revision 1)
>    // already called it. This can happen when the layout frame for
>    // an iframe is constructed *between* the Embed() call for the
>    // docshell in the iframe, and the content sink's call to OpenBody().
>    // (Bug 153815)
>    if (shell && !shell->DidInitialize()) {
> -    nsCOMPtr<nsIPresShell> shellGrip = shell;
> +    const nsCOMPtr<nsIPresShell>& shellGrip = shell;

This isn't a rewrite that we can safely run over the entire code base.  For example, this line adds a bug, because it replaces a desirable side effect (create a copy of the nsCOMPtr which does the AddRef and Release as the variable starts and finishes its lifetime on the stack) with nothing.

The clang static analysis itself should probably be modified to only do this if the constructor of the type being copied is a trivial constructor or something...

I'm minusing the idea of running this automatically on the entire codebase.