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

debuginfo: Support for generic functions and self arguments #8554

Closed
wants to merge 12 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@michaelwoerister
Contributor

michaelwoerister commented Aug 16, 2013

This pull request includes support for generic functions and self arguments in methods, and combinations thereof. This also encompasses any kind of trait methods, regular and static, with and without default implementation. The implementation is backed up by a felt ton of test cases :)

This is a very important step towards being able to compile larger programs with debug info, since practically any generic function caused an ICE before.

One point worth discussing is that activating debug info now automatically (and silently) sets the no_monomorphic_collapse flag. Otherwise debug info would show wrong type names in all but one instance of the monomorphized function.

Another thing to note is that the handling of generic types does not strictly follow the DWARF specification. That is, variables with type T (where T=int) are described as having type int and not as having type T. In other words, we are losing information whether a variable has been declared with a type parameter as its type. In practice this should not make much of difference though since the concrete type is mostly what one is interested in. I'll post an issue later so this won't be forgotten.

Also included are a number of bug fixes:

  • Closes #1758
  • Closes #8513
  • Closes #8443
  • Fixes handling of field names in tuple structs
  • Fixes and re-enables test case for option-like enums that relied on undefined behavior before
  • Closes #1339 (should have been closed a while ago)

Cheers,
Michael

michaelwoerister added some commits Aug 8, 2013

debuginfo: Generate template type parameters for generic functions.
Conflicts:
	src/librustc/lib/llvm.rs
	src/librustc/middle/trans/debuginfo.rs
	src/rustllvm/RustWrapper.cpp
	src/rustllvm/rustllvm.def.in
debuginfo: Added test cases for generic structs and enums.
Also, always set no_monomorphic_collapse flags if debuginfo is generated.
debuginfo: Added test cases for methods on structs, enums, traits, an…
…d tuple-structs.

Also new test cases for tuple structs and by-value parameter passing.
debuginfo: Test cases for [generic][default][static] methods and func…
…tions:

* closure-in-generic-function
* generic-functions-nested
* generic-method-on-generic-struct
* generic-trait-generic-static-default-method
* method-on-generic-struct
* self-in-generic-default-method
* trait-generic-static-default-method

Also, fixed an 'unused variable' warning in debuginfo.rs
@brson

This comment has been minimized.

Contributor

brson commented Aug 17, 2013

wowie ❗️

@brson

This comment has been minimized.

brson commented on 80fb2f2 Aug 17, 2013

r+ p=1

@brson

This comment has been minimized.

Contributor

brson commented Aug 17, 2013

Well this is just awesome. I love that it's mostly self-contained in debuginfo.rs and comes with a felt-ton of test cases.

@bors

This comment has been minimized.

Contributor

bors commented on 80fb2f2 Aug 17, 2013

saw approval from brson
at michaelwoerister@80fb2f2

This comment has been minimized.

Contributor

bors replied Aug 17, 2013

merging michaelwoerister/rust/generics = 80fb2f2 into auto

This comment has been minimized.

Contributor

bors replied Aug 17, 2013

michaelwoerister/rust/generics = 80fb2f2 merged ok, testing candidate = 6791021

This comment has been minimized.

This comment has been minimized.

Contributor

bors replied Aug 18, 2013

fast-forwarding master to auto = 6791021

bors added a commit that referenced this pull request Aug 17, 2013

auto merge of #8554 : michaelwoerister/rust/generics, r=brson
This pull request includes support for generic functions and self arguments in methods, and combinations thereof. This also encompasses any kind of trait methods, regular and static, with and without default implementation. The implementation is backed up by a felt ton of test cases `:)`

This is a very important step towards being able to compile larger programs with debug info, since practically any generic function caused an ICE before.

One point worth discussing is that activating debug info now automatically (and silently) sets the `no_monomorphic_collapse` flag. Otherwise debug info would show wrong type names in all but one instance of the monomorphized function.

Another thing to note is that the handling of generic types does not strictly follow the DWARF specification. That is, variables with type `T` (where `T=int`) are described as having type `int` and not as having type `T`. In other words, we are losing information whether a variable has been declared with a type parameter as its type. In practice this should not make much of difference though since the concrete type is mostly what one is interested in. I'll post an issue later so this won't be forgotten.

Also included are a number of bug fixes:
* Closes #1758
* Closes #8513
* Closes #8443
* Fixes handling of field names in tuple structs
* Fixes and re-enables test case for option-like enums that relied on undefined behavior before
* Closes #1339 (should have been closed a while ago)

Cheers,
Michael
@graydon

This comment has been minimized.

Contributor

graydon commented Aug 17, 2013

Superb! Amazing! 🍰 😹 👍

@bors bors closed this Aug 18, 2013

@jdm

This comment has been minimized.

jdm commented on src/librustc/middle/trans/debuginfo.rs in 907633b Aug 18, 2013

This could just be a map call.

@jdm

This comment has been minimized.

jdm commented on src/librustc/middle/trans/debuginfo.rs in 907633b Aug 18, 2013

Extraneous parens.

This comment has been minimized.

Owner

michaelwoerister replied Aug 18, 2013

This piece of code is gone in the final PR version.

@jdm

This comment has been minimized.

jdm commented on src/librustc/middle/trans/debuginfo.rs in 907633b Aug 18, 2013

Is it possible to get the proper span for this type's line and column positions?

This comment has been minimized.

Owner

michaelwoerister replied Aug 18, 2013

Not at the moment. The AST does not store span information for type parameters.

@jdm

This comment has been minimized.

jdm commented on src/librustc/middle/trans/debuginfo.rs in 689929c Aug 18, 2013

This does not appear to be used.

This comment has been minimized.

Owner

michaelwoerister replied Aug 18, 2013

This was an artifact of trying to fix issue rust-lang#8512. The unused parameter is already gone in the PR version of this code.

@jdm

This comment has been minimized.

jdm commented on src/librustc/middle/trans/debuginfo.rs in 689929c Aug 18, 2013

It would be nice to get a proper line and column number for this if possible.

This comment has been minimized.

Owner

michaelwoerister replied Aug 18, 2013

Same as with type parameters: The span of the self argument is not recorded in the AST. Extending the parser to do so should not be too hard though. I'll look into it.

@michaelwoerister

This comment has been minimized.

Contributor

michaelwoerister commented Aug 18, 2013

@brson and @graydon: Thanks for cake, cat and exclamation marks :) I really appreciate that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment