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

Compiler error with MSVC due to max() and min() #7

Open
srupprecht-tt opened this issue Feb 1, 2024 · 8 comments
Open

Compiler error with MSVC due to max() and min() #7

srupprecht-tt opened this issue Feb 1, 2024 · 8 comments

Comments

@srupprecht-tt
Copy link

Hey there,

I built the repository using the Release 0.0.1-alpha.1 using Visual Studio 17 2022 (Debug x64).
When trying to include the built "aas_core3.dll" into my project, I got some errors within the "types.hpp" header file.
After some tries, I realized that my Visual Studio is confused by the naming since there are standard functions named "min" or "max". I solved this by refactoring all "min"/"max"-functions in the "types.hpp" file to "const_min"/"const_max".
For some reason the alternative, putting "#define NOMINMAX" before including the header, didn't work, maybe I did it wrong.

Best regards

@mristin
Copy link
Contributor

mristin commented Feb 1, 2024

Hi @srupprecht-tt ,
Thanks a lot for the report!

Could you please indicate me the lines which you had to change?

I am aware of this solution to the problem: https://stackoverflow.com/a/22023122/1600678, and actually believed that I always implemented accordingly. Obviously I was wrong.

Could you also try to apply the fix at one of the lines to see if it works on your side?

@mristin
Copy link
Contributor

mristin commented Feb 7, 2024

@srupprecht-tt just a kind reminder: any update on the issue?

@srupprecht-tt
Copy link
Author

Hi @mristin

  1. the files I changed are
    • enhancing.hpp: lines 3798, 3799, 3812, 3813, 7174, 7175, 7216, 7217
    • types.hpp: lines 1795, 1810, 2991, 3030, 6694, 6706, 10321, 10357
    • jsonization.cpp: lines 30897, 30906, 35822, 35828
    • types.cpp: lines 3368, 3382, 6977, 7019
    • verification.cpp: lines 15351, 15353, 15373, 15375, 15502, 15511, 15556, 15565
    • xmlization.cpp: lines 45395, 45439, 53096, 53213
  2. I tried using the (std::...)() solution, but either it didn't work or I didn't do it correctly. Could you explain where exactly I would have to add the "std"-namespace together with brackets? Because in the declaration const common::optional<std::wstring>& max() const override; this should not fit. Maybe I got something wrong.

@mristin
Copy link
Contributor

mristin commented Feb 8, 2024

Thanks, @srupprecht-tt ! I plan to have a look at this tomorrow and let you know what I find out.

@mristin mristin changed the title Error when including .dll with max() and min() Error with MSVC due to max() and min() Feb 10, 2024
@mristin mristin changed the title Error with MSVC due to max() and min() Compiler error with MSVC due to max() and min() Feb 10, 2024
@mristin
Copy link
Contributor

mristin commented Feb 10, 2024

@srupprecht-tt I had a cursory look at it. Unfortunately, my MSVC does not complain, so we will have to use your machine for debugging :-(.

Could you please try to add #define NOMINMAX at line 19 in common.cpp before #include <windows.h>?

I found this StackOverflow question and answers useful: https://stackoverflow.com/questions/4913922/possible-problems-with-nominmax-on-visual-c/4914108#4914108

What really confuses me is that windows.h is only included in common.cpp and not in any other implementation nor header file.

There also seems to be a flag for MSVC: https://stackoverflow.com/a/13416506/1600678, namely -DNOMINMAX. Could you perhaps try that for your particular MSVC version?

It seems to me that your MSVC defines the min and max macros even if no header file is included.

@srupprecht-tt
Copy link
Author

@mristin

  • Adding #define NOMINMAX at l. 19 in common.cpp before #include <windows.h> did not work for some reason. I also tried shifting it before the #pragma warning(push, 0) which also did not work...
  • Adding -DNOMINMAX to the C/C++ command line options works somehow!

I don't know exactly why the one works but the other doesn't and how to include this "bug fix" into your development, but that's good news at least.

@mristin
Copy link
Contributor

mristin commented Feb 14, 2024

Thanks for the information, @srupprecht-tt !

I'll document this issue in the documentation. I don't think there is an easy way around it as I can not reproduce the issue on other compiler. (see my reply below)

@mristin
Copy link
Contributor

mristin commented Feb 14, 2024

I'll check for MSVC in CMake (https://cmake.org/cmake/help/latest/variable/MSVC.html) and then specify -DNOMINMAX with ADD_DEFINITIONS(-DNOMINMAX).

For more info on ADD_DEFINITIONS: https://cmake.org/cmake/help/latest/command/add_definitions.html

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

2 participants