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

Mixed _WIN32/WIN32 definitions causes compiler error over mkdir defintion in verilated.cpp in Visual Studio #1406

Closed
veripoolbot opened this issue Mar 4, 2019 · 5 comments

Comments

@veripoolbot
Copy link

@veripoolbot veripoolbot commented Mar 4, 2019


Author Name: Benjamin Gartner
Original Redmine Issue: 1406 from https://www.veripool.org

Original Assignee: Wilson Snyder (@wsnyder)


I get the following compile error when compiling verilated.cpp in Visual Studio.
Error C2039 'mkdir': is not a member of '`global namespace'' verilated.cpp 1783

The offending code seems to be here. Visual Studio defines _WIN32 and not WIN32 so it doesn't include direct.h and has no definition for mkdir. Looks like this was added in changeset c93d280 recently, so presumably this was tested in a cygwin environment that defines both.

#if defined(WIN32) || defined(__MINGW32__)
1. include <direct.h>  // mkdir
#endif

Easy enough to define WIN32 in Visual Studio, but probably confusing to figure out that's what's needed. Probably as easy as changing to the following, assuming this doesn't break on cygwin environments:

#if defined(WIN32) || defined(__MINGW32__)
1. include <direct.h>  // mkdir
#endif

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Mar 4, 2019


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-03-04T23:33:48Z


For the fixed version did you mean?

#if defined(WIN32) || defined(_WIN32) || defined(__MINGW32__)
1. include <direct.h>  // mkdir
#endif

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Mar 5, 2019


Original Redmine Comment
Author Name: Benjamin Gartner
Original Date: 2019-03-05T00:51:57Z


Oops.

I actually meant

#if defined(_WIN32)
1. include <direct.h>  // mkdir
#endif

because I'm not sure why we need to check WIN32 || or MINGW32 here. Everywhere else in the codebase uses the symbol _WIN32 and that worked fine for me when I built it under MSYS2, but I'm no expert on cygwin. But your proposed version would certainly fix the issue under Visual Studio. I just don't know if the other boolean terms are actually necessary.

Thanks for the response!

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Mar 5, 2019


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-03-05T01:30:22Z


You're right, these two should be changed from WIN32 to _WIN32, everywhere else _WIN32 was used.

Fixed in git towards 4.012.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Mar 5, 2019


Original Redmine Comment
Author Name: Benjamin Gartner
Original Date: 2019-03-05T14:39:39Z


Thanks! Works in my environment.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Mar 24, 2019


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-03-24T01:15:46Z


In 4.012.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.