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

Fix C4003 NOMINMAX bug #531

Merged
merged 2 commits into from
Jul 25, 2020
Merged

Fix C4003 NOMINMAX bug #531

merged 2 commits into from
Jul 25, 2020

Conversation

cugone
Copy link
Contributor

@cugone cugone commented Jul 22, 2020

Closes #530
Closes #96

This fixes the std::min and std::max functions being parsed as macros when <windows.h> is included and #define NOMINMAX is neither present nor included before all other headers.

Note: There is a use of std::max in the single-include header that does not appear anywhere else in the separated-out code. I could not find where the single-include header is generated to see where it's coming from.

@skypjack skypjack self-assigned this Jul 23, 2020
@skypjack skypjack added the triage pending issue, PR or whatever label Jul 23, 2020
@skypjack skypjack changed the base branch from master to experimental July 24, 2020 09:02
@skypjack
Copy link
Owner

Please, don't update the single include file. It's generated automatically.
Instead, runtime_view.hpp uses std::min. It should be added to the PR.
Also, feel free to add yourself to the AUTHORS file if you like (lexical order).

@cugone
Copy link
Contributor Author

cugone commented Jul 24, 2020

Please, don't update the single include file. It's generated automatically.

I've removed the changes to the single include file.

Instead, runtime_view.hpp uses std::min. It should be added to the PR.

I did a search for "min" in runtime_view.hpp and the only occurrence is a single use of std::min_element. This is okay.

A search for std::max in literally every file reveals the only place std::max appears is in the single include header on line 8821. Where is this being generated from so it can be changed there? Just a thought, is the script used to generate the SIH using an out-dated file version? I ask because there are uses in view.hpp that have their functions declared [[nodiscard]] but the versions of those same functions in the SIH are not declared [[nodiscard]].

@skypjack skypjack added enhancement accepted requests, sooner or later I'll do it and removed triage pending issue, PR or whatever labels Jul 25, 2020
@skypjack
Copy link
Owner

There is a script to generate the single include file. I update it regularly (well, more or less apparently 😄).

@cugone
Copy link
Contributor Author

cugone commented Jul 25, 2020

Is the script part of the repo? If it is, where is it located?

@skypjack
Copy link
Owner

skypjack commented Jul 25, 2020

Yup, scripts. It's amalgamate.py, courtesy of I don't remember what project but there is a copyright note in the file.

@skypjack skypjack merged commit 617fb18 into skypjack:experimental Jul 25, 2020
skypjack pushed a commit that referenced this pull request Jul 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement accepted requests, sooner or later I'll do it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fixing the NOMINMAX C4003 bug EnTT really work on windows ?
2 participants