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

Several unit tests for spvBinaryParse #28

Merged
merged 1 commit into from
Nov 27, 2015

Conversation

dneto0
Copy link
Collaborator

@dneto0 dneto0 commented Nov 24, 2015

Add a non-zero spv_result_t value SPV_REQUESTED_TERMINATION
which should be used to signal an ok result, but signals
early termination for a process, such as binary parsing.

Tests include:

  • correct contents sent to header and instruction callbacks
  • non-zero status from a callback should terminate parsing,
    but the parser should not generate its own diagnostic.

TODO: Check diagnostics generated by the parser itself.

// Returns true if two spv_parsed_operand_t values are equal.
// To use this operator, this definition must appear in the same namespace
// as spv_parsed_operand_t.
static bool operator==(const spv_parsed_operand_t& a,
Copy link

Choose a reason for hiding this comment

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

Didn't we say we'll use anonymous namespace instead of static?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, but that doesn't compile, as alluded to in the blurb comment.

This is the error you get if you put the operator in the anonymous namespace instead:

stl_algobase.h: In instantiation of ‘static bool std::__equal<_BoolType>::equal(_II1, _II1, _II2) [with II1 = const spv_parsed_operand_t; II2 = const spv_parsed_operand_t; bool _BoolType = false]’:
...
/sr/include/c++/4.8/bits/stl_algobase.h:797:22: error: no match for ‘operator==’ (operand types are ‘const spv_parsed_operand_t’ and ‘const spv_parsed_operand_t’)

and about 630 more lines of errors showing you various things it tried.

The problem is that spv_parse_operand_t is declared outside of any namespace. So an equality operator on it must be in the same namespace.

Copy link

Choose a reason for hiding this comment

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

Ah, right, that makes sense. Hopefully the C++ interface from #2 will obviate the need for local operator== everywhere.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I expect that.

@dekimir
Copy link

dekimir commented Nov 27, 2015

+2

Add a non-zero spv_result_t value SPV_REQUESTED_TERMINATION
which should be used to signal an ok result, but signals
early termination for a process, such as binary parsing.

Tests include:
 - correct contents sent to header and instruction callbacks
 - non-zero status from a callback should terminate parsing,
   but the parser should not generate its own diagnostic.

TODO: Check diagnostics generated by the parser itself.
@dneto0 dneto0 merged commit 750f205 into KhronosGroup:master Nov 27, 2015
@dneto0
Copy link
Collaborator Author

dneto0 commented Nov 27, 2015

Squashed and pushed into master.

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.

None yet

4 participants