Make override=1 and pure=1 imply virtual=1 to simplify MethodDecl

RESOLVED FIXED in Firefox 59

Status

()

Core
IPC
P3
normal
RESOLVED FIXED
13 days ago
5 days ago

People

(Reporter: cpeterson, Assigned: cpeterson)

Tracking

Trunk
mozilla59
Unspecified
All
Points:
---

Firefox Tracking Flags

(firefox-esr52 wontfix, firefox57 wontfix, firefox58 wontfix, firefox59 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments)

(Assignee)

Description

13 days ago
This bug is follow-up ipdl cleanup from bug 1428535 comment 6:

1. Make construction of `MethodDecl` not terrible, so we can specify `override=1` at construction time and have that imply `virtual`, rather than having to specify both.

2. And `pure=1` would work similarly.

3. Maybe we could even consolidate multiple keyword args into an enum as you suggest.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 4

7 days ago
While neither MethodDecl inline nor never_inline are set, force_inline is set. However, the force_inline name is misleading because is used to split class declarations and definitions, not to emit `MOZ_ALWAYS_INLINE` in the generated code.

Comment 5

6 days ago
mozreview-review
Comment on attachment 8942560 [details]
Bug 1428984 - Part 1: Consolidate virtual, pure, override, and static MethodDecl types into an MethodSpec enum.

https://reviewboard.mozilla.org/r/212812/#review218880

The idea of `MethodSpec` is better than the proliferation of keywords we have for some of these classes.  It's not great that we now have invalid values that can be passed into things like `FunctionDecl` or `DestructorDecl`...but I think on balance we're probably better off than we were before.
Attachment #8942560 - Flags: review?(nfroyd) → review+

Comment 6

6 days ago
mozreview-review
Comment on attachment 8942561 [details]
Bug 1428984 - Part 2: Remove unused never_inline flag.

https://reviewboard.mozilla.org/r/212814/#review218886
Attachment #8942561 - Flags: review?(nfroyd) → review+

Comment 7

6 days ago
mozreview-review
Comment on attachment 8942562 [details]
Bug 1428984 - Part 3: Remove unused inline flag.

https://reviewboard.mozilla.org/r/212816/#review218888
Attachment #8942562 - Flags: review?(nfroyd) → review+

Comment 8

5 days ago
Pushed by cpeterson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/612150969113
Part 1: Consolidate virtual, pure, override, and static MethodDecl types into an MethodSpec enum. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/18fb324726e7
Part 2: Remove unused never_inline flag. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/1f3edc2b0da5
Part 3: Remove unused inline flag. r=froydnj

Comment 9

5 days ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/612150969113
https://hg.mozilla.org/mozilla-central/rev/18fb324726e7
https://hg.mozilla.org/mozilla-central/rev/1f3edc2b0da5
Status: NEW → RESOLVED
Last Resolved: 5 days ago
status-firefox59: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.