Use class member initializer list for style structs

RESOLVED FIXED in Firefox 49

Status

()

RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: TYLin, Assigned: TYLin)

Tracking

Trunk
mozilla49
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox49 fixed)

Details

Attachments

(20 attachments)

58 bytes, text/x-review-board-request
heycam
: review+
Details
58 bytes, text/x-review-board-request
heycam
: review+
Details
58 bytes, text/x-review-board-request
heycam
: review+
Details
58 bytes, text/x-review-board-request
heycam
: review+
Details
58 bytes, text/x-review-board-request
heycam
: review+
Details
58 bytes, text/x-review-board-request
heycam
: review+
Details
58 bytes, text/x-review-board-request
heycam
: review+
Details
58 bytes, text/x-review-board-request
heycam
: review+
Details
58 bytes, text/x-review-board-request
heycam
: review+
Details
58 bytes, text/x-review-board-request
heycam
: review+
Details
58 bytes, text/x-review-board-request
heycam
: review+
Details
58 bytes, text/x-review-board-request
heycam
: review+
Details
58 bytes, text/x-review-board-request
heycam
: review+
Details
58 bytes, text/x-review-board-request
heycam
: review+
Details
58 bytes, text/x-review-board-request
heycam
: review+
Details
58 bytes, text/x-review-board-request
heycam
: review+
Details
58 bytes, text/x-review-board-request
heycam
: review+
Details
58 bytes, text/x-review-board-request
heycam
: review+
Details
58 bytes, text/x-review-board-request
heycam
: review+
Details
58 bytes, text/x-review-board-request
heycam
: review+
Details
(Assignee)

Description

3 years ago
Many style structs like nsStyleDisplay initialize their members in the body of the constructor [1]. nsStyleDisplay has mixed usage [2].

We should make those members initialize in the member initializer list. It's more C++-ish, and could be faster. Compilers also output warnings when the initialization of the struct members is out of order.

[1] https://dxr.mozilla.org/mozilla-central/rev/fc15477ce628599519cb0055f52cc195d640dc94/layout/style/nsStyleStruct.cpp#3567
[2] https://dxr.mozilla.org/mozilla-central/rev/fc15477ce628599519cb0055f52cc195d640dc94/layout/style/nsStyleStruct.cpp#2838
(Assignee)

Comment 1

3 years ago
Created attachment 8745203 [details]
MozReview Request: Bug 1267524 Part 1 - Use member initializer list for nsStyleOutline.

mCachedOutlineWidth was not initialized, I set it to 0.

Review commit: https://reviewboard.mozilla.org/r/48903/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48903/
Attachment #8745203 - Flags: review?(cam)
Attachment #8745204 - Flags: review?(cam)
Attachment #8745205 - Flags: review?(cam)
Attachment #8745206 - Flags: review?(cam)
Attachment #8745207 - Flags: review?(cam)
Attachment #8745208 - Flags: review?(cam)
Attachment #8745209 - Flags: review?(cam)
Attachment #8745210 - Flags: review?(cam)
Attachment #8745211 - Flags: review?(cam)
Attachment #8745212 - Flags: review?(cam)
Attachment #8745213 - Flags: review?(cam)
Attachment #8745214 - Flags: review?(cam)
Attachment #8745215 - Flags: review?(cam)
Attachment #8745216 - Flags: review?(cam)
Attachment #8745217 - Flags: review?(cam)
Attachment #8745218 - Flags: review?(cam)
Attachment #8745219 - Flags: review?(cam)
(Assignee)

Comment 2

3 years ago
Created attachment 8745204 [details]
MozReview Request: Bug 1267524 Part 2 - Use member initializer list for nsStyleXUL.

Review commit: https://reviewboard.mozilla.org/r/48905/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48905/
(Assignee)

Comment 3

3 years ago
Created attachment 8745205 [details]
MozReview Request: Bug 1267524 Part 3 - Use member initializer list for nsStyleColumn.

Review commit: https://reviewboard.mozilla.org/r/48907/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48907/
(Assignee)

Comment 4

3 years ago
Created attachment 8745206 [details]
MozReview Request: Bug 1267524 Part 4.2 - Use member initializer list for nsStyleSVG.

In the initializer list of the copy constructor, mStrokeDasharray and
mStrokeDasharrayLength are both initialized to nullptr and 0
respectively. If aSource.mStrokeDasharray has value, they're further be
set to proper values later in the function body. In this way, the two
'else' clauses can be removed.

Review commit: https://reviewboard.mozilla.org/r/48909/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48909/
(Assignee)

Comment 5

3 years ago
Created attachment 8745207 [details]
MozReview Request: Bug 1267524 Part 5 - Use member initializer list for nsStyleSVGReset.

Review commit: https://reviewboard.mozilla.org/r/48911/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48911/
(Assignee)

Comment 6

3 years ago
Created attachment 8745208 [details]
MozReview Request: Bug 1267524 Part 6 - Use member initializer list for nsStylePosition.

Review commit: https://reviewboard.mozilla.org/r/48913/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48913/
(Assignee)

Comment 7

3 years ago
Created attachment 8745209 [details]
MozReview Request: Bug 1267524 Part 7 - Use member initializer list for nsStyleTable.

Review commit: https://reviewboard.mozilla.org/r/48915/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48915/
(Assignee)

Comment 8

3 years ago
Created attachment 8745210 [details]
MozReview Request: Bug 1267524 Part 8 - Use member initializer list for nsStyleTableBorder.

Review commit: https://reviewboard.mozilla.org/r/48917/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48917/
(Assignee)

Comment 9

3 years ago
Created attachment 8745211 [details]
MozReview Request: Bug 1267524 Part 9 - Use member initializer list for nsStyleColor.

Review commit: https://reviewboard.mozilla.org/r/48919/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48919/
(Assignee)

Comment 10

3 years ago
Created attachment 8745212 [details]
MozReview Request: Bug 1267524 Part 10 - Use member initializer list for nsStyleDisplay.

Review commit: https://reviewboard.mozilla.org/r/48921/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48921/
(Assignee)

Comment 11

3 years ago
Created attachment 8745213 [details]
MozReview Request: Bug 1267524 Part 11 - Use member initializer list for nsStyleVisibility.

Review commit: https://reviewboard.mozilla.org/r/48923/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48923/
(Assignee)

Comment 12

3 years ago
Created attachment 8745214 [details]
MozReview Request: Bug 1267524 Part 12 - Use member initializer list for nsStyleContent.

Review commit: https://reviewboard.mozilla.org/r/48925/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48925/
(Assignee)

Comment 13

3 years ago
Created attachment 8745215 [details]
MozReview Request: Bug 1267524 Part 13 - Use member initializer list for nsStyleTextReset.

Review commit: https://reviewboard.mozilla.org/r/48927/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48927/
(Assignee)

Comment 14

3 years ago
Created attachment 8745216 [details]
MozReview Request: Bug 1267524 Part 14 - Use member initializer list for nsStyleText.

Review commit: https://reviewboard.mozilla.org/r/48929/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48929/
(Assignee)

Comment 15

3 years ago
Created attachment 8745217 [details]
MozReview Request: Bug 1267524 Part 15 - Use member initializer list for nsStyleUserInterface.

Review commit: https://reviewboard.mozilla.org/r/48931/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48931/
(Assignee)

Comment 16

3 years ago
Created attachment 8745218 [details]
MozReview Request: Bug 1267524 Part 16 - Use member initializer list for nsStyleUIReset.

Review commit: https://reviewboard.mozilla.org/r/48933/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48933/
(Assignee)

Comment 17

3 years ago
Created attachment 8745219 [details]
MozReview Request: Bug 1267524 Part 17 - Use member initializer list for nsStyleVariables.

Review commit: https://reviewboard.mozilla.org/r/48935/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48935/
David, am I right in thinking that initializers weren't traditionally used on style structs because we used to rely on the zero-initialization that pres arena allocation did, and that using initializers would maybe obscure that fact?  And that now we explicitly initialize everything, we should indeed just go ahead and use initializers?
Flags: needinfo?(dbaron)
(Assignee)

Comment 19

3 years ago
https://reviewboard.mozilla.org/r/48929/#review45787

::: layout/style/nsStyleStruct.cpp:3587
(Diff revision 1)
> -  mTextEmphasisColor = aContext.DefaultColor();
> -  mWebkitTextFillColor = aContext.DefaultColor();
> -  mWebkitTextStrokeColor = aContext.DefaultColor();
> -  mControlCharacterVisibility = nsCSSParser::ControlCharVisibilityDefault();
> -
>    mWordSpacing.SetCoordValue(0);

One general notice is that I leave nsStyleCoord untouched because we lack a constructor to construct a nsStyleCoord with SetCoordValue(0). However we can construct nsStyleCoord with SetNormalValue() by nsStyleCoord(eStyleUnit_Normal).

If we feel constructing nsStyleCoord in two pass is awkard and it's a better style to construct by something like nsStyleCoord(eStyleUnit_Normal) in the initializer list, I can make it happen in next patch set.

Perhaps we could modify [1] to let it accept eStyleUnit_Coord. I don't see the reason behind the comment.

[1] https://dxr.mozilla.org/mozilla-central/rev/fc15477ce628599519cb0055f52cc195d640dc94/layout/style/nsStyleCoord.cpp#25-26
(Assignee)

Comment 20

3 years ago
FWIW, Some of the style structs like nsStyleFont already use initializer list [1].

[1] https://dxr.mozilla.org/mozilla-central/rev/fc15477ce628599519cb0055f52cc195d640dc94/layout/style/nsStyleStruct.cpp#118
(In reply to Cameron McCormack (:heycam) from comment #18)
> David, am I right in thinking that initializers weren't traditionally used
> on style structs because we used to rely on the zero-initialization that
> pres arena allocation did, and that using initializers would maybe obscure
> that fact?  And that now we explicitly initialize everything, we should
> indeed just go ahead and use initializers?

I'm not really sure of the history, but using member initializers makes sense.


(In reply to Ting-Yu Lin [:TYLin] (UTC+8) from comment #0)
> Compilers also output warnings when the
> initialization of the struct members is out of order.

One note here:  the members are initialized in order even when the declarations are out of order.  So the compiler warns because having them listed out of order is confusing code, because things happen in a different order from the order they're written.
Flags: needinfo?(dbaron)
Comment on attachment 8745203 [details]
MozReview Request: Bug 1267524 Part 1 - Use member initializer list for nsStyleOutline.

https://reviewboard.mozilla.org/r/48903/#review46579
Attachment #8745203 - Flags: review?(cam) → review+
Attachment #8745204 - Flags: review?(cam) → review+
Comment on attachment 8745204 [details]
MozReview Request: Bug 1267524 Part 2 - Use member initializer list for nsStyleXUL.

https://reviewboard.mozilla.org/r/48905/#review46581
Comment on attachment 8745205 [details]
MozReview Request: Bug 1267524 Part 3 - Use member initializer list for nsStyleColumn.

https://reviewboard.mozilla.org/r/48907/#review46583

::: layout/style/nsStyleStruct.cpp:814
(Diff revision 1)
> +  : mColumnCount(NS_STYLE_COLUMN_COUNT_AUTO)
> +  , mColumnRuleColor(NS_RGB(0, 0, 0))
> +  , mColumnRuleStyle(NS_STYLE_BORDER_STYLE_NONE)
> +  , mColumnFill(NS_STYLE_COLUMN_FILL_BALANCE)
> +  , mColumnRuleColorIsForeground(true)
> +  , mColumnRuleWidth((StaticPresData::Get()->GetBorderWidthTable())[NS_STYLE_BORDER_WIDTH_MEDIUM])

Let's wrap this to fit 80 columns:

  , mColumnRuleWidth(StaticPresData::Get()->
                       GetBorderWidthTable()[NS_STYLE_BORDER_WIDTH_MEDIUM])
Attachment #8745205 - Flags: review?(cam) → review+
Attachment #8745206 - Flags: review?(cam)
Comment on attachment 8745206 [details]
MozReview Request: Bug 1267524 Part 4.2 - Use member initializer list for nsStyleSVG.

https://reviewboard.mozilla.org/r/48909/#review46585

::: layout/style/nsStyleStruct.cpp:876
(Diff revision 1)
>  
>  // --------------------
>  // nsStyleSVG
>  //
>  nsStyleSVG::nsStyleSVG(StyleStructContext aContext)
> +  : mFill(eStyleSVGPaintType_Color)

Since it might not be obvious that it is initialized, add a comment at the end of this line saying that it sets it to NS_RGB(0, 0, 0).

::: layout/style/nsStyleStruct.cpp
(Diff revision 1)
>  
> -  mMarkerEnd = aSource.mMarkerEnd;
> -  mMarkerMid = aSource.mMarkerMid;
> -  mMarkerStart = aSource.mMarkerStart;
> -
> -  mStrokeDasharrayLength = aSource.mStrokeDasharrayLength;

Can you add an assertion in here to verify that if aSource.mStrokeDasharray is null, then aSource.mStrokeDasharrayLength is 0?

In fact I think it would be better just to use an nsTArray to store the dasharray, then we don't need to worry about manual memory management for the array.  Please file a followup to do that.

::: layout/style/nsStyleStruct.cpp
(Diff revision 1)
>  {
>    if (mType == eStyleSVGPaintType_Server) {
>      this->~nsStyleSVGPaint();
> -    new (this) nsStyleSVGPaint();
> +    new (this) nsStyleSVGPaint(aType);
>    }
> -  mType = aType;

Why don't we need to set mType any more?  If the old type is not eStyleSVGPaintType_Server, don't we still need to set the new type?

I don't really like the explicit destructor / placement new calls.  What do you think about:

* making the constructor just initialize mType to nsStyleSVGPaint(0) and then immediately call SetType(aType)

* adding a Reset method that does the work that the constructor currently does, and then sets mType to nsStyleSVGPaint(0)

* making SetType first call Reset() and then do the work that is currently in the constructor

This is more like what nsCSSValue does.
Attachment #8745207 - Flags: review?(cam) → review+
Comment on attachment 8745207 [details]
MozReview Request: Bug 1267524 Part 5 - Use member initializer list for nsStyleSVGReset.

https://reviewboard.mozilla.org/r/48911/#review46587
Comment on attachment 8745208 [details]
MozReview Request: Bug 1267524 Part 6 - Use member initializer list for nsStylePosition.

https://reviewboard.mozilla.org/r/48913/#review46589
Attachment #8745208 - Flags: review?(cam) → review+
Comment on attachment 8745209 [details]
MozReview Request: Bug 1267524 Part 7 - Use member initializer list for nsStyleTable.

https://reviewboard.mozilla.org/r/48915/#review46591
Attachment #8745209 - Flags: review?(cam) → review+
Comment on attachment 8745210 [details]
MozReview Request: Bug 1267524 Part 8 - Use member initializer list for nsStyleTableBorder.

https://reviewboard.mozilla.org/r/48917/#review46593

::: layout/style/nsStyleStruct.cpp:1805
(Diff revision 1)
> -  mCaptionSide = NS_STYLE_CAPTION_SIDE_TOP;
> -  mBorderSpacingCol = 0;
> -  mBorderSpacingRow = 0;
>  }
>  
> -nsStyleTableBorder::~nsStyleTableBorder(void) 
> +nsStyleTableBorder::~nsStyleTableBorder(void)

While you're touching that line, you can remove the "void".
Attachment #8745210 - Flags: review?(cam) → review+
(In reply to Cameron McCormack (:heycam) from comment #29)
> While you're touching that line, you can remove the "void".

That applies to some of the previous patches, too.
Comment on attachment 8745211 [details]
MozReview Request: Bug 1267524 Part 9 - Use member initializer list for nsStyleColor.

https://reviewboard.mozilla.org/r/48919/#review46595
Attachment #8745211 - Flags: review?(cam) → review+
(In reply to Cameron McCormack (:heycam) from comment #29)
> While you're touching that line, you can remove the "void".

A cleanup like that could also be an extra patch on top.

(In C, () means "we don't specify what the arguments are" and (void) means "no arguments", but in C++ they both mean "no arguments".  We consider using (void) poor style in C++.)
Comment on attachment 8745212 [details]
MozReview Request: Bug 1267524 Part 10 - Use member initializer list for nsStyleDisplay.

https://reviewboard.mozilla.org/r/48921/#review46597

::: layout/style/nsStyleStruct.cpp:2833
(Diff revision 1)
> +  , mOriginalDisplay(mDisplay)
> +  , mContain(NS_STYLE_CONTAIN_NONE)

Regarding the initialization order issue that David mentioned, our compiler warnings (turned into errors on some platforms) would catch this, if mDisplay were initialized after mOriginalDisplay (resulting in the assignment to mOriginalDisplay using an uninitialized value).  But I don't think there's much to gain from initializing mOriginalDisplay (and mOriginalFloats) in this way, so let's just use NS_STYLE_DISPLAY_INLINE (and NS_STYLE_FLOAT_NONE).
Attachment #8745212 - Flags: review?(cam) → review+
Comment on attachment 8745213 [details]
MozReview Request: Bug 1267524 Part 11 - Use member initializer list for nsStyleVisibility.

https://reviewboard.mozilla.org/r/48923/#review46599
Attachment #8745213 - Flags: review?(cam) → review+
Comment on attachment 8745214 [details]
MozReview Request: Bug 1267524 Part 12 - Use member initializer list for nsStyleContent.

https://reviewboard.mozilla.org/r/48925/#review46601

::: layout/style/nsStyleStruct.cpp:3332
(Diff revision 1)
>  //-----------------------
>  // nsStyleContent
>  //
>  
>  nsStyleContent::nsStyleContent(StyleStructContext aContext)
> -  : mMarkerOffset(),
> +  : mMarkerOffset()

mMarkerOffset will be default constructed without being listed here, so we can remove this.
Attachment #8745214 - Flags: review?(cam) → review+
Comment on attachment 8745215 [details]
MozReview Request: Bug 1267524 Part 13 - Use member initializer list for nsStyleTextReset.

https://reviewboard.mozilla.org/r/48927/#review46603
Attachment #8745215 - Flags: review?(cam) → review+
Attachment #8745216 - Flags: review?(cam) → review+
Comment on attachment 8745216 [details]
MozReview Request: Bug 1267524 Part 14 - Use member initializer list for nsStyleText.

https://reviewboard.mozilla.org/r/48929/#review46605
https://reviewboard.mozilla.org/r/48929/#review45787

> One general notice is that I leave nsStyleCoord untouched because we lack a constructor to construct a nsStyleCoord with SetCoordValue(0). However we can construct nsStyleCoord with SetNormalValue() by nsStyleCoord(eStyleUnit_Normal).
> 
> If we feel constructing nsStyleCoord in two pass is awkard and it's a better style to construct by something like nsStyleCoord(eStyleUnit_Normal) in the initializer list, I can make it happen in next patch set.
> 
> Perhaps we could modify [1] to let it accept eStyleUnit_Coord. I don't see the reason behind the comment.
> 
> [1] https://dxr.mozilla.org/mozilla-central/rev/fc15477ce628599519cb0055f52cc195d640dc94/layout/style/nsStyleCoord.cpp#25-26

mSomeValue(eStyleUnit_Normal) is better than a separate SetNormalValue() call, I think.

For coord values, that comment might be because we have some support in nsCoord.h for nscoord being float rather than int32_t, although I am sure nobody has tested that configuration for a long time and it's unlikely we'll ever use it.  If we just added a second argument to the nsStyleCoord(nscoord) constructor, it would be ambiguous with the one taking an int32_t.

We could add a static method to nsStyleCoord:

  static nsStyleCoord CoordValue(nscoord aValue) { nsStyleCoord v; v.SetCoordValue(aValue); return v; }
  
that we could then use in initializer lists.
Comment on attachment 8745217 [details]
MozReview Request: Bug 1267524 Part 15 - Use member initializer list for nsStyleUserInterface.

https://reviewboard.mozilla.org/r/48931/#review46609

::: layout/style/nsStyleStruct.cpp:3783
(Diff revision 1)
>  nsStyleUserInterface::nsStyleUserInterface(StyleStructContext aContext)
> -{ 
> +  : mUserInput(NS_STYLE_USER_INPUT_AUTO)
> +  , mUserModify(NS_STYLE_USER_MODIFY_READ_ONLY)
> +  , mUserFocus(NS_STYLE_USER_FOCUS_NONE)
> +  , mPointerEvents(NS_STYLE_POINTER_EVENTS_AUTO)
> +  , mCursor(NS_STYLE_CURSOR_AUTO) // fix for bugzilla bug 51113

This old comment isn't necessary, in my opinion.  (There's nothing special about setting the property to its initial value, here.)  So please remove it.
Attachment #8745217 - Flags: review?(cam) → review+
Comment on attachment 8745218 [details]
MozReview Request: Bug 1267524 Part 16 - Use member initializer list for nsStyleUIReset.

https://reviewboard.mozilla.org/r/48933/#review46611
Attachment #8745218 - Flags: review?(cam) → review+
Comment on attachment 8745219 [details]
MozReview Request: Bug 1267524 Part 17 - Use member initializer list for nsStyleVariables.

https://reviewboard.mozilla.org/r/48935/#review46613
Attachment #8745219 - Flags: review?(cam) → review+
(Assignee)

Comment 42

3 years ago
https://reviewboard.mozilla.org/r/48917/#review46593

> While you're touching that line, you can remove the "void".

I'll remove all "void" from zero-argument functions in a separate patch.
(Assignee)

Updated

3 years ago
Blocks: 1269175
(Assignee)

Comment 43

3 years ago
https://reviewboard.mozilla.org/r/48909/#review46585

> Since it might not be obvious that it is initialized, add a comment at the end of this line saying that it sets it to NS_RGB(0, 0, 0).

Will do.

> Can you add an assertion in here to verify that if aSource.mStrokeDasharray is null, then aSource.mStrokeDasharrayLength is 0?
> 
> In fact I think it would be better just to use an nsTArray to store the dasharray, then we don't need to worry about manual memory management for the array.  Please file a followup to do that.

OK. Will add assertion to guantee the correctness of aSource.mStrokeDasharray and aSource.mStrokeDasharrayLength.

File bug 1269175 for using nsTArray for nsStyleSVG::mStrokeDasharray.

> Why don't we need to set mType any more?  If the old type is not eStyleSVGPaintType_Server, don't we still need to set the new type?
> 
> I don't really like the explicit destructor / placement new calls.  What do you think about:
> 
> * making the constructor just initialize mType to nsStyleSVGPaint(0) and then immediately call SetType(aType)
> 
> * adding a Reset method that does the work that the constructor currently does, and then sets mType to nsStyleSVGPaint(0)
> 
> * making SetType first call Reset() and then do the work that is currently in the constructor
> 
> This is more like what nsCSSValue does.

My bad. We need to set mType. I agree the explicit destructor / placement new calls is not appealing. See my Part 4.1 for the rewrite of nsStyleSVGPaint.
(Assignee)

Comment 44

3 years ago
https://reviewboard.mozilla.org/r/48929/#review45787

> mSomeValue(eStyleUnit_Normal) is better than a separate SetNormalValue() call, I think.
> 
> For coord values, that comment might be because we have some support in nsCoord.h for nscoord being float rather than int32_t, although I am sure nobody has tested that configuration for a long time and it's unlikely we'll ever use it.  If we just added a second argument to the nsStyleCoord(nscoord) constructor, it would be ambiguous with the one taking an int32_t.
> 
> We could add a static method to nsStyleCoord:
> 
>   static nsStyleCoord CoordValue(nscoord aValue) { nsStyleCoord v; v.SetCoordValue(aValue); return v; }
>   
> that we could then use in initializer lists.

I just found that we do have a inline nsStyleCoord constructor in the header [1] for setting the type to eStyleUnit_Coord and value. Since most of the parts in this bug have been reviewed, I'll move the rest of the nsStyleCoord to initilization list in a separate patch.

[1] https://dxr.mozilla.org/mozilla-central/rev/1461a4071341c282afcf7b72e33036412d2251d4/layout/style/nsStyleCoord.h#321
(Assignee)

Comment 45

3 years ago
Created attachment 8747572 [details]
MozReview Request: Bug 1267524 Part 4.1 - Add Reset() and rewrite methods for nsStyleSVGPaint.

Add Reset() to eliminate the explicit destructor / placement new calls
in SetType().

Review commit: https://reviewboard.mozilla.org/r/49935/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/49935/
Attachment #8745206 - Attachment description: MozReview Request: Bug 1267524 Part 4 - Use member initializer list for nsStyleSVG. → MozReview Request: Bug 1267524 Part 4.2 - Use member initializer list for nsStyleSVG.
Attachment #8745206 - Flags: review?(cam)
(Assignee)

Comment 46

3 years ago
Created attachment 8747573 [details]
MozReview Request: Bug 1267524 Part 18 - Remove "void" from zero argument functions.

Review commit: https://reviewboard.mozilla.org/r/49937/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/49937/
(Assignee)

Comment 47

3 years ago
Created attachment 8747574 [details]
MozReview Request: Bug 1267524 Part 19 - Move nsStyleCoord members to initializer list.

Review commit: https://reviewboard.mozilla.org/r/49939/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/49939/
(Assignee)

Comment 48

3 years ago
Comment on attachment 8745203 [details]
MozReview Request: Bug 1267524 Part 1 - Use member initializer list for nsStyleOutline.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48903/diff/1-2/
(Assignee)

Comment 49

3 years ago
Comment on attachment 8745204 [details]
MozReview Request: Bug 1267524 Part 2 - Use member initializer list for nsStyleXUL.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48905/diff/1-2/
(Assignee)

Comment 50

3 years ago
Comment on attachment 8745205 [details]
MozReview Request: Bug 1267524 Part 3 - Use member initializer list for nsStyleColumn.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48907/diff/1-2/
(Assignee)

Comment 51

3 years ago
Comment on attachment 8745206 [details]
MozReview Request: Bug 1267524 Part 4.2 - Use member initializer list for nsStyleSVG.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48909/diff/1-2/
(Assignee)

Comment 52

3 years ago
Comment on attachment 8745207 [details]
MozReview Request: Bug 1267524 Part 5 - Use member initializer list for nsStyleSVGReset.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48911/diff/1-2/
(Assignee)

Comment 53

3 years ago
Comment on attachment 8745208 [details]
MozReview Request: Bug 1267524 Part 6 - Use member initializer list for nsStylePosition.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48913/diff/1-2/
(Assignee)

Comment 54

3 years ago
Comment on attachment 8745209 [details]
MozReview Request: Bug 1267524 Part 7 - Use member initializer list for nsStyleTable.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48915/diff/1-2/
(Assignee)

Comment 55

3 years ago
Comment on attachment 8745210 [details]
MozReview Request: Bug 1267524 Part 8 - Use member initializer list for nsStyleTableBorder.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48917/diff/1-2/
(Assignee)

Comment 56

3 years ago
Comment on attachment 8745211 [details]
MozReview Request: Bug 1267524 Part 9 - Use member initializer list for nsStyleColor.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48919/diff/1-2/
(Assignee)

Comment 57

3 years ago
Comment on attachment 8745212 [details]
MozReview Request: Bug 1267524 Part 10 - Use member initializer list for nsStyleDisplay.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48921/diff/1-2/
(Assignee)

Comment 58

3 years ago
Comment on attachment 8745213 [details]
MozReview Request: Bug 1267524 Part 11 - Use member initializer list for nsStyleVisibility.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48923/diff/1-2/
(Assignee)

Comment 59

3 years ago
Comment on attachment 8745214 [details]
MozReview Request: Bug 1267524 Part 12 - Use member initializer list for nsStyleContent.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48925/diff/1-2/
(Assignee)

Comment 60

3 years ago
Comment on attachment 8745215 [details]
MozReview Request: Bug 1267524 Part 13 - Use member initializer list for nsStyleTextReset.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48927/diff/1-2/
(Assignee)

Comment 61

3 years ago
Comment on attachment 8745216 [details]
MozReview Request: Bug 1267524 Part 14 - Use member initializer list for nsStyleText.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48929/diff/1-2/
(Assignee)

Comment 62

3 years ago
Comment on attachment 8745217 [details]
MozReview Request: Bug 1267524 Part 15 - Use member initializer list for nsStyleUserInterface.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48931/diff/1-2/
(Assignee)

Comment 63

3 years ago
Comment on attachment 8745218 [details]
MozReview Request: Bug 1267524 Part 16 - Use member initializer list for nsStyleUIReset.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48933/diff/1-2/
(Assignee)

Comment 64

3 years ago
Comment on attachment 8745219 [details]
MozReview Request: Bug 1267524 Part 17 - Use member initializer list for nsStyleVariables.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48935/diff/1-2/
(Assignee)

Updated

3 years ago
Attachment #8747572 - Flags: review?(cam)
Attachment #8747573 - Flags: review?(cam)
Attachment #8747574 - Flags: review?(cam)
(Assignee)

Comment 65

3 years ago
I got try fails. I guess nsStyleSVGPaint needs a copy constructor to get the mPaint.mPaintServer reference count correct.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9be669ce446d
(Assignee)

Comment 66

3 years ago
Comment on attachment 8747572 [details]
MozReview Request: Bug 1267524 Part 4.1 - Add Reset() and rewrite methods for nsStyleSVGPaint.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49935/diff/1-2/
(Assignee)

Comment 67

3 years ago
Comment on attachment 8745206 [details]
MozReview Request: Bug 1267524 Part 4.2 - Use member initializer list for nsStyleSVG.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48909/diff/2-3/
(Assignee)

Comment 68

3 years ago
Comment on attachment 8745207 [details]
MozReview Request: Bug 1267524 Part 5 - Use member initializer list for nsStyleSVGReset.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48911/diff/2-3/
(Assignee)

Comment 69

3 years ago
Comment on attachment 8745208 [details]
MozReview Request: Bug 1267524 Part 6 - Use member initializer list for nsStylePosition.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48913/diff/2-3/
(Assignee)

Comment 70

3 years ago
Comment on attachment 8745209 [details]
MozReview Request: Bug 1267524 Part 7 - Use member initializer list for nsStyleTable.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48915/diff/2-3/
(Assignee)

Comment 71

3 years ago
Comment on attachment 8745210 [details]
MozReview Request: Bug 1267524 Part 8 - Use member initializer list for nsStyleTableBorder.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48917/diff/2-3/
(Assignee)

Comment 72

3 years ago
Comment on attachment 8745211 [details]
MozReview Request: Bug 1267524 Part 9 - Use member initializer list for nsStyleColor.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48919/diff/2-3/
(Assignee)

Comment 73

3 years ago
Comment on attachment 8745212 [details]
MozReview Request: Bug 1267524 Part 10 - Use member initializer list for nsStyleDisplay.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48921/diff/2-3/
(Assignee)

Comment 74

3 years ago
Comment on attachment 8745213 [details]
MozReview Request: Bug 1267524 Part 11 - Use member initializer list for nsStyleVisibility.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48923/diff/2-3/
(Assignee)

Comment 75

3 years ago
Comment on attachment 8745214 [details]
MozReview Request: Bug 1267524 Part 12 - Use member initializer list for nsStyleContent.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48925/diff/2-3/
(Assignee)

Comment 76

3 years ago
Comment on attachment 8745215 [details]
MozReview Request: Bug 1267524 Part 13 - Use member initializer list for nsStyleTextReset.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48927/diff/2-3/
(Assignee)

Comment 77

3 years ago
Comment on attachment 8745216 [details]
MozReview Request: Bug 1267524 Part 14 - Use member initializer list for nsStyleText.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48929/diff/2-3/
(Assignee)

Comment 78

3 years ago
Comment on attachment 8745217 [details]
MozReview Request: Bug 1267524 Part 15 - Use member initializer list for nsStyleUserInterface.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48931/diff/2-3/
(Assignee)

Comment 79

3 years ago
Comment on attachment 8745218 [details]
MozReview Request: Bug 1267524 Part 16 - Use member initializer list for nsStyleUIReset.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48933/diff/2-3/
(Assignee)

Comment 80

3 years ago
Comment on attachment 8745219 [details]
MozReview Request: Bug 1267524 Part 17 - Use member initializer list for nsStyleVariables.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48935/diff/2-3/
(Assignee)

Comment 81

3 years ago
Comment on attachment 8747573 [details]
MozReview Request: Bug 1267524 Part 18 - Remove "void" from zero argument functions.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49937/diff/1-2/
(Assignee)

Comment 82

3 years ago
Comment on attachment 8747574 [details]
MozReview Request: Bug 1267524 Part 19 - Move nsStyleCoord members to initializer list.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49939/diff/1-2/
Attachment #8745206 - Flags: review?(cam)
Comment on attachment 8745206 [details]
MozReview Request: Bug 1267524 Part 4.2 - Use member initializer list for nsStyleSVG.

https://reviewboard.mozilla.org/r/48909/#review46919

::: layout/style/nsStyleStruct.cpp:942
(Diff revisions 1 - 3)
>    , mStrokeWidthFromObject(aSource.mStrokeWidthFromObject)
>  {
>    MOZ_COUNT_CTOR(nsStyleSVG);
>  
> +  MOZ_ASSERT(bool(aSource.mStrokeDasharray) ==
> +             bool(aSource.mStrokeDasharrayLength));

Please add a message to the assertion.

::: layout/style/nsStyleStruct.cpp:2702
(Diff revisions 1 - 3)
> -bool nsStyleBackground::HasFixedBackground() const
> +bool nsStyleBackground::HasFixedBackground(nsIFrame* aFrame) const
>  {
>    NS_FOR_VISIBLE_IMAGE_LAYERS_BACK_TO_FRONT(i, mImage) {
>      const nsStyleImageLayers::Layer &layer = mImage.mLayers[i];
>      if (layer.mAttachment == NS_STYLE_IMAGELAYER_ATTACHMENT_FIXED &&
> -        !layer.mImage.IsEmpty()) {
> +        !layer.mImage.IsEmpty() &&
> +        !nsLayoutUtils::IsTransformed(aFrame)) {

Are these changes meant to be here?
Comment on attachment 8747572 [details]
MozReview Request: Bug 1267524 Part 4.1 - Add Reset() and rewrite methods for nsStyleSVGPaint.

https://reviewboard.mozilla.org/r/49935/#review46921
Attachment #8747572 - Flags: review?(cam) → review+
Comment on attachment 8747573 [details]
MozReview Request: Bug 1267524 Part 18 - Remove "void" from zero argument functions.

https://reviewboard.mozilla.org/r/49937/#review46923
Attachment #8747573 - Flags: review?(cam) → review+
Comment on attachment 8747574 [details]
MozReview Request: Bug 1267524 Part 19 - Move nsStyleCoord members to initializer list.

https://reviewboard.mozilla.org/r/49939/#review46925

::: layout/style/nsStyleStruct.h:1755
(Diff revision 2)
> +  // These members get their default constructors which initialize them to
> +  // representations of their respective initial value.
> +  // mGridTemplate{Rows,Columns}: false and empty arrays for 'none'
> +  // mGridTemplateAreas: nullptr for 'none'
> +  // mGrid{Column,Row}{Start,End}: false/0/empty values for 'auto'

I think this comment would be best left in the body of the constructor.  (Feels like it clutters the header file here a bit.)

::: layout/style/nsStyleStruct.cpp:2939
(Diff revision 2)
>    , mTransformStyle(aSource.mTransformStyle)
>    , mTransformBox(aSource.mTransformBox)
>    , mSpecifiedTransform(aSource.mSpecifiedTransform)
> +  , mTransformOrigin{ aSource.mTransformOrigin[0],
> +                      aSource.mTransformOrigin[1],
> +                       aSource.mTransformOrigin[2] }

Nit: this line is not aligned.
Attachment #8747574 - Flags: review?(cam) → review+
(Assignee)

Comment 87

3 years ago
https://reviewboard.mozilla.org/r/48909/#review46919

> Please add a message to the assertion.

Will do.

> Are these changes meant to be here?

Yes. Mozreview will show everything changed including those from rebasing. If the diff is between origin and revision 3, they won't show in my part 4.2 :)
Comment on attachment 8745206 [details]
MozReview Request: Bug 1267524 Part 4.2 - Use member initializer list for nsStyleSVG.

https://reviewboard.mozilla.org/r/48909/#review46933
Attachment #8745206 - Flags: review+
(Assignee)

Comment 89

3 years ago
https://reviewboard.mozilla.org/r/49939/#review46925

> I think this comment would be best left in the body of the constructor.  (Feels like it clutters the header file here a bit.)

OK. Let's not move it.

> Nit: this line is not aligned.

Nice catch!
(Assignee)

Comment 90

3 years ago
Comment on attachment 8745203 [details]
MozReview Request: Bug 1267524 Part 1 - Use member initializer list for nsStyleOutline.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48903/diff/2-3/
(Assignee)

Comment 91

3 years ago
Comment on attachment 8745204 [details]
MozReview Request: Bug 1267524 Part 2 - Use member initializer list for nsStyleXUL.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48905/diff/2-3/
(Assignee)

Comment 92

3 years ago
Comment on attachment 8745205 [details]
MozReview Request: Bug 1267524 Part 3 - Use member initializer list for nsStyleColumn.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48907/diff/2-3/
(Assignee)

Comment 93

3 years ago
Comment on attachment 8747572 [details]
MozReview Request: Bug 1267524 Part 4.1 - Add Reset() and rewrite methods for nsStyleSVGPaint.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49935/diff/2-3/
(Assignee)

Comment 94

3 years ago
Comment on attachment 8745206 [details]
MozReview Request: Bug 1267524 Part 4.2 - Use member initializer list for nsStyleSVG.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48909/diff/3-4/
(Assignee)

Comment 95

3 years ago
Comment on attachment 8745207 [details]
MozReview Request: Bug 1267524 Part 5 - Use member initializer list for nsStyleSVGReset.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48911/diff/3-4/
(Assignee)

Comment 96

3 years ago
Comment on attachment 8745208 [details]
MozReview Request: Bug 1267524 Part 6 - Use member initializer list for nsStylePosition.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48913/diff/3-4/
(Assignee)

Comment 97

3 years ago
Comment on attachment 8745209 [details]
MozReview Request: Bug 1267524 Part 7 - Use member initializer list for nsStyleTable.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48915/diff/3-4/
(Assignee)

Comment 98

3 years ago
Comment on attachment 8745210 [details]
MozReview Request: Bug 1267524 Part 8 - Use member initializer list for nsStyleTableBorder.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48917/diff/3-4/
(Assignee)

Comment 99

3 years ago
Comment on attachment 8745211 [details]
MozReview Request: Bug 1267524 Part 9 - Use member initializer list for nsStyleColor.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48919/diff/3-4/
(Assignee)

Comment 100

3 years ago
Comment on attachment 8745212 [details]
MozReview Request: Bug 1267524 Part 10 - Use member initializer list for nsStyleDisplay.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48921/diff/3-4/
(Assignee)

Comment 101

3 years ago
Comment on attachment 8745213 [details]
MozReview Request: Bug 1267524 Part 11 - Use member initializer list for nsStyleVisibility.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48923/diff/3-4/
(Assignee)

Comment 102

3 years ago
Comment on attachment 8745214 [details]
MozReview Request: Bug 1267524 Part 12 - Use member initializer list for nsStyleContent.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48925/diff/3-4/
(Assignee)

Comment 103

3 years ago
Comment on attachment 8745215 [details]
MozReview Request: Bug 1267524 Part 13 - Use member initializer list for nsStyleTextReset.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48927/diff/3-4/
(Assignee)

Comment 104

3 years ago
Comment on attachment 8745216 [details]
MozReview Request: Bug 1267524 Part 14 - Use member initializer list for nsStyleText.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48929/diff/3-4/
(Assignee)

Comment 105

3 years ago
Comment on attachment 8745217 [details]
MozReview Request: Bug 1267524 Part 15 - Use member initializer list for nsStyleUserInterface.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48931/diff/3-4/
(Assignee)

Comment 106

3 years ago
Comment on attachment 8745218 [details]
MozReview Request: Bug 1267524 Part 16 - Use member initializer list for nsStyleUIReset.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48933/diff/3-4/
(Assignee)

Comment 107

3 years ago
Comment on attachment 8745219 [details]
MozReview Request: Bug 1267524 Part 17 - Use member initializer list for nsStyleVariables.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48935/diff/3-4/
(Assignee)

Comment 108

3 years ago
Comment on attachment 8747573 [details]
MozReview Request: Bug 1267524 Part 18 - Remove "void" from zero argument functions.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49937/diff/2-3/
(Assignee)

Comment 109

3 years ago
Comment on attachment 8747574 [details]
MozReview Request: Bug 1267524 Part 19 - Move nsStyleCoord members to initializer list.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49939/diff/2-3/
(Assignee)

Comment 110

3 years ago
Comment on attachment 8745210 [details]
MozReview Request: Bug 1267524 Part 8 - Use member initializer list for nsStyleTableBorder.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48917/diff/4-5/
(Assignee)

Comment 111

3 years ago
Comment on attachment 8745211 [details]
MozReview Request: Bug 1267524 Part 9 - Use member initializer list for nsStyleColor.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48919/diff/4-5/
(Assignee)

Comment 112

3 years ago
Comment on attachment 8745212 [details]
MozReview Request: Bug 1267524 Part 10 - Use member initializer list for nsStyleDisplay.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48921/diff/4-5/
(Assignee)

Comment 113

3 years ago
Comment on attachment 8745213 [details]
MozReview Request: Bug 1267524 Part 11 - Use member initializer list for nsStyleVisibility.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48923/diff/4-5/
(Assignee)

Comment 114

3 years ago
Comment on attachment 8745214 [details]
MozReview Request: Bug 1267524 Part 12 - Use member initializer list for nsStyleContent.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48925/diff/4-5/
(Assignee)

Comment 115

3 years ago
Comment on attachment 8745215 [details]
MozReview Request: Bug 1267524 Part 13 - Use member initializer list for nsStyleTextReset.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48927/diff/4-5/
(Assignee)

Comment 116

3 years ago
Comment on attachment 8745216 [details]
MozReview Request: Bug 1267524 Part 14 - Use member initializer list for nsStyleText.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48929/diff/4-5/
(Assignee)

Comment 117

3 years ago
Comment on attachment 8745217 [details]
MozReview Request: Bug 1267524 Part 15 - Use member initializer list for nsStyleUserInterface.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48931/diff/4-5/
(Assignee)

Comment 118

3 years ago
Comment on attachment 8745218 [details]
MozReview Request: Bug 1267524 Part 16 - Use member initializer list for nsStyleUIReset.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48933/diff/4-5/
(Assignee)

Comment 119

3 years ago
Comment on attachment 8745219 [details]
MozReview Request: Bug 1267524 Part 17 - Use member initializer list for nsStyleVariables.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48935/diff/4-5/
(Assignee)

Comment 120

3 years ago
Comment on attachment 8747573 [details]
MozReview Request: Bug 1267524 Part 18 - Remove "void" from zero argument functions.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49937/diff/3-4/
(Assignee)

Comment 121

3 years ago
Comment on attachment 8747574 [details]
MozReview Request: Bug 1267524 Part 19 - Move nsStyleCoord members to initializer list.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49939/diff/3-4/
Class member initializer lists are not supported by MSVC 2013, which we still support. This should be backed out.
(Assignee)

Comment 124

3 years ago
Mike, many structs in nsStyleStruct.cpp already use initializer list in the constructor like [1]. This is the C++98 syntax. The only C++11 syntax I used is the list-initialization for an array in the constructor [2]. However, "Initializer lists" is supported by visual studio 2013 according to [3]. Could you elaborate a bit more? Thanks.

[1] https://dxr.mozilla.org/mozilla-central/rev/fc15477ce628599519cb0055f52cc195d640dc94/layout/style/nsStyleStruct.cpp#2838
[2] https://hg.mozilla.org/integration/mozilla-inbound/rev/bae0edbcd259#l1.164
[3] https://msdn.microsoft.com/en-US/library/hh567368.aspx
Flags: needinfo?(mh+mozilla)
(Assignee)

Comment 125

3 years ago
If the list-initialization for an array in Part 19 is causing trouble for msvc 2013, I could do a follow-up to remove the syntax from Part 19.
I got this error building with 2013: https://msdn.microsoft.com/en-us/library/9f53ks1w.aspx
Flags: needinfo?(mh+mozilla)
(In reply to Ting-Yu Lin [:TYLin] (UTC+8) from comment #125)
> If the list-initialization for an array in Part 19 is causing trouble for
> msvc 2013, I could do a follow-up to remove the syntax from Part 19.

Yes, please, because it did break it, and I imagine I am not the only person currently blocked with a non-functioning build.

Is there a follow up bug for this?  This is a large patch, it would be difficult to back out, so please get a fix in another bug.  Since we knew about this problem, we shouldn't have landed it in the first place, but it is too late for that.
Flags: needinfo?(tlin)
Flags: needinfo?(cam)
The error in question (there may be more once these are cleared):

 0:33.57 c:/Users/msreckovic/Repos/mozilla-central/layout/style/nsStyleStruct.cpp(2909) : error C2536: 'nsStyleDisplay::nsStyleDisplay::mTransformOrigin' : cannot specify explicit initializer for arrays
 0:33.57 c:\users\msreckovic\repos\mozilla-central\layout\style\nsStyleStruct.h(2578) : see declaration of 'nsStyleDisplay::mTransformOrigin'
 0:33.57 c:/Users/msreckovic/Repos/mozilla-central/layout/style/nsStyleStruct.cpp(2909) : error C2536: 'nsStyleDisplay::nsStyleDisplay::mPerspectiveOrigin' : cannot specify explicit initializer for arrays
 0:33.57         c:\users\msreckovic\repos\mozilla-central\layout\style\nsStyleStruct.h(2580) : see declaration of 'nsStyleDisplay::mPerspectiveOrigin'
 0:33.57 c:/Users/msreckovic/Repos/mozilla-central/layout/style/nsStyleStruct.cpp(2974) : error C2536: 'nsStyleDisplay::nsStyleDisplay::mTransformOrigin' : cannot specify explicit initializer for arrays
 0:33.57 c:\users\msreckovic\repos\mozilla-central\layout\style\nsStyleStruct.h(2578) : see declaration of 'nsStyleDisplay::mTransformOrigin'
 0:33.57 c:/Users/msreckovic/Repos/mozilla-central/layout/style/nsStyleStruct.cpp(2974) : error C2536: 'nsStyleDisplay::nsStyleDisplay::mPerspectiveOrigin' : cannot specify explicit initializer for arrays
 0:33.57 c:\users\msreckovic\repos\mozilla-central\layout\style\nsStyleStruct.h(2580) : see declaration of 'nsStyleDisplay::mPerspectiveOrigin'
(In reply to Milan Sreckovic [:milan] from comment #129)
> (In reply to Ting-Yu Lin [:TYLin] (UTC+8) from comment #125)
> > If the list-initialization for an array in Part 19 is causing trouble for
> > msvc 2013, I could do a follow-up to remove the syntax from Part 19.
> 
> Yes, please, because it did break it, and I imagine I am not the only person
> currently blocked with a non-functioning build.
> 
> Is there a follow up bug for this?  This is a large patch, it would be
> difficult to back out, so please get a fix in another bug.  Since we knew
> about this problem, we shouldn't have landed it in the first place, but it
> is too late for that.

Looks like a fix was already pushed to inbound (see comment 127).
Could you rebase to inbound and check again?
Flags: needinfo?(tlin)
Flags: needinfo?(milan)
Flags: needinfo?(cam)
(Assignee)

Comment 133

3 years ago
Sorry for breaking msvc 2013. The fix is landed on m-c now.
Randomly, I still can't build, but it's probably a different bug.  Don't have time to chase it down, but it's in nsStyleStruct.h.  Now that we're moving back to VC 2013 for the official builds, I imagine this will get sorted out quickly :)
Flags: needinfo?(milan)
(Assignee)

Updated

2 years ago
Blocks: 1309813
You need to log in before you can comment on or make changes to this bug.