Closed Bug 1522599 Opened 7 years ago Closed 7 years ago

make the logic for cxxTypeNeedsMove more straightforward; r=Alex_Gaynor

Categories

(Core :: IPC, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox66 --- fixed

People

(Reporter: froydnj, Assigned: froydnj)

Details

Attachments

(1 file)

I started with this to implement move Recv, but thought I might as well try to
land this while move Recv waits for other things.

Nested conditionals are hard to read; separating things out should make the flow somewhat more obvious.
Attachment #9038884 - Flags: review?(agaynor)
Comment on attachment 9038884 [details] [diff] [review] make the logic for cxxTypeNeedsMove more straightforward; r=Alex_Gaynor Review of attachment 9038884 [details] [diff] [review]: ----------------------------------------------------------------- One potential cleanup, LGTM otherwise. ::: ipc/ipdl/ipdl/lower.py @@ +572,5 @@ > + > + if ipdltype.isCxx(): > + return ipdltype.isMoveonly() > + > + if not ipdltype.isIPDL(): I think it might be slightly cleaner to express this as: ``` if isIPDL(): return isArray() or isShmem() ... return False ``` WDYT?
Attachment #9038884 - Flags: review?(agaynor) → review+

(In reply to Alex Gaynor [:Alex_Gaynor] from comment #2)

Comment on attachment 9038884 [details] [diff] [review]
make the logic for cxxTypeNeedsMove more straightforward; r=Alex_Gaynor

Review of attachment 9038884 [details] [diff] [review]:

One potential cleanup, LGTM otherwise.

::: ipc/ipdl/ipdl/lower.py
@@ +572,5 @@

  • if ipdltype.isCxx():
  •    return ipdltype.isMoveonly()
    
  • if not ipdltype.isIPDL():

I think it might be slightly cleaner to express this as:

if isIPDL():
   return isArray() or isShmem() ...

return False

WDYT?

I was aiming for early-exits as much as possible, but perhaps the positive logic is easier to read in this case? Let's just go with the positive logic.

Pushed by nfroyd@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/a3b8503e6109 make the logic for cxxTypeNeedsMove more straightforward; r=Alex_Gaynor
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: