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.

Sign up for GitHub

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

Silence msvc warning about an unused named parameter #2289

Merged
merged 1 commit into from
May 16, 2021
Merged

Silence msvc warning about an unused named parameter #2289

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions include/fmt/core.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2446,6 +2446,7 @@ constexpr int get_arg_index_by_name(basic_string_view<Char> name) {
}
if constexpr (sizeof...(Args) > 0)
return get_arg_index_by_name<N + 1, Args...>(name);
(void)name;
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like a bug in MSVC because name is used in the line above. I recommend reporting to Microsoft.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so: if the control flow in an instantiation of

template <int N, typename T, typename... Args, typename Char>
constexpr int get_arg_index_by_name(basic_string_view<Char> name)

reaches line 2449, then both if constexpr statements before were evaluated as false and their first (in this case: only) substatements become so called discarded statements [stmt.if]/2 which leave no use of name in that template specialization.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like it was also discussed on Microsoft Developer Community, and resulted in joining the other compilers (GCC and Clang, at least) for these cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for both links!

Regarding the supposed GCC bug: the original poster gave a wrong description of the nature of the problem. He was missing the difference between a runtime if and a compile-time if constexpr as layed out in the standard. And "This is a valid program which should not produce a warning" misses the point of -Wunused-but-set-parameter explicitly requesting a warning.

This is exactly what the last poster on MS DevCon is rightfully complaining about.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've referenced this PR on DevCon. According to my knowledge of the release schedule this will certainly not make it for 16.10 which will probably be around for a long time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, whatever is preferred here. It's just the same as a few lines later.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for investigating and I'm glad that this nonsense has already been reported. Merged this as a workaround for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

"This is a valid program which should not produce a warning" misses the point of -Wunused-but-set-parameter explicitly requesting a warning.

I disagree, using a parameter in one branch of if constexpr is not conceptually different from using it in one branch of if (that may be optimized away). It's great that it was fixed in GCC.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The "misses" is referring to the blanket statement. A valid program may have many good reasons to issue warnings.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, that is true.

return invalid_arg_index;
}
#endif
Expand Down