New issue
Advanced search Search tips

Issue 595189 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug

Blocking:
issue 440500



Sign in to add a comment

VS 2015 Update 2 RC fails on texture_definition.cc

Project Member Reported by brucedaw...@chromium.org, Mar 16 2016

Issue description

From https://bugs.chromium.org/p/chromium/issues/detail?id=440500#c322 and #c323 - a report that the compiler complains that gpu::gles2::GLStreamTextureImage is not defined when instantiating scoped_refptr<>::AddRef.

Investigations show that this repros with Update 2 RC, but not with the checked in toolchain. It turns out that what matters is not what compiler does the compiling, but what compiler does the preprocessing. That is, the difference is in the header files.

I reduced the code to this minimal source file:

#include <vector>
#include "base/memory/ref_counted.h"

class NotDefined;

struct LevelInfo2 {
  scoped_refptr<NotDefined> stream_texture_image;
};

struct FaceInfo2 {
  FaceInfo2();
  FaceInfo2(const FaceInfo2& other);
  ~FaceInfo2();

  std::vector<LevelInfo2> level_infos;
};

struct Texture2 {
  std::vector<FaceInfo2> face_infos_;
};

void UpdateTextureInternal(Texture2* texture) {
  texture->face_infos_.resize(1);
}

The resize() line is the crucial one - it compiles with Update 1 header files but not with Update 2 RC header files.

 
Cc: thakis@chromium.org
thakis@ - any thoughts on whether this code is correct? Resizing a vector of objects that contains a scoped_refptr<NotDefined> seems odd, but I don't know if it's wrong.
Possible workaround available here:

https://codereview.chromium.org/1806743002/

A more scoped workaround is probably possible.
The compile error occurs because the Update 2 version of vector causes the definition of GLStreamTextureImage to be required, for some reason. The fix in https://codereview.chromium.org/1806743002/ works by making the definition available.
Update: the VC++ team has confirmed that this is actually a compiler bug that is triggered by the new <vector> header file, rather than a bug in <vector>. The bug may get fixed for Update 2 final, or maybe not.

A temporary workaround still seems appropriate.
I now get updates on compiler bugs spontaneously sent on twitter:

https://twitter.com/StephanTLavavej/status/710191543840219136
https://twitter.com/StephanTLavavej/status/710180339214262272

So I now know that it's a compiler bug that is triggered by a type trait used in std::vector for error checking an optimization.
Project Member

Comment 7 by bugdroid1@chromium.org, Mar 17 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4d9ecc15b1ca04d30113408a4d2c428456176ce0

commit 4d9ecc15b1ca04d30113408a4d2c428456176ce0
Author: brucedawson <brucedawson@chromium.org>
Date: Thu Mar 17 02:03:53 2016

Fix VS 2015 Update 2 specific bug

When building Chrome with VS 2015 Update 2 RC there is a compiler error:

1>  texture_definition.cc
1>base/memory/ref_counted.h(414): error C2027: use of undefined type 'gpu::gles2::GLStreamTextureImage'
1>  gpu/command_buffer/service/texture_manager.h(31): note: see declaration of 'gpu::gles2::GLStreamTextureImage'
1>  base/memory/ref_counted.h(413): note: while compiling class template member function 'void scoped_refptr<gpu::gles2::GLStreamTextureImage>::AddRef(T *)'

This is caused by a compiler bug, together with a type trait used in
std::vector:
https://twitter.com/StephanTLavavej/status/710191543840219136

A VS bug has been filed at:
https://connect.microsoft.com/VisualStudio/feedback/details/2475971

 Bug 595189  has a more minimal repro.

BUG= 440500 , 595189 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel

Review URL: https://codereview.chromium.org/1806743002

Cr-Commit-Position: refs/heads/master@{#381640}

[modify] https://crrev.com/4d9ecc15b1ca04d30113408a4d2c428456176ce0/gpu/command_buffer/service/gl_stream_texture_image.h
[modify] https://crrev.com/4d9ecc15b1ca04d30113408a4d2c428456176ce0/gpu/command_buffer/service/texture_definition.cc

I'm leaving this open to remind me to undo the workaround when Update 2 RTW (which will have a fix) ships.

Comment 9 by thakis@chromium.org, Mar 21 2016

Blocking: 440500
Update 2 has the fix for this bug. That is confirmed in the connect bug and by building with Update 2 with the workaround removed.

I'll switch us to Update 2 and then decide when to remove the workaround.
Labels: -Pri-2 Pri-3
Project Member

Comment 12 by bugdroid1@chromium.org, Feb 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/39e9ce2fb50f025d2176f24e90d3f2ea15a940fd

commit 39e9ce2fb50f025d2176f24e90d3f2ea15a940fd
Author: brucedawson <brucedawson@chromium.org>
Date: Tue Feb 28 21:19:19 2017

Revert of Fix VS 2015 Update 2 specific bug (patchset #3 id:40001 of https://codereview.chromium.org/1806743002/ )

Reason for revert:
This was a compiler bug workaround and the bug was fixed long ago. Note that
the #include "gpu/gpu_export.h" line that was added for the workaround is
being left in because it is now needed.

Original issue's description:
> Fix VS 2015 Update 2 specific bug
>
> When building Chrome with VS 2015 Update 2 RC there is a compiler error:
>
> 1>  texture_definition.cc
> 1>base/memory/ref_counted.h(414): error C2027: use of undefined type 'gpu::gles2::GLStreamTextureImage'
> 1>  gpu/command_buffer/service/texture_manager.h(31): note: see declaration of 'gpu::gles2::GLStreamTextureImage'
> 1>  base/memory/ref_counted.h(413): note: while compiling class template member function 'void scoped_refptr<gpu::gles2::GLStreamTextureImage>::AddRef(T *)'
>
> This is caused by a compiler bug, together with a type trait used in
> std::vector:
> https://twitter.com/StephanTLavavej/status/710191543840219136
>
> A VS bug has been filed at:
> https://connect.microsoft.com/VisualStudio/feedback/details/2475971
>
>  Bug 595189  has a more minimal repro.
>
> BUG= 440500 , 595189 
> CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
>
> Committed: https://crrev.com/4d9ecc15b1ca04d30113408a4d2c428456176ce0
> Cr-Commit-Position: refs/heads/master@{#381640}

# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 440500 , 595189 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2710683014
Cr-Commit-Position: refs/heads/master@{#453704}

[modify] https://crrev.com/39e9ce2fb50f025d2176f24e90d3f2ea15a940fd/gpu/command_buffer/service/texture_definition.cc

Status: Fixed (was: Assigned)
Workaround removed, nothing left to do.

Sign in to add a comment