Defend public headers against inclusion of Windows.h (#1719)

* Defend public headers against inclusion of Windows.h

... which defines some nasty function-like macros `min` and `max` when `_NOMINMAX` isn't defined. We prevent expansion as a function-like macro by inserting some token(s) between `min`/`max` and the following `(`. Most commonly that means wrapping the entire qualified-name in `()` a la `(std::min)(x, y)`, but an explicit template argument list (`std::min<int>(x, y)`) works as well.

* clang-format all the things

* Test coverage

I assume that the `azure-meow-common` headers are fully covered by the tests for the `azure-meow-woof` SDKs.

sdk/core/azure-core/inc/azure/core/context.hpp

@@ -272,7 +272,9 @@ namespace Azure { namespace Core {
         return time_point() + static_cast<std::chrono::milliseconds>(msec);
       }
 
-      explicit ContextSharedState() : CancelAtMsecSinceEpoch(ToMsecSinceEpoch(time_point::max())) {}
+      explicit ContextSharedState() : CancelAtMsecSinceEpoch(ToMsecSinceEpoch((time_point::max)()))
+      {
+      }
 
       explicit ContextSharedState(
           const std::shared_ptr<ContextSharedState>& parent,
@@ -330,7 +332,7 @@ namespace Azure { namespace Core {
     Context WithValue(const std::string& key, ContextValue&& value) const
     {
       return Context{std::make_shared<ContextSharedState>(
-          m_contextSharedState, time_point::max(), key, std::move(value))};
+          m_contextSharedState, (time_point::max)(), key, std::move(value))};
     }
 
     /**
@@ -389,7 +391,7 @@ namespace Azure { namespace Core {
     void Cancel()
     {
       m_contextSharedState->CancelAtMsecSinceEpoch
-          = ContextSharedState::ToMsecSinceEpoch(time_point::min());
+          = ContextSharedState::ToMsecSinceEpoch((time_point::min)());
     }
 
     /**

sdk/core/azure-core/inc/azure/core/http/body_stream.hpp

@@ -263,7 +263,7 @@ namespace Azure { namespace Core { namespace Http {
      * @param max_length Maximum number of bytes to provide to the readers.
      */
     LimitBodyStream(BodyStream* inner, int64_t max_length)
-        : m_inner(inner), m_length(std::min(inner->Length(), max_length))
+        : m_inner(inner), m_length((std::min)(inner->Length(), max_length))
     {
     }
 

sdk/core/azure-core/test/ut/CMakeLists.txt

@@ -40,6 +40,7 @@ add_executable (
     http.cpp
     json.cpp
     logging.cpp
+    macro_guard.cpp
     main.cpp
     match_conditions.cpp
     md5.cpp

sdk/core/azure-core/test/ut/macro_guard.cpp

@@ -0,0 +1,15 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+// Define `min` and `max` as function-like macros before including all public
+// headers to ensure that uses of those identifiers are defended against
+// expansion as function-like macros. Define `small` as an object-like macro to
+// ensure that identifier isn't used at all. Windows.h is badly behaved and
+// defines similar macros with these names and we want to ensure the SDK headers
+// function even when a naive user includes Windows.h first.
+//
+#define small FAIL><TO][COMPILE)(VERY{{{LOUDLY!!!
+#define max(x, y) small
+#define min(x, y) small
+
+#include <azure/core.hpp>

sdk/identity/azure-identity/test/ut/CMakeLists.txt

@@ -13,6 +13,7 @@ include(GoogleTest)
 
 add_executable (
   azure-identity-test
+    macro_guard.cpp
     main.cpp
     simplified_header.cpp
 )

sdk/identity/azure-identity/test/ut/macro_guard.cpp

@@ -0,0 +1,15 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+// Define `min` and `max` as function-like macros before including all public
+// headers to ensure that uses of those identifiers are defended against
+// expansion as function-like macros. Define `small` as an object-like macro to
+// ensure that identifier isn't used at all. Windows.h is badly behaved and
+// defines similar macros with these names and we want to ensure the SDK headers
+// function even when a naive user includes Windows.h first.
+//
+#define small FAIL><TO][COMPILE)(VERY{{{LOUDLY!!!
+#define max(x, y) small
+#define min(x, y) small
+
+#include <azure/identity.hpp>

sdk/keyvault/azure-security-keyvault-keys/test/ut/CMakeLists.txt

@@ -13,6 +13,7 @@ include(GoogleTest)
 add_executable (
   azure-security-keyvault-keys-test
   key_client_test.cpp
+  macro_guard.cpp
   main.cpp
   mocked_transport_adapter_test.hpp
   telemetry_header_test.cpp

sdk/keyvault/azure-security-keyvault-keys/test/ut/macro_guard.cpp

@@ -0,0 +1,15 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+// Define `min` and `max` as function-like macros before including all public
+// headers to ensure that uses of those identifiers are defended against
+// expansion as function-like macros. Define `small` as an object-like macro to
+// ensure that identifier isn't used at all. Windows.h is badly behaved and
+// defines similar macros with these names and we want to ensure the SDK headers
+// function even when a naive user includes Windows.h first.
+//
+#define small FAIL><TO][COMPILE)(VERY{{{LOUDLY!!!
+#define max(x, y) small
+#define min(x, y) small
+
+#include <azure/keyvault/key_vault.hpp>

sdk/storage/azure-storage-blobs/CMakeLists.txt

@@ -96,6 +96,7 @@ if(BUILD_TESTING)
         test/ut/blob_service_client_test.cpp
         test/ut/block_blob_client_test.cpp
         test/ut/block_blob_client_test.hpp
+        test/ut/macro_guard.cpp
         test/ut/page_blob_client_test.cpp
         test/ut/page_blob_client_test.hpp
         test/ut/storage_retry_policy_test.cpp

sdk/storage/azure-storage-blobs/test/ut/macro_guard.cpp

@@ -0,0 +1,15 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+// Define `min` and `max` as function-like macros before including all public
+// headers to ensure that uses of those identifiers are defended against
+// expansion as function-like macros. Define `small` as an object-like macro to
+// ensure that identifier isn't used at all. Windows.h is badly behaved and
+// defines similar macros with these names and we want to ensure the SDK headers
+// function even when a naive user includes Windows.h first.
+//
+#define small FAIL><TO][COMPILE)(VERY{{{LOUDLY!!!
+#define max(x, y) small
+#define min(x, y) small
+
+#include <azure/storage/blobs.hpp>

sdk/storage/azure-storage-common/inc/azure/storage/common/concurrent_transfer.hpp

@@ -36,7 +36,7 @@ namespace Azure { namespace Storage { namespace Details {
           break;
         }
         int64_t chunkOffset = offset + chunkSize * chunkId;
-        int64_t chunkLength = std::min(length - chunkSize * chunkId, chunkSize);
+        int64_t chunkLength = (std::min)(length - chunkSize * chunkId, chunkSize);
         try
         {
           transferFunc(chunkOffset, chunkLength, chunkId, numChunks);

sdk/storage/azure-storage-files-datalake/CMakeLists.txt

@@ -100,6 +100,7 @@ if(BUILD_TESTING)
         test/datalake_sas_test.cpp
         test/datalake_service_client_test.cpp
         test/datalake_service_client_test.hpp
+        test/macro_guard.cpp
   )
 
   target_link_libraries(azure-storage-test PRIVATE azure-storage-files-datalake)

sdk/storage/azure-storage-files-datalake/test/macro_guard.cpp

@@ -0,0 +1,15 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+// Define `min` and `max` as function-like macros before including all public
+// headers to ensure that uses of those identifiers are defended against
+// expansion as function-like macros. Define `small` as an object-like macro to
+// ensure that identifier isn't used at all. Windows.h is badly behaved and
+// defines similar macros with these names and we want to ensure the SDK headers
+// function even when a naive user includes Windows.h first.
+//
+#define small FAIL><TO][COMPILE)(VERY{{{LOUDLY!!!
+#define max(x, y) small
+#define min(x, y) small
+
+#include <azure/storage/files/datalake.hpp>

sdk/storage/azure-storage-files-shares/CMakeLists.txt

@@ -86,6 +86,7 @@ if(BUILD_TESTING)
   target_sources(
     azure-storage-test
       PRIVATE
+        test/macro_guard.cpp
         test/share_client_test.cpp
         test/share_client_test.hpp
         test/share_directory_client_test.cpp

sdk/storage/azure-storage-files-shares/test/macro_guard.cpp

@@ -0,0 +1,15 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+// Define `min` and `max` as function-like macros before including all public
+// headers to ensure that uses of those identifiers are defended against
+// expansion as function-like macros. Define `small` as an object-like macro to
+// ensure that identifier isn't used at all. Windows.h is badly behaved and
+// defines similar macros with these names and we want to ensure the SDK headers
+// function even when a naive user includes Windows.h first.
+//
+#define small FAIL><TO][COMPILE)(VERY{{{LOUDLY!!!
+#define max(x, y) small
+#define min(x, y) small
+
+#include <azure/storage/files/shares.hpp>

sdk/template/azure-template/test/CMakeLists.txt

@@ -13,6 +13,7 @@ include(GoogleTest)
 
 add_executable (
      azure-template-test
+     ut/macro_guard.cpp
      ut/template_test.cpp
      )
 
@@ -23,4 +24,3 @@ if (MSVC)
 endif()
 
 gtest_discover_tests(azure-template-test TEST_PREFIX azure-template.)
-

sdk/template/azure-template/test/ut/macro_guard.cpp

@@ -0,0 +1,15 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+// Define `min` and `max` as function-like macros before including all public
+// headers to ensure that uses of those identifiers are defended against
+// expansion as function-like macros. Define `small` as an object-like macro to
+// ensure that identifier isn't used at all. Windows.h is badly behaved and
+// defines similar macros with these names and we want to ensure the SDK headers
+// function even when a naive user includes Windows.h first.
+//
+#define small FAIL><TO][COMPILE)(VERY{{{LOUDLY!!!
+#define max(x, y) small
+#define min(x, y) small
+
+#include <azure/template.hpp>