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

Windows: fatal error C1001: An internal error has occurred in the compiler. #596

Closed
wilhelmberg opened this issue Feb 12, 2016 · 22 comments
Closed
Milestone

Comments

@wilhelmberg
Copy link
Contributor

mapnik@master and node-mapnik@master as of now.

 c:\mb\windows-builds-64\packages\node-mapnik\mapnik-sdk\include\mapnik/symbolizer_base.hpp(104): note: see reference to function
   template instantiation 'mapnik::util::variant<mapnik::value_bool,mapnik::value_integer,mapnik::enumeration_wrapper,mapnik::valu
  e_double,std::string,mapnik::color,mapnik::expression_ptr,mapnik::path_expression_ptr,mapnik::transform_type,mapnik::text_placem
  ents_ptr,mapnik::dash_array,mapnik::raster_colorizer_ptr,mapnik::group_symbolizer_properties_ptr,mapnik::font_feature_settings>:
  :variant<const char*&,mapbox::util::detail::value_traits<T,mapnik::value_bool,mapnik::value_integer,mapnik::enumeration_wrapper,
  mapnik::value_double,std::string,mapnik::color,mapnik::expression_ptr,mapnik::path_expression_ptr,mapnik::transform_type,mapnik:
  :text_placements_ptr,mapnik::dash_array,mapnik::raster_colorizer_ptr,mapnik::group_symbolizer_properties_ptr,mapnik::font_featur
  e_settings>,void>(const char *&) noexcept' being compiled
          with
          [
              T=const char *&
          ] (compiling source file ..\src\mapnik_map.cpp)
..\src\mapnik_vector_tile.cpp(2706): warning C4101: 'ex': unreferenced local variable [c:\mb\windows-builds-64\packages\node-mapni k\build\mapnik.vcxproj]
..\src\mapnik_vector_tile.cpp(3650): fatal error C1001: An internal error has occurred in the compiler. [c:\mb\windows-builds-64\p ackages\node-mapnik\build\mapnik.vcxproj]
  (compiler file 'msc1.cpp', line 1421)
   To work around this problem, try simplifying or changing the program near the locations listed above.
  Please choose the Technical Support command on the Visual C++
   Help menu, or open the Technical Support help file for more information
  ..\src\mapnik_vector_tile.cpp(3650): note: while compiling class template member function 'mapnik::util::variant<Image *,CairoSu
  rface *,Grid *>::variant<nullptr,mapbox::util::detail::value_traits<T,Image *,CairoSurface *,Grid *>,void>(nullptr &&) noexcept(
  <expr>)'
          with
          [
              T=nullptr
          ]
  Internal Compiler Error in C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\bin\amd64\CL.exe.  You will be prompted to sen
  d an error report to Microsoft later.
  INTERNAL COMPILER ERROR in 'C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\bin\amd64\CL.exe'
      Please choose the Technical Support command on the Visual C++
      Help menu, or open the Technical Support help file for more information
cl : Command line error D8040: error creating or communicating with child process [c:\mb\windows-builds-64\packages\node-mapnik\bu ild\mapnik.vcxproj]

Possible hints:

@wilhelmberg
Copy link
Contributor Author

Doesn't seem to be the optimization.

Currently we don't have optimization specified explicitly, but tried with

  • 0: off
  • 2: speed
  • 3: full

same error.

@lightmare
Copy link

Alas, more ICE! I'd try this to make it easier for the compiler:

@springmeyer
Copy link
Member

@BergWerkGIS - I just looked into this a bit. I'm feeling like the use of mapbox::variant here is not mandatory. If this is the only place this compiler problem is happening we could workaround by using raw pointers and dropping variant usage. Of course that would just dodge the problem and it is likely worth fully fixing in variant - just really hard to do that given we'd need to reduce a testcase for what is happening and what is happening is a compiler crash not a clear code error.

So I think the best route here is to troubleshoot this in node-mapnik. Could you create a branch of node-mapnik that points at a Mapnik master SDK then ping me? I'll then poke by committing to that branch and letting appveyor tell me if we've dodged the issue.

@springmeyer
Copy link
Member

Could you create a branch of node-mapnik that points at a Mapnik master SDK then ping me?

@BergWerkGIS - nevermind! I got this and bumped to start testing in 238035e

@springmeyer
Copy link
Member

That commit passed: 238035e. I assumed it would fail and the original compiler error would be replicated. Not sure why not....

@springmeyer
Copy link
Member

@BergWerkGIS - let me know when you've had a chance to test to see if you can still replicate this.

@wilhelmberg
Copy link
Contributor Author

@springmeyer sorry, forgot to report back yesterday.

Tried again today on three different machines.

Built all deps, mapnik and node-mapnik from scratch.

Interestingly node-mapnik fails on all of them when using the locally created mapnik SDK.
Also tried [email protected]/0.12.7/5.1.0 -> all fail.

However, if I use mapnik-win-sdk-v3.0.9-246-gd751731-x64-14.0.7z (created 3 days ago by the same scripts), which also gets pulled in by AppVeyor, the build succeeds.


My current hunch: Visual Studio 2015 Update 1 which I have installed on all local machines but neither on our build server nor on AppVeyor.

Maybe Update 1 creates a mapnik SDK that does work on its own but not in conjunction with node-mapnik (clash of some optimization or other settings???).

Also interesting that Visual Studio 2015 Update 1 is able to build node-mapnik with a mapnik SDK created by stock Visual Studio 2015.

@wilhelmberg
Copy link
Contributor Author

Could this be why node-mapnik is failing to build on Windows and that VS is unable to determine dependent names apart from non-dependent names?

Partial Support for Expression SFINAE in VS 2015 Update 1

Especially the "unsupported section".

@lightmare
Copy link

Can you confirm that the error is not caused by initializing the variant with nullptr?

Regardless of the above, I think this is where no_init might actually be useful. But for that to work, visitation of uninitialized variant would have to do something other than attempt (and fail) to visit the last alternative. I'll submit an issue at variant.

@springmeyer
Copy link
Member

@lightmare - good clues.

@BergWerkGIS - you were going to diff out the differences in the Mapnik SDK that crashes vs not - what was the outcome of that investigation?

Also @BergWerkGIS @artemp will ping you in the morning to try debugging interactively with a few ideas that might help.

@artemp
Copy link
Member

artemp commented Feb 24, 2016

Here is my rough plan to try tomorrow with @BergWerkGIS :

  • First thing first - Lets get a diff to see if code is different between local build and AppVeyor
    /cc @BergWerkGIS ^.
  • Second - lets try something like this to avoid initialising variant with nullptr
struct dummy {};
using surface_type = mapnik::util::variant<
    dummy,
    Image *
  , CairoSurface *
#if defined(GRID_RENDERER)
  , Grid *
#endif
  >;

In vector_tile_render_baton_t constructor replace surface(nullptr) with surface() + amend struct deref_visitor

  • Third - storing raw pointers in variant doesn't look like a good idea anyways

@wilhelmberg
Copy link
Contributor Author

Diff: Not exactly sure how that happened, but old variant made it into the SDK that already should have contained a new one.

I updated the build chain with a new SDK.
Now AppVeyor fails with the same error:

 ..\src\mapnik_vector_tile.cpp(3650): note: while compiling class template member function 'mapnik::util::variant<Image *,CairoSurface *,Grid *>::variant<nullptr,mapbox::util::detail::value_traits<T,Image *,CairoSurface *,Grid *>,void>(nullptr &&) noexcept(<expr>)'
          with
          [
              T=nullptr
          ]
  Internal Compiler Error in C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\bin\amd64\CL.exe.  You will be prompted to send an error report to Microsoft later.
cl : Command line error D8040: error creating or communicating with child process [C:\projects\node-mapnik\build\mapnik.vcxproj]

https://ci.appveyor.com/project/Mapbox/node-mapnik/build/1.0.1242/job/m3n76h4mg5oqinjp#L405

@wilhelmberg
Copy link
Contributor Author

Forgot, all tests passed with https://github.com/mapbox/variant/compare/crazy_usage

@wilhelmberg
Copy link
Contributor Author

Can you confirm that the error is not caused by initializing the variant with nullptr?

y, also found a reference that VS compiler doesn't like that, but haven't checked yet.

@springmeyer
Copy link
Member

Okay, thanks @BergWerkGIS - so sounds like:

  • The reason it was tricky to replicate is different variant versions at play
  • It is the new (and latest) variant release that is problematic

Therefore despite this being a compiler crash (a compiler should never crash) I think variant is likely partly to blame. So the idea above with @artemp is a good next step. I also see that @lightmare created mapbox/variant#93 which might be another thing to try. Both might workaround the problem.

I presume that you can easily test workarounds by:

  • Manually editing the variant.hpp inside the Mapnik SDK
  • Rebuilding node-mapnik

So: you should not need to recompile Mapnik core to iterate on testing tomorrow.

@lightmare
Copy link

The MSVC compiler has a really hard time with some C++11 features, noexcept being one of them. Yesterday I tried inheriting variant's constructors in image_view_any and got another ICE lightmare/mapnik@84f16bb -- might be completely unrelated, but the error mentioned a constructor template with noexcept...

@wilhelmberg
Copy link
Contributor Author

All ✅

Only change needed was:

surface(nullptr) => surface()

https://ci.appveyor.com/project/Mapbox/node-mapnik/build/1.0.1243
https://travis-ci.org/mapnik/node-mapnik/builds/111685697

Maybe compiler was not able to infer the type of Nan::ObjectWrap::CairoSurface(nullptr) object?


Alsot tried mapbox/variant#93 but that crashed the compiler like before (with un-modified node-mapik).

@artemp
Copy link
Member

artemp commented Feb 25, 2016

@BergWerkGIS - sounds good ^ .
Using an extra Dummy type would be a better solution to avoid default constructed Image.

^ disregard, I'm not fully awake yet. Default constructed Image* would be a nullptr :)
Anyways, I'll push a change in a few.

artemp added a commit that referenced this issue Feb 25, 2016
…ult contructor in `vector_tile_render_baton_t` (ref #596)
artemp added a commit that referenced this issue Feb 25, 2016
@lightmare
Copy link

Alsot tried mapbox/variant#93 but that crashed the compiler like before (with un-modified node-mapik).

That was not supposed to fix the issue by itself. It will only allow you to keep variant uninitialized and not blow up when visited, i.e.:

surface(mapbox::util::no_init) // instead of surface(nullptr)
struct deref_visitor
{
    void operator() (/* no arg*/) {} // no-op, this is called when uninitialized
    template <typename SurfaceType>
    void operator() (SurfaceType * surface)
    ...
};

edit: btw, if MSVC has trouble matching nullptr to pointer type, you can probably solve that by telling it explicitly surface((Image*)nullptr)

@springmeyer
Copy link
Member

Thanks for everyones efforts here. So before closing this a few followup questions:

@springmeyer springmeyer added this to the 3.4.17 milestone Feb 25, 2016
@artemp
Copy link
Member

artemp commented Feb 25, 2016

Only change needed was: surface(nullptr) => surface()

because relying on nullptr is a not a smart idea and it breaks strong typing and trips MSVC etc etc. So having a dummy struct is a totally sound approach in this case. It also easy to understand and follow in my opinion. As @lightmare suggested above no_init might be a better fit but I'd like to review changes to variant, also we'll need to do a formal release I guess aka 1.1.1 ?

@springmeyer ^

@springmeyer
Copy link
Member

Thanks @artemp. Sounds like then:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants