Skip to content

Fix for clash with Windows "min" and "max" macros - #52

Closed
T-640 wants to merge 1 commit into
martinus:mainfrom
T-640:main
Closed

Fix for clash with Windows "min" and "max" macros#52
T-640 wants to merge 1 commit into
martinus:mainfrom
T-640:main

Conversation

@T-640

@T-640 T-640 commented Oct 18, 2023

Copy link
Copy Markdown
Contributor

Greetings!

Due to Windows having "min" and "max" defined as macros a compile error occurs whenever Windows.h and svector.h headers are included together on that system.

It results in the following error:

...svector.h:137: error: C2589: '(': illegal token on right side of '::'

Here is the code to demonstrate the issue:

#include <Windows.h> // <-- The culprit right here!
#include <iostream>
#include "svector.h"

int main (int, char *[])
{
    ankerl::svector <int, 10> vector {1, 2, 3, 4, 5};
    for (int i : vector)
        std::cout << i << std::endl;
    return 0;
}

The changes proposed simpy add parentheses around statements with std::min and std::max. It appears to be the easiest and least intrusive solution, no need to do things like messing with NOMINMAX macro, for instance.

Further reading:

https://stackoverflow.com/questions/1394132/macro-and-member-function-conflict
https://learn.microsoft.com/en-us/windows/win32/multimedia/min
https://learn.microsoft.com/en-us/windows/win32/multimedia/max

Fixes clash with Windows "min" and "max" macros
@martinus

Copy link
Copy Markdown
Owner

Thanks for this, and sorry for the long delay. The diagnosis and the approach are both right.

I've picked it up in #57, which keeps your commit and adds two things: three more std::min call sites in erase() and the move helper that were still unparenthesized (so erase()/insert() users would still have hit the problem), and space indentation, since .clang-format has UseTab: Never and the lint job rejected the tabs.

Measured with a TU that defines the Windows min/max macros before including the header: main 9 errors, this PR 3, #57 zero.

@martinus martinus closed this in #57 Jul 27, 2026
@martinus

Copy link
Copy Markdown
Owner

Merged as 73c882c via #57 — your commit is in main with your authorship intact. GitHub can't auto-close this because the repo uses rebase-merge, which rewrites SHAs. Thanks, and sorry it took so long.

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

Successfully merging this pull request may close these issues.

2 participants