Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Visual Studio 2017 CE error #4488

Closed
MikeGarrod opened this issue Apr 6, 2018 · 10 comments
Closed

Visual Studio 2017 CE error #4488

MikeGarrod opened this issue Apr 6, 2018 · 10 comments
Assignees
Labels

Comments

@MikeGarrod
Copy link

After building a MSVC 2017 Solution file using

cmake -G "Visual Studio 15 2017 Win64" - DCMAKE_INSTALL_PREFIX=../../../../install ../..

I am getting a number of compilation errors in type_traits.cc at line 527, The offending code looks like

_INLINE_VAR constexpr bool is_convertible_v = is_convertible<_From, _To>::value;

And the 6 errors all complain "A native array cannot contain this type"

Full error below... (There are actually 6 similar errors

Error C2728
'google::protobuf::compiler::cpp::FieldGenerator': a native array cannot contain this type (compiling source file D:\Dev\Libraries\protobuf\src\google\protobuf\compiler\cpp\cpp_field.cc)

Project: libprotoc

File: c:\program files (x86)\microsoft visual studio\2017\community\vc\tools\msvc\14.13.26128\include\type_traits

Line: 525

I am pretty new to this project and was hoping that someone could point out what I am missing.

Thanks

@acozzette acozzette self-assigned this Apr 9, 2018
@acozzette acozzette added the c++ label Apr 9, 2018
@acozzette
Copy link
Member

@MikeGarrod Which version or Git commit are you using? Does the full error message say something about what line in cpp_field.cc is causing the problem?

@MikeGarrod
Copy link
Author

My mistake, I pulled the latest Master, Release 3.5.1.1 does not have the same issue.

Thanks

@acozzette
Copy link
Member

@MikeGarrod No problem, I'm glad it's working for you now.

@rcane
Copy link
Contributor

rcane commented Apr 11, 2018

The problematic code is the allocation of arrays of unique_ptr like in java_field.cc(214):

template <>
FieldGeneratorMap<ImmutableFieldGenerator>::FieldGeneratorMap(...)
  : descriptor_(descriptor),
    field_generators_(new std::unique_ptr<
      ImmutableFieldGenerator>[descriptor->field_count()]) {
...
}

The code itself is not wrong but apparently triggers a bug in Visual C++ 2017 15.6 (see here). Although the VS bug was supposedly fixed already the problem still persists with the current VS 2017 15.6.6.

@acozzette
Copy link
Member

@rcane Thanks for tracking down the problem! Do you think we should try to work around this or just wait for the compiler fix? I'm not really familiar with Visual Studio and so I'm not sure how their release process works.

@rcane
Copy link
Contributor

rcane commented Apr 12, 2018

Good question. The bug was reported 4 month ago. They said they had it fixed three month ago. Since then there have been 6 service releases of VS 2017 15.6.x. And neither one of them contained the fix. So it might be in the next service release (which they do approximately every week) or it will come in the next minor release (which might come next month). So a workaround might not be such a bad idea.

A possible workaround could be using a std::vector<std::unique_ptr<...>> instead of a the unique_ptr<unique_ptr []> construct for the various generator arrays.

So for example in cpp_field.h change

std::unique_ptr<std::unique_ptr<FieldGenerator> []> field_generators_;

into

std::vector<std::unique_ptr<FieldGenerator>> field_generators_;

And in the implementation the constructor needs to be changed from

field_generators_(
      new std::unique_ptr<FieldGenerator>[descriptor->field_count()]) {

to

field_generators_(descriptor->field_count()) {

The same pattern can be applied to java_field, java_file, and objectivec_field.

Although this makes the all code compile I did not have the time to actually test it.

@acozzette
Copy link
Member

That seems like a good idea. Aside from the compiler bug your change is nice anyway since it gets rid of the double unique_ptr indirection. Would you be interested in sending us a pull request for this?

@rcane
Copy link
Contributor

rcane commented Apr 13, 2018

I can send you a pull request in the next couple of days.

rcane added a commit to rcane/protobuf that referenced this issue Apr 16, 2018
The current 15.6.x versions of Visual Studio 2017 contain a bug that
prevent them from compiling the following construct under certain
conditions:

std::unique_ptr<std::unique_ptr<Foo> []> foos;

This will fail to compile if Foo is an abstract class. To work-around
the problem the whole construct was change into:

std::vector<std::unique_ptr<Foo>> foos;

This not only fixes the compiler error but is also more readable than
previous version.
@rcane
Copy link
Contributor

rcane commented Apr 16, 2018

I sent the following pull request: #4517

acozzette pushed a commit that referenced this issue Apr 16, 2018
Fixed a Visual Studio 2017 build error. (#4488)
@acozzette
Copy link
Member

@rcane Thanks for the pull request with the fix!

drashnane pushed a commit to drashnane/protobuf that referenced this issue Jun 6, 2018
The current 15.6.x versions of Visual Studio 2017 contain a bug that
prevent them from compiling the following construct under certain
conditions:

std::unique_ptr<std::unique_ptr<Foo> []> foos;

This will fail to compile if Foo is an abstract class. To work-around
the problem the whole construct was change into:

std::vector<std::unique_ptr<Foo>> foos;

This not only fixes the compiler error but is also more readable than
previous version.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants