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

add TOML_DISABLE_CONDITIONAL_NOEXCEPT_LAMBDA workaround MSVC error C2057 #247

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

N-Dekker
Copy link
Contributor

@N-Dekker N-Dekker commented Feb 6, 2025

Offered a workaround to compile errors like "error C2057: expected constant expression", when using the "legacy lambda processor" of Visual C++. Such compile errors were reported by Kevin Dick, Jan 19, 2024, at issue #219

  • I've read CONTRIBUTING.md
  • I've rebased my changes against the current HEAD of origin/master (if necessary)
  • I've added new test cases to verify my change
  • I've regenerated toml.hpp (how-to)
  • I've updated any affected documentation
  • I've rebuilt and run the tests with at least one of:
    • Clang 8 or higher
    • GCC 8 or higher
    • MSVC 19.20 (Visual Studio 2019) or higher
  • I've added my name to the list of contributors in README.md

// Define this macro as a workaround to MSVC 2019/2022 "error C2057: expected constant expression",
// as reported by Kevin Dick, Jan 19, 2024, at https://github.com/marzer/tomlplusplus/issues/219
// These compile errors were caused by a bug in the old "legacy lambda processor" of Visual C++.
#ifndef TOML_DISABLE_CONDITIONAL_NOEXCEPT_LAMBDA
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, if you have a look in the "user defines" section of preprocessor.hpp you'll see that all the "bool-like" #define compiler switches are implemented in terms of them being zero or non-zero, not simply defined or undefined.

This is based on a personal preference of mine; I don't like #define-based feature tests because they allow this logical inconsistency:

#define FOO_ENABLE_MAGIC_FEATURE 0

The user has defined it, but it's zero; if the feature test is implemented in terms of simply #define, this turns MAGIC_FEATURE on 😅

For for this new switch to fit in with the rest of them, you'd need to:

  1. have this in the user defines section of preprocessor.hpp:
#ifndef TOML_DISABLE_CONDITIONAL_NOEXCEPT_LAMBDA
#define TOML_DISABLE_CONDITIONAL_NOEXCEPT_LAMBDA 0
#endif
  1. change the logical tests to be simply #if !TOML_DISABLE_CONDITIONAL_NOEXCEPT_LAMBDA

@marzer
Copy link
Owner

marzer commented Feb 7, 2025

Oh, hmmn, also it seems like my CI setup in this repo has gotten stale 😅 Guess I should address whatever it's complaining about.

@marzer
Copy link
Owner

marzer commented Feb 7, 2025

Oh, actually, here's something else useful: MSVC actually gives you a way to detect that you're using the old or new preprocessor, per https://learn.microsoft.com/en-us/cpp/preprocessor/predefined-macros?view=msvc-170:

image

It may be sensible to apply this detection to use it to set the default value of your new TOML_DISABLE_CONDITIONAL_NOEXCEPT_LAMBDA, e.g.

#ifndef TOML_DISABLE_CONDITIONAL_NOEXCEPT_LAMBDA
	#if TOML_MSVC && (!defined(_MSVC_TRADITIONAL) || _MSVC_TRADITIONAL)
		#define TOML_DISABLE_CONDITIONAL_NOEXCEPT_LAMBDA 1
	#else
		#define TOML_DISABLE_CONDITIONAL_NOEXCEPT_LAMBDA 0
	#endif
#endif

@N-Dekker N-Dekker force-pushed the TOML_DISABLE_CONDITIONAL_NOEXCEPT_LAMBDA branch from d595082 to 7d5918a Compare February 7, 2025 09:14
Offered a workaround to compile errors like "error C2057: expected constant expression", when using the "legacy lambda processor" of Visual C++. Such compile errors were reported by Kevin Dick, Jan 19, 2024, at issue marzer#219
@N-Dekker N-Dekker force-pushed the TOML_DISABLE_CONDITIONAL_NOEXCEPT_LAMBDA branch from 7d5918a to 9760658 Compare February 7, 2025 09:16
@N-Dekker
Copy link
Contributor Author

N-Dekker commented Feb 7, 2025

Oh, actually, here's something else useful: MSVC actually gives you a way to detect that you're using the old or new preprocessor

Interesting, but does that also detect which lambda processor is being used? (The compile errors are caused by the lambda processor, not the preprocessor.)


Update: With my local MSVC 2022 build of toml++.sln, _MSVC_TRADITIONAL appears 1 for all test projects. Even though none of them need to have the workaround. Sorry 🤷

@marzer Are you able to reproduce the original problem? (The compile errors when using the legacy lambda processor.)

@marzer
Copy link
Owner

marzer commented Feb 7, 2025

Oh, yeah, never mind. I was mixing up pre-processor with lambda processor. D'oh. Doesn't look like they have any magic macro for indicating the lambda processor that's in use.

@@ -951,7 +951,10 @@ TOML_NAMESPACE_START
static_cast<node_ref>(static_cast<Array&&>(arr)[i])
.visit(
[&]([[maybe_unused]] auto&& elem) //
// Define this macro as a workaround to compile errors caused by a bug in MSVC's "legacy lambda processor".
#if !TOML_DISABLE_CONDITIONAL_NOEXCEPT_LAMBDA
noexcept(for_each_is_nothrow_one<Func&&, Array&&, decltype(elem)>::value)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For what it's worth, I did try to work around the problem (locally) by introducing a variable template for_each_is_nothrow_one_v, in order to do:

    noexcept( for_each_is_nothrow_one_v<Func&&, Array&&, decltype(elem)> )

But it still did not make the old legacy lambda processor happy. 🤷

@N-Dekker N-Dekker marked this pull request as ready for review February 7, 2025 10:14
@N-Dekker
Copy link
Contributor Author

N-Dekker commented Feb 7, 2025

FYI, The legacy lambda processor errors are still there "by default", when generating a vcxproj file for a C++17 build by the latest version of CMake (version 3.31.5) and using the latest VS2022 update (version 17.12.4). Specifically, CMake generates project files that have Conformance mode = Default, which implies using the legacy lambda processor.

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