Skip to content

Debuginfo revamp #5148

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

Closed
wants to merge 4 commits into from
Closed

Debuginfo revamp #5148

wants to merge 4 commits into from

Conversation

bleibig
Copy link
Contributor

@bleibig bleibig commented Feb 27, 2013

This is a rewrite of create_ty in debuginfo along with some other functions to modernize it with the current state of the language. The major change is that it no longer takes in an ast::Ty and instead only works with a ty::t, this is because ty::t seems to offer a more complete representation of the rust type system, and most types (like pointers and vecs) do not really need the span info that an ast::Ty provides.

What works right now are basic types, tuples and structs. Box, owning pointers, and vecs are not supported even though the code is there, apparently the implementation of these types changed since the debuginfo code was last updated. Also not yet implemented are strings, enums, traits, barefns, and closures, and will cause an ICE if you try to generate debuginfo for them.

This makes progress towards #2012 (or completes it, depending on the criteria).

@nikomatsakis
Copy link
Contributor

nice to see progress in this area!

@jdm
Copy link
Contributor

jdm commented Feb 27, 2013

\o/

@bleibig
Copy link
Contributor Author

bleibig commented Feb 28, 2013

The test failure was because different versions of gdb print structs differently, i.e. the old version (6.3) that ships on OS X prints a tuple like

$1 = {
  4,
  5.5,
  true
}

whereas a more recent version prints on one line like $1 = {4, 5.5, true}. The output checker (which is a rudimentary version of FileCheck) sees those two forms as different. I don't see an easy way to immediately fix this, so I'll just disable these checks for now.

This is due to the way different versions of gdb print out structs: older versions have them always spread out with fields on different lines, while newer versions will compactly print them on one line. This makes it hard for the output checker to verify the expected output.
@jdm
Copy link
Contributor

jdm commented Feb 28, 2013

That may be due to differences in gdb pretty-printing defaults. Does |set print pretty false| make a difference in the old gdb's output?

@bleibig
Copy link
Contributor Author

bleibig commented Feb 28, 2013

The command is set print pretty off, but yes, that works on the old gdb and will print structs in the compact form.

@bleibig
Copy link
Contributor Author

bleibig commented Mar 5, 2013

The test failure is due to gdb requiring administrator authentication on the mac build server. If that can somehow be fixed I bet the tests will finally pass and these changes can be landed.

@brson
Copy link
Contributor

brson commented Mar 7, 2013

We can probably arrange for the bots to run with the correct privileges to run gdb, but developers will still end up running into this issue when running the test suite.

Is it ok to just not run debuginfo tests on OS X?

@bleibig
Copy link
Contributor Author

bleibig commented Mar 7, 2013

I guess that will work, but maybe you can re-enable it with a configure flag. The test suite wiki page should also be updated with details as to why the tests are disabled by default and how to enable them. I found that the command DevToolsSecurity -enable puts you into developer mode and eliminates the authentication prompt when attaching gdb to another process.

@brson brson mentioned this pull request Mar 11, 2013
@brson
Copy link
Contributor

brson commented Mar 11, 2013

Continued at #5319

@brson brson closed this Mar 11, 2013
bors added a commit to rust-lang-ci/rust that referenced this pull request May 2, 2020
…nv-unwrap, r=flip1995

Add `option-env-unwrap` lint

changelog: Add `option-env-unwrap` lint

Fixes rust-lang#5147
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.

5 participants