missing define guard around NOMINMAX

[diablodale]

The header is missing a define guard. Causing cascading macro redefinition problems that can span max, microsoft, and an infinite number of other-party headers.

max-api/include/c74_max_platform.h

Line 104 in c5e4ac0

#define NOMINMAX

Easy fix. Put a guard around it.
PR forthcoming.

[diablodale]

Though before the trivial PR, why is c74_max_platform.h defining NOMINMAX? I see no value, instead potential harm due to already found macro redefinition compiler errors and user/3rd party potentially needing those legacy macros yet this header intrudes in defining such exclusion.

Throughout this repo, there are no occurrences of min( or max( which would trigger a conflict with those legacy minmax macros. Instead, I find explicit occurrences of std::max(, std::min(.

It could be possible that closed-source Cycling74 internal code uses those legacy macros. And in that case an internal closed-source header or compiler switch should define NOMINMAX.

I recommend removing line 104. Removing the define NOMINMAX as it is not needed for this Cycling74 max-api header/library. And removing it will remove problems with conflicts and forced workarounds (for the NOMINMAX workaround).

[tap]

This file has #pragma once which serves as a header guard.

A thorough review of existing code used by all 3rd-parties will be required before safely removing the NOMINMAX, though removing this macro would be ideal if it has no unintended consequences.

[diablodale]

Hi. You slightly misread. The header is missing a define guard.
Not a header guard.
Instead, missing a define guard.

Meaning...it is missing

#ifndef NOMINMAX
#define NOMINMAX
#endif

Naturally, its your headers. However, from a customer's perspective, these headers are new and drastically different. A lot of behaviors/settings/values/defines have changed. Any use of these new headers will require work by a customer. How do I know? Because I tried and already created this issue. ;-) And I chose to not use these new headers because I've got other things more important to do. I'll use new headers on a new project (not existing code).

Given that there is rework...then removing old legacy (like already done elsewhere in these headers) is appropriate. And in fact, it is odd for this old NOMINMAX define to still be there. Its legacy. It shouldn't be there historically (but was) and given these are new headers it shouldn't be there now. Otherwise, why use these new headers? Just use the crusty time-tested ones.

I strongly suggest you consider that if full backwards compat is desired by a customer...then use the 6.1.9 download or 7.x SDK on github. It 100% works.
Otherwise, a customer can choose to use these new headers and by choosing (nothing forced) they also chose some rework.

Then...consider that Max isn't the focus of most customers. Instead, they are building a solution and Max is one of many components. However, think about keeping that NOMINMAX in there. Due to the way the headers/define is setup, this is Max intruding into other areas. Depending on how a project's compilation units are setup, this define could affect widely across the project. But what if other libraries or customer code needs MINMAX (the need that define to not be there). Well...that's not possible. Max headers (which don't need the define) are for a bizarre reason defining that macro. But this forces all the other customer code to comply.

Max (doesn't need/use it) but Max forces it. That make no logical sense. Max has no business or stake in NOMINMAX. So why define it? It seems a mistake. And if it is for legacy reasons, then that's a mistake also. These are new headers which require rework if used on existing code.

Following my thinking here?