use MSVC preprocessor version when choosing macro versions in Tf's preprocessorUtilsLite.h

[mattyjams]

Hello!

When building with relatively recent versions of MSVC, one of two different versions of preprocessor may be in use. The older "traditional" preprocessor may in some cases treat macros differently versus other toolchains, particularly in cases that are officially undefined according to the language standard. In the past, this required specific versions of macros for MSVC. The newer, more standards-conforming preprocessor exhibits behavior much more closely aligned with other toolchains.

This new preprocessor was available in experimental form as early as Visual Studio 2017 version 15.8, but it is considered feature complete as of Visual Studio 2019 version 16.5. The compiler option /Zc:preprocessor can be used to enable the new preprocessor.

Enabling the new preprocessor currently causes compilation of OpenUSD to fail however, since it does not handle the custom set of macros for MSVC the same way as the traditional preprocessor. Here is an example of the kind of errors and warnings that are emitted:

C:\Users\MattJohnson\Developer\OpenUSD\pxr\base\tf\debugCodes.h(37,1): error C4002: too many arguments for function-like macro invocation 'TF_PP_CAT_IMPL' [D:\OpenUSD_test_inst\build\OpenUSD\pxr\base\tf\tf.vcxproj]
  (compiling source file 'C:/Users/MattJohnson/Developer/OpenUSD/pxr/base/tf/scriptModuleLoader.cpp')
  C:\Users\MattJohnson\Developer\OpenUSD\pxr\base\tf\debug.h(394,1):
  in expansion of macro 'TF_DEBUG_CODES'
  C:\Users\MattJohnson\Developer\OpenUSD\pxr\base\tf\debug.h(432,1):
  in expansion of macro 'TF_CONDITIONALLY_COMPILE_TIME_ENABLED_DEBUG_CODES'
  C:\Users\MattJohnson\Developer\OpenUSD\pxr\base\tf\preprocessorUtilsLite.h(297,1):
  in expansion of macro 'TF_PP_FOR_EACH'
  C:\Users\MattJohnson\Developer\OpenUSD\pxr\base\tf\preprocessorUtilsLite.h(40,1):
  in expansion of macro 'TF_PP_CAT'

C:\Users\MattJohnson\Developer\OpenUSD\pxr\base\tf\debugCodes.h(53,1): warning C5103: pasting '"TF_DISCOVERY_TERSE"' and '"TF_DISCOVERY_DETAILED"' does not result in a valid preprocessing token [D:\OpenUSD_test_inst\build\OpenUSD\pxr\base\tf\tf.vcxproj]
  (compiling source file 'C:/Users/MattJohnson/Developer/OpenUSD/pxr/base/tf/scriptModuleLoader.cpp')
  C:\Users\MattJohnson\Developer\OpenUSD\pxr\base\tf\debug.h(394,1):
  in expansion of macro 'TF_DEBUG_CODES'
  C:\Users\MattJohnson\Developer\OpenUSD\pxr\base\tf\debug.h(432,1):
  in expansion of macro 'TF_CONDITIONALLY_COMPILE_TIME_ENABLED_DEBUG_CODES'
  C:\Users\MattJohnson\Developer\OpenUSD\pxr\base\tf\preprocessorUtilsLite.h(297,1):
  in expansion of macro 'TF_PP_FOR_EACH'
  C:\Users\MattJohnson\Developer\OpenUSD\pxr\base\tf\preprocessorUtilsLite.h(40,1):
  in expansion of macro 'TF_PP_CAT'
  C:\Users\MattJohnson\Developer\OpenUSD\pxr\base\tf\preprocessorUtilsLite.h(35,1):
  in expansion of macro 'TF_PP_CAT_IMPL'

This patch was used to enable MSVC's new preprocessor:

diff --git a/cmake/defaults/msvcdefaults.cmake b/cmake/defaults/msvcdefaults.cmake
index 0f7fb7ef6..682d814a6 100644
--- a/cmake/defaults/msvcdefaults.cmake
+++ b/cmake/defaults/msvcdefaults.cmake
@@ -49,6 +49,9 @@ if (${PXR_STRICT_BUILD_MODE})
     set(_PXR_CXX_FLAGS "${_PXR_CXX_FLAGS} /WX")
 endif()

+# Turn on the conforming preprocessor.
+set(_PXR_CXX_FLAGS "${_PXR_CXX_FLAGS} /Zc:preprocessor")
+
 # The Visual Studio preprocessor does not conform to the C++ standard,
 # resulting in warnings like:
 #

The changes here define ARCH_PREPROCESSOR_MSVC_TRADITIONAL when building with MSVC using its traditional
preprocessor. This then replaces the use of ARCH_COMPILER_MSVC in preprocessorUtilsLite.h so that only in cases where we're
compiling with MSVC and using its traditional preprocessor do we use the alternate macros. Otherwise when using MSVC's newer, more standards-conforming preprocessor, we use the same macros as other toolchains.

See here for more detail about MSVC's preprocessors:
https://learn.microsoft.com/en-us/cpp/preprocessor/preprocessor-experimental-overview

I ran the unit tests for each of two separate builds with these changes in place, once without the patch above causing the traditional preprocessor to be used and again with the patch applied to enable the new preprocessor. All tests passed for both runs.

• I have verified that all unit tests pass with the proposed changes
• I have submitted a signed Contributor License Agreement

[nvmkuruc]

Just noting that this seems similar to #2675

[mattyjams]

Just noting that this seems similar to #2675

Ahh, it is indeed, thanks @nvmkuruc. I somehow missed seeing that one go by.

The only difference between the two looks to be that I didn't change the use of ARCH_COMPILER_MSVC in preprocessorUtils.h (not the "Lite" version) since the new preprocessor seemed to have no trouble with that one. That may have changed since the first PR was opened and may very well be thanks to your work, as it seems like we're close to being able to deprecate the Boost-based macros in there since I don't think I see them being used anywhere anymore.

Anyway, would be happy to see either #2675 or this find its way to being merged if they're agreeable to everyone.

Thanks!

[nvmkuruc]

The only difference between the two looks to be that I didn't change the use of ARCH_COMPILER_MSVC in preprocessorUtils.h (not the "Lite" version) since the new preprocessor seemed to have no trouble with that one. That may have changed since the first PR was opened and may very well be thanks to your work, as it seems like we're close to being able to deprecate the Boost-based macros in there since I don't think I see them being used anywhere anymore.

That's correct! preprocessorUtils is no longer used in OpenUSD (except in its test). In fact, I think thanks to one of the recent dev pushes from @matthewcpp, boost/preprocessor isn't used at all (not sure if it's still transitively required).

Anyway, would be happy to see either #2675 or this find its way to being merged if they're agreeable to everyone.

Agreed!

[jesschimein]

Filed as internal issue #USD-9393

[jesschimein]

/AzurePipelines run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[anthony-linaro]

Hi Folks, was there ever any movement on this?

I'm currently having to work around this issue in blender for Windows ARM64 builds, meaning I can't use sse2neon (which needs the new preprocessor) in any files which also include certain USD headers.

[mattyjams]

Freshening with a rebase on the current dev branch (35dbce1).