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

Fix ambiguity of a type deduction in TEST_EQ macro for enum class args. #5630

Merged
merged 1 commit into from
Nov 25, 2019

Conversation

vglavnyy
Copy link
Contributor

Before this PR this code (in the test.cpp) was impossible:

enum class TestEnum : int8_t {  A, B };
TEST_EQ(MyGame::Example::TestEnum::A, Foo());

Due to ambiguity of type deduction, this code caused a compilation error like this:

error C2679: binary '<<': no operator found which takes a right-hand operand of type 'T' (or there is no acceptable conversion) with [ T=MyGame::Example::TestEnum ]
.... (listing of all possible specializations)

This is a well-known problem (C++ DR1601):

However, all modern compilers break the rule C++17 16.4.2 introduced by DR 1601:

A conversion that promotes an enumeration whose underlying type is fixed to its underlying type is better than one that promotes to the promoted underlying type, if the two are different.

This PR adds a workaround and allows the use of simple checking of enum class values in tests:

TEST_EQ(MyGame::Example::TestEnum::A, MyGame::Example::TestEnum::B);

@aardappel
Copy link
Collaborator

Actually, CI errors appear related.

@vglavnyy
Copy link
Contributor Author

CI errors are connected with #5611.

TEST_EQ(root_v1_viewed_from_v2->j(), NULL);
TEST_EQ(root_v1_viewed_from_v2->e()->c(), NULL);
TEST_EQ(root_v1_viewed_from_v2->c_as_TableC(), NULL);

NULL is int. I already wrote this in #5611.
I will re-run CI after fixes.

@vglavnyy vglavnyy force-pushed the fix_TestEq_for_enum_class branch from 0c63d1c to b68ec9f Compare November 21, 2019 23:25
@aardappel
Copy link
Collaborator

Still an Android error: https://travis-ci.org/google/flatbuffers/jobs/615311373#L2409

Looks like the STLPort emulation we have is missing operator== for unique_ptr and nullptr or something :(

@vglavnyy vglavnyy force-pushed the fix_TestEq_for_enum_class branch from 37b4f7c to 3bc7b81 Compare November 22, 2019 04:33
@vglavnyy
Copy link
Contributor Author

Looks like the STLPort emulation we have is missing operator== for unique_ptr and nullptr or something :(

@aardappel, fixed, all tests passed.
I'm sorry. I squashed the fix with first commit before review.
Can you review this PR again?

Changes from the initial commit:

  • tests/test_assert.h
      - added missed () int TEST_NOTNULL(),
      - added explicit cast to bool in TEST_ASSERT().
  • stl_emulation.h
      - added operators for comparison with nullptr.

@aardappel
Copy link
Collaborator

Nice, LGTM!

@aardappel aardappel merged commit c3c32ec into google:master Nov 25, 2019
@vglavnyy vglavnyy deleted the fix_TestEq_for_enum_class branch November 26, 2019 15:56
LuckyRu pushed a commit to LuckyRu/flatbuffers that referenced this pull request Oct 2, 2020
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