Closed Bug 1531027 Opened 9 months ago Closed 9 months ago

new_() could accept its parameter by reference

Categories

(Core :: DMD, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla67
Tracking Status
firefox67 --- fixed

People

(Reporter: ehsan, Assigned: ehsan)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

I'm puzzled by this. tmp is passed to InfallibleAllocPolicy::new_(P1 aP1), which calls new (mem) StackTrace(aP1), i.e. the copy constructor, which takes a const StackTrace&, which is a reference.

But I can see that an explicit copy constructor could be more efficient, by avoiding copying uninitialized elements. Maybe that would also appease Coverity?

(In reply to Nicholas Nethercote [:njn] from comment #1)

I'm puzzled by this. tmp is passed to InfallibleAllocPolicy::new_(P1 aP1), which calls new (mem) StackTrace(aP1), i.e. the copy constructor, which takes a const StackTrace&, which is a reference.

Right. The analysis was complaining about tmp being passed to InfallibleAllocPolicy::new_(P1 aP1). At that point the StackTrace object gets copied once. Then inside InfallibleAllocPolicy::new_ a new copy gets copy constructed from the first copy.

But I can see that an explicit copy constructor could be more efficient, by avoiding copying uninitialized elements. Maybe that would also appease Coverity?

I think that could be a nice but unrelated improvement as far as this bug is concerned. Let me submit what I think would be a fix to this bug.

This only copies the first mLength elements in mPcs.

Depends on D21487

Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/738bde54c69c
Make InfallibleAllocPolicy::new_ accept its argument as a reference; r=njn
Status: NEW → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
Pushed by nnethercote@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c59e07cae80f
Add an explicit copy constructor to StackTrace. r=ehsan
You need to log in before you can comment on or make changes to this bug.