Skip to content

Conversation

N-Dekker
Copy link
Contributor

@N-Dekker N-Dekker commented May 7, 2025

What does this change do?

Allowed users of the tomlplusplus module to compile using MSVC.

Without TOML_DISABLE_NOEXCEPT_NOEXCEPT, MSVC appears to produce the following error:

include\toml++\impl\value.hpp(270,39): error C3878: syntax error: unexpected token ',' following 'simple-type-specifier'
missing one of: '(' '{' ?
include\toml++\impl\value.hpp(270,39):
the template instantiation context (the oldest one first) is
include\toml++\impl\parser.inl(2757,26):
while compiling class template member function 'toml::v3::value<int64_t>::value<int64_t,0>(int64_t &&) noexcept()'

Is it related to an existing bug report or feature request?

Pre-merge checklist

  • 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

Allowed users of the tomlplusplus module to compile using MSVC.

Without `TOML_DISABLE_NOEXCEPT_NOEXCEPT`, MSVC appears to produce the following error:

> include\toml++\impl\value.hpp(270,39): error C3878: syntax error: unexpected token ',' following 'simple-type-specifier'
>    missing one of: '(' '{' ?
>    include\toml++\impl\value.hpp(270,39):
>    the template instantiation context (the oldest one first) is
>        include\toml++\impl\parser.inl(2757,26):
>        while compiling class template member function 'toml::v3::value<int64_t>::value<int64_t,0>(int64_t &&) noexcept(<expr>)'
Comment on lines -269 to +273
explicit value(Args&&... args) noexcept(noexcept(value_type(
impl::native_value_maker<value_type, std::decay_t<Args>...>::make(static_cast<Args&&>(args)...))))
explicit value(Args&&... args)
#if !TOML_DISABLE_NOEXCEPT_NOEXCEPT
noexcept(noexcept(value_type(
impl::native_value_maker<value_type, std::decay_t<Args>...>::make(static_cast<Args&&>(args)...))))
#endif
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the only use of noexcept(noexcept(<expression>)) in toml++, and while a regular MSVC build appears to be able to parse it, it fails when using the tomlplusplus module (doing a C++20 compilation).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Honestly, MSVC does compile noexcept(noexcept(<expression>)) in other situations. I don't know exactly in which particular situations noexcept(noexcept(<expression>)) triggers an error, but this is one of them 🤷

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Still "Draft" because I haven't yet figured out if this is a known compiler bug. Otherwise, maybe it should be reported!

The added value of this particular noexcept specifier looks rather small, right? 🤷 So then, is it OK to put it inside an #if ... #endif?

Copy link
Owner

Choose a reason for hiding this comment

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

Hmmn, might be able to work around this by extracting the inner noexcept() out into a separate typedef or template variable, then using that in the outer noexcept()?

Otherwise yeah I'm OK with a switch to turn it off.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My feeling is that the combination of variadic templates + noexcept(noexcept(...)) + modules is more than MSVC can handle. You can use each feature, just don't combine them 😸

@N-Dekker N-Dekker marked this pull request as ready for review May 15, 2025 08:04
@marzer marzer merged commit 8eb4012 into marzer:master May 15, 2025
11 checks passed
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