-
Notifications
You must be signed in to change notification settings - Fork 7
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
Improve textual notation of Call targets. #49
Conversation
bors r+ |
49: Improve textual notation of Call targets. r=ptersilie a=vext01 The textual notation for a Call target is kind of weird. If there's a known symbol, it looks like this: ``` call operand=sym_name=_ZN50_$LT$T$u20$as$u20$core..convert..Into$LT$U$GT$$GT$4into17hcac79ab93d213ca6E, ret_bb=bb23 ``` I think `operand=sym_name=` looks weird. This change kills `sym_name=` and in case of an unknown target uses `<unknown>` (instead of `unknown`). Co-authored-by: Edd Barrett <[email protected]>
Build failed |
That's new... |
|
Yeah, I wondered if it might be related to that. Next week I'll be investigating. At this point I'm not sure what a "dynamic test" is :P |
Dynamic tests are for example used by |
I can confirm that the failure is caused by the abort strategy. The failing tests are the benchmark tests marked |
And here's the problem: you cannot do benchmark tests with the abort strategy It looks like normal tests and benchmark tests get mixed into the same binary, so we can't build just benchmark tests with the unwind strategy and the others with the abort strategy. And besides, we probably do want to allow benchmarks of our tracer. So we might have to honour the unwind strategy after all. @ptersilie @ltratt any thoughts on this? |
This reverts commit c8c964a. The reason for this is that benchmark tests require the unwind strategy and since benchmarks are mixed with normal tests in the same binary, it means that all of our tests would have to use the unwind strategy. See: ykjit/yk#49 (comment)
92: Revert "Use the abort strategy when tracing." r=ptersilie a=vext01 This reverts commit c8c964a. The reason for this is that benchmark tests require the unwind strategy and since benchmarks are mixed with normal tests in the same binary, it means that all of our tests would have to use the unwind strategy. See: ykjit/yk#49 (comment) Co-authored-by: Edd Barrett <[email protected]>
I've reverted the switch to the abort strategy for now. bors try |
tryBuild succeeded |
@ptersilie this is ready for merge. |
bors r+ |
bors ping |
pong |
bors r+ |
Build succeeded |
The textual notation for a Call target is kind of weird.
If there's a known symbol, it looks like this:
I think
operand=sym_name=
looks weird.This change kills
sym_name=
and in case of an unknown target uses<unknown>
(instead ofunknown
).