Skip to content

Refactor validation failure and printing, validate atomic alignment#1090

Merged
dschuff merged 5 commits intomasterfrom
valid_atomic
Jul 13, 2017
Merged

Refactor validation failure and printing, validate atomic alignment#1090
dschuff merged 5 commits intomasterfrom
valid_atomic

Conversation

@dschuff
Copy link
Member

@dschuff dschuff commented Jul 12, 2017

No description provided.


namespace wasm {

// Print anything that can be streamed to an ostream
Copy link
Member

Choose a reason for hiding this comment

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

perhaps these two could go in wasm-printing.h?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about that but they are kind of specific to the validator where we want to easily make everything print with the extra type info. Otherwise they sort of just duplicate the stream-related functions there.

Copy link
Member

Choose a reason for hiding this comment

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

oh, i see. sounds good.

shouldBeEqualOrFirstIsUnreachable(curr->value->type, curr->valueType, curr, "store value type must match");
}
void WasmValidator::visitAtomicRMW(AtomicRMW* curr) {
//validateAlignment(curr->
Copy link
Member

Choose a reason for hiding this comment

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

unneeded comment?

//validateAlignment(curr->
}
void WasmValidator::visitAtomicCmpxchg(AtomicCmpxchg* curr) {
}
Copy link
Member

Choose a reason for hiding this comment

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

if there's nothing to validate, these methods can just not be defined?

case 2:
case 4:
break;
case 8:
Copy link
Member

Choose a reason for hiding this comment

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

the style in other places is

case 8: {
  ...
  break;
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

@jgravelle-google
Copy link
Contributor

Looks generally good modulo @kripken's comments. Nice

@dschuff dschuff merged commit 25cbf64 into master Jul 13, 2017
@dschuff dschuff deleted the valid_atomic branch September 6, 2017 17:12
@dschuff dschuff mentioned this pull request Sep 7, 2018
10 tasks
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.

3 participants