1321094 - ANGLE's chromium/2924 doesn't build on VS2015u2

Status: RESOLVED WONTFIX

(Core :: Graphics, defect, P1)

Description

[Kelsey Gilbert [:jgilbert]]

VS2015u3 isn't required yet, so we should see how involved patching the build errors is.

Comment 2

[Peter Chang[:pchang]]

Attachment: Bug 1321094 - Change angle::Format from constexpr to const to pass build with VS2015u2,

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

Comment 3

[Peter Chang[:pchang]]

 Philip,
   Could you help to confirm attachment 8815609 [details] work well with VS2015u2 in your side? It works in my machine with VS2015u2.

Comment 4

[Peter Chang[:pchang]]

(In reply to Peter Chang[:pchang] from comment #2)
> Created attachment 8815609 [details]
> Bug 1321094 - Change angle::Format from constexpr to const to pass build
> with VS2015u2,
> 
> Review commit: https://reviewboard.mozilla.org/r/96480/diff/#index_header
> See other reviews: https://reviewboard.mozilla.org/r/96480/

I believe there were some compiler fixes in VS2015u3 to get build passed without my patch.

From MSDN[1], The primary difference between const and constexpr variables is that the initialization of a const variable can be deferred until run time whereas a constexpr variable must be initialized at compile time.

Therefore, I think my change (for beta51) should be fine(using constexpr should save some time in run time phase).

[1]https://msdn.microsoft.com/en-us/library/dn956974.aspx

Comment 5

[Philip Chee]

(In reply to Peter Chang[:pchang] from comment #3)
>  Philip,
>    Could you help to confirm attachment 8815609 [details] work well with
> VS2015u2 in your side? It works in my machine with VS2015u2.
Unfortunately I have since applied Update 3 so I cannot test.

Comment 6

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8815609 [details]
Bug 1321094 - Change angle::Format from constexpr to const to pass build with VS2015u2,

https://reviewboard.mozilla.org/r/96480/#review96944

Sweet.

Comment 7

[Peter Chang[:pchang]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d227c2a82af37264f25d894e21875c59a9b57453&selectedJob=32120269

The try with beta repo looks good.

Comment 8

[Peter Chang[:pchang]]

Ryan, I would like to uplift this patch only for beta. Any concern? The try result is in comment 7.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Funny story, the logs from that Try push show vs2015u3 being used. It turns out that's Beta is also using vs2015u3 (bug 1283203)!
https://hg.mozilla.org/releases/mozilla-beta/file/2dec3c6c7c90e2e27093b8a3512c1b32a8263a8f/browser/config/tooltool-manifests/win32/releng.manifest

So, if you really want to test your patch against vs2015u2, you'll need to revert rev 90a82f2bfb3a first. Or we can make a case for landing bug 1319901 on 51+ once the patch is ready. Sorry for the hassle :(

Comment 10

[Peter Chang[:pchang]]

I see, then I don't think we need this patch.