Could not parse `event_id`. Found ASCII control character in <text>

[jyn514]

I am not sure whether this is a rustc bug or a measureme bug so I'm reporting it here under the theory that it's more likely to be seen by the right people :)

I ran

x clean rustc_middle && MAGIC_EXTRA_RUSTFLAGS='-Zself-profile -Zself-profile-events=default,query-keys' x check --stage 0 rustc_middle

on https://github.com/rust-lang/rust/commit/ba6f5e3b4d60ea5a847cd4402cca594cd40b218f. That generated a .mm_profdata file that the measureme tools fail to parse:

; crox rustc_middle-2790234.mm_profdata
Could not parse `event_id`. Found ASCII control character in <text> at 112 in "erase_regions_ty^^impl for<'a, 'b> FnOnce(&'b mut DiagnosticBuilder<'a, ()>)
-> &'b mut DiagnosticBuilder<'a, ()>"

It looks like FnOnce impls are getting an extra newline generated at the end. That seems like a bug we should fix in rustc, but not such a bad bug that it should prevent measureme from generating a report - could we translated it to an ASCII space instead, maybe?

[jyn514]

The relevant code looks like https://github.com/rust-lang/rust/blob/3df55382d46842dcaeff48ea83e0107183550dd6/compiler/rustc_middle/src/ty/print/pretty.rs#L999-L1003, which ... seems fine?

cc @compiler-errors, I see you were the last one to touch that pretty-printing code.

[compiler-errors]

If this is the consequence of the pretty printing infrastructure, then I'd expect we'd see in in more places than just measureme.

[compiler-errors]

So it wasn't rustc_middle::ty::pretty but it was rustc_ast_pretty's fault, since the impl Trait in the examples given by jyn were APITs.

[michaelwoerister]

I don't remember the details of the encoding but I do remember that it assigns special meaning to certain ASCII control characters in order to be space efficient. I doubt, however, that newlines are affected by that.

I can look into relaxing the checks here, but unless we consider it high priority, it will probably be a while before I get to that.

[bjorn3]

I doubt, however, that newlines are affected by that.

Yes they are. I did actually run into this while first wiring up cranelift's profiling system to -Zself-profile in cg_clif. Initially I just replaced all \n with | to workaround this, but also changed cranelift's profiling system to allow a proper integration later.

[michaelwoerister]

I doubt, however, that newlines are affected by that.

What I mean is: I doubt that newlines really have a reserved meaning in the encoding -- so it's probably fine to relax the requirements here 🙂

[bjorn3]

Should this issue be kept open for when someone does need a newline in an argument?

[andjo403]

@michaelwoerister made a suggestion last night but forgot to post the PR. posted it now. if you have something better in mind I can close it

[bjorn3]

It got closed again by a subtree sync...