Pull the trace struct out of the low-level interface. by vext01 · Pull Request #5 · softdevteam/ykrustc

vext01 · 2019-01-29T16:47:42Z

This is a companion change to softdevteam/yktrace#1

ltratt · 2019-01-29T19:44:15Z

- for i in 0..t.len() {
- let loc = t.loc(i);
+ for i in 0..len {
+ if i > isize::max_value() as usize {


Wouldn't it be easier to hoist this outside the loop? Something like assert!(len < (isize::max_value() as usize)).

ltratt · 2019-01-30T10:23:26Z

Which PR should be done first?

vext01 · 2019-01-30T10:27:34Z

Which PR should be done first?

I think we should decide if we like the API outlined in softdevteam/yktrace#1, and if we do, then we merge this ykrustc PR first, then the yktrace PR. This is because the yktrace PR depends on the compiler changes.

BTW, you may have noticed the FIXME. I'd like to restructure our libcore code at some point, but I didn't want to mix it in with this PR and make it confusing to review. I hope that's OK.

vext01 · 2019-01-30T10:28:47Z

Your comment was addressed in my recent push BTW, so this is also ready for re-review.

ltratt · 2019-01-30T14:19:53Z

The updates look good, but I guess we'll need to wait for the Option change to start_tracing to be fixed here too?

vext01 · 2019-01-30T14:21:09Z

Correct

ltratt · 2019-01-30T15:19:13Z

Do we need to do something to this PR before considering it for merging?

vext01 · 2019-01-30T15:19:53Z

If you have no further comments, we can squash this.

ltratt · 2019-01-30T15:22:28Z

Please squash.

By pulling the struct from here, and moving it into yktrace, we can offer the user a more abstracted interface, including a Drop and an Iterator. Also revise some comments while we are here.

vext01 · 2019-01-30T15:32:11Z

squashed and revised commit message.

ltratt · 2019-01-30T15:32:54Z

bors r+

5: Pull the trace struct out of the low-level interface. r=ltratt a=vext01 This is a companion change to softdevteam/yktrace#1 Co-authored-by: Edd Barrett <vext01@gmail.com>

bors · 2019-01-30T15:34:28Z

Build failed

buildbot/buildbot-build-script

vext01 · 2019-01-30T15:37:22Z

+ ./x.py clean
fatal: ambiguous argument 'HEAD': unknown revision or path not in the working tree.

vext01 · 2019-01-30T15:44:44Z

bors retry

5: Pull the trace struct out of the low-level interface. r=ltratt a=vext01 This is a companion change to softdevteam/yktrace#1 Co-authored-by: Edd Barrett <vext01@gmail.com>

vext01 · 2019-01-30T15:47:13Z

OK it's building on worker1 (it failed on worker4). I restarted all of the workers just in case that's needed to pick up the "fresh clone" settings we changed yesterday.

I'll be keeping an eye on this, as I'm not totally sure if it's fixed it for all workers or not...

bors · 2019-01-30T20:28:29Z

Build succeeded

buildbot/buildbot-build-script

update from origin 2020-06-19

This is a combination of 18 commits. Commit softdevteam#2: Additional examples and some small improvements. Commit softdevteam#3: fixed mir-opt non-mir extensions and spanview title elements Corrected a fairly recent assumption in runtest.rs that all MIR dump files end in .mir. (It was appending .mir to the graphviz .dot and spanview .html file names when generating blessed output files. That also left outdated files in the baseline alongside the files with the incorrect names, which I've now removed.) Updated spanview HTML title elements to match their content, replacing a hardcoded and incorrect name that was left in accidentally when originally submitted. Commit softdevteam#4: added more test examples also improved Makefiles with support for non-zero exit status and to force validation of tests unless a specific test overrides it with a specific comment. Commit softdevteam#5: Fixed rare issues after testing on real-world crate Commit softdevteam#6: Addressed PR feedback, and removed temporary -Zexperimental-coverage -Zinstrument-coverage once again supports the latest capabilities of LLVM instrprof coverage instrumentation. Also fixed a bug in spanview. Commit softdevteam#7: Fix closure handling, add tests for closures and inner items And cleaned up other tests for consistency, and to make it more clear where spans start/end by breaking up lines. Commit softdevteam#8: renamed "typical" test results "expected" Now that the `llvm-cov show` tests are improved to normally expect matching actuals, and to allow individual tests to override that expectation. Commit softdevteam#9: test coverage of inline generic struct function Commit softdevteam#10: Addressed review feedback * Removed unnecessary Unreachable filter. * Replaced a match wildcard with remining variants. * Added more comments to help clarify the role of successors() in the CFG traversal Commit softdevteam#11: refactoring based on feedback * refactored `fn coverage_spans()`. * changed the way I expand an empty coverage span to improve performance * fixed a typo that I had accidently left in, in visit.rs Commit softdevteam#12: Optimized use of SourceMap and SourceFile Commit softdevteam#13: Fixed a regression, and synched with upstream Some generated test file names changed due to some new change upstream. Commit softdevteam#14: Stripping out crate disambiguators from demangled names These can vary depending on the test platform. Commit softdevteam#15: Ignore llvm-cov show diff on test with generics, expand IO error message Tests with generics produce llvm-cov show results with demangled names that can include an unstable "crate disambiguator" (hex value). The value changes when run in the Rust CI Windows environment. I added a sed filter to strip them out (in a prior commit), but sed also appears to fail in the same environment. Until I can figure out a workaround, I'm just going to ignore this specific test result. I added a FIXME to follow up later, but it's not that critical. I also saw an error with Windows GNU, but the IO error did not specify a path for the directory or file that triggered the error. I updated the error messages to provide more info for next, time but also noticed some other tests with similar steps did not fail. Looks spurious. Commit softdevteam#16: Modify rust-demangler to strip disambiguators by default Commit softdevteam#17: Remove std::process::exit from coverage tests Due to Issue #77553, programs that call std::process::exit() do not generate coverage results on Windows MSVC. Commit softdevteam#18: fix: test file paths exceeding Windows max path len

Don't run `resolve_vars_if_possible` in `normalize_erasing_regions` Neither `@eddyb` nor I could figure out what this was for. I changed it to `assert_eq!(normalized_value, infcx.resolve_vars_if_possible(&normalized_value));` and it passed the UI test suite. <details><summary> Outdated, I figured out the issue - `needs_infer()` needs to come _after_ erasing the lifetimes </summary> Strangely, if I change it to `assert!(!normalized_value.needs_infer())` it panics almost immediately: ``` query stack during panic: #0 [normalize_generic_arg_after_erasing_regions] normalizing `<str::IsWhitespace as str::pattern::Pattern>::Searcher` #1 [needs_drop_raw] computing whether `str::iter::Split<str::IsWhitespace>` needs drop softdevteam#2 [mir_built] building MIR for `str::<impl str>::split_whitespace` softdevteam#3 [unsafety_check_result] unsafety-checking `str::<impl str>::split_whitespace` softdevteam#4 [mir_const] processing MIR for `str::<impl str>::split_whitespace` softdevteam#5 [mir_promoted] processing `str::<impl str>::split_whitespace` softdevteam#6 [mir_borrowck] borrow-checking `str::<impl str>::split_whitespace` softdevteam#7 [analysis] running analysis passes on this crate end of query stack ``` I'm not entirely sure what's going on - maybe the two disagree? </details> For context, this came up while reviewing rust-lang/rust#77467 (cc `@lcnr).` Possibly this needs a crater run? r? `@nikomatsakis` cc `@matthewjasper`

ltratt reviewed Jan 29, 2019
View reviewed changes

vext01 mentioned this pull request Jan 30, 2019
Flesh out the initial tracing SHIM API. softdevteam/yktrace#1
Merged

Pull the trace struct out of the low-level interface.
ca660b4
By pulling the struct from here, and moving it into yktrace, we can offer the user a more abstracted interface, including a Drop and an Iterator. Also revise some comments while we are here.

vext01 force-pushed the yk-yktrace-integration branch from 395dbf9 to ca660b4Compare January 30, 2019 15:31

borsBot added a commit that referenced this pull request Jan 30, 2019
Merge#5
1219d0c
5: Pull the trace struct out of the low-level interface. r=ltratt a=vext01 This is a companion change to softdevteam/yktrace#1 Co-authored-by: Edd Barrett <vext01@gmail.com>

borsBot added a commit that referenced this pull request Jan 30, 2019
Merge#5
03c5d50
5: Pull the trace struct out of the low-level interface. r=ltratt a=vext01 This is a companion change to softdevteam/yktrace#1 Co-authored-by: Edd Barrett <vext01@gmail.com>

borsBot merged commit ca660b4 into masterJan 30, 2019

borsBot deleted the yk-yktrace-integration branch January 30, 2019 20:28

borsBot pushed a commit that referenced this pull request Jul 9, 2020
Merge pull request#5from rust-lang/master
791cccc
update from origin 2020-06-19

Uh oh!

Conversation

vext01 commented Jan 29, 2019

Uh oh!

ltrattJan 29, 2019

Choose a reason for hiding this comment

Uh oh!

vext01Jan 30, 2019

Choose a reason for hiding this comment

Uh oh!

ltratt commented Jan 30, 2019

Uh oh!

vext01 commented Jan 30, 2019

Uh oh!

vext01 commented Jan 30, 2019

Uh oh!

ltratt commented Jan 30, 2019

Uh oh!

vext01 commented Jan 30, 2019

Uh oh!

ltratt commented Jan 30, 2019

Uh oh!

vext01 commented Jan 30, 2019

Uh oh!

ltratt commented Jan 30, 2019

Uh oh!

vext01 commented Jan 30, 2019

Uh oh!

ltratt commented Jan 30, 2019

Uh oh!

borsBot commented Jan 30, 2019

Build failed

Uh oh!

vext01 commented Jan 30, 2019

Uh oh!

vext01 commented Jan 30, 2019

Uh oh!

vext01 commented Jan 30, 2019

Uh oh!

borsBot commented Jan 30, 2019

Build succeeded

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants