Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Workaround MSVC __VA_ARGS__ weirdness #2514

Merged
merged 8 commits into from
Oct 23, 2019

Conversation

thejcannon
Copy link
Contributor

@thejcannon thejcannon commented Oct 11, 2019

Should fix #2490 and the errors after #2498 was merged.

The good news is we no longer need GMOCK_PP_INTERNAL_USE_MSVC 😄

Also see this Godbolt link which shows everyone agrees on the preprocessed output after this change (on MSVC, it's printed to stdout, so scroll to the bottom of the stdout output) (Stale now, but we're taking the same approach)

Comment on lines 659 to 664
// TODO(thejcannon): This just tests that this will compile, as gmock repeating the
// noexcept specifier isn't supported yet
struct MockUsesNoexceptWithParenthesis
{
MOCK_METHOD(void, func, (), (noexcept(false)));
};
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'll just delete this when we re-introduce #2498

@Corristo
Copy link

Just tested it, it does not fix #2490. However, I don't think it can be fixed in googletest itself, as it is a bug in clang's ms-compatibility mode. (https://bugs.llvm.org/show_bug.cgi?id=43282)

@thejcannon
Copy link
Contributor Author

Just tested it, it does not fix #2490. However, I don't think it can be fixed in googletest itself, as it is a bug in clang's ms-compatibility mode. (https://bugs.llvm.org/show_bug.cgi?id=43282)

Well that's odd as both clang and MSVC natively now produce the right result, I would expect the "compatibility mode" to not matter. But also not surprised, these things are gross.

@thejcannon
Copy link
Contributor Author

@gennadiycivil Is there anything missing barring this from going to internal review?

@vslashg vslashg self-assigned this Oct 22, 2019
@vslashg
Copy link
Member

vslashg commented Oct 22, 2019

Thanks, we've started the internal review. Please avoid making further changes to the PR, as they might get lost in the process.

vslashg added a commit that referenced this pull request Oct 23, 2019
vslashg added a commit that referenced this pull request Oct 23, 2019
@vslashg vslashg merged commit e1b67b0 into google:master Oct 23, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MOCK_METHOD macro not working with clang-cl
4 participants