Skip to content
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

Add: Added the instructions for debugging from rust-forge #113

Merged
merged 6 commits into from
Apr 15, 2018
Merged

Add: Added the instructions for debugging from rust-forge #113

merged 6 commits into from
Apr 15, 2018

Conversation

cg-cnu
Copy link
Contributor

@cg-cnu cg-cnu commented Apr 9, 2018

  • Added the instructions for debugging in debugging.md
  • Added a link to SUMMARY.md at the end of the file

Let me know if you got any comments. Thanks 🙂

@cg-cnu
Copy link
Contributor Author

cg-cnu commented Apr 9, 2018

Working on the issue #11

Copy link
Member

@mark-i-m mark-i-m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM :)

Thanks!

Just a few nits, and when travis is happy, r=me

3: std::panicking::default_hook
4: std::panicking::rust_panic_with_hook
5: std::panicking::begin_panic
6: rustc_typeck::check::cast::<impl rustc_typeck::check::FnCtxt<'a, 'gcx, \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this line is too long, then we could just remove it altogether...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to reduce the line length, so kept it in a separate line. Now removed it.

In the default configuration, you don't have line numbers enabled, so the
backtrace looks like this:

```
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be ```text

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

made it text

src/SUMMARY.md Outdated
@@ -52,6 +52,7 @@
- [miri const evaluator](./miri.md)
- [Parameter Environments](./param_env.md)
- [Generating LLVM IR](./trans.md)
- [Debugging the Compiler](./compiler-debugging.md)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be moved after the testing chapter (ch 4)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!


When you have an ICE (panic in the compiler), you can set
`RUST_BACKTRACE=1` to get the stack trace of the `panic!` like in
normal Rust programs. IIRC backtraces **don't work** on Mac and on MinGW,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nikomatsakis Is this still true?

```
stack backtrace:
(~~~~ LINES REMOVED BY ME FOR BREVITY ~~~~)
6: rustc_typeck::check::cast::<impl rustc_typeck::check::FnCtxt<'a, 'gcx, \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed the line and made it ```text

and look at the log output with a text editor.

So to put it together.
```
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be ```text

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, couple of shell commands and comments, so made it ```bash

### Logging etiquette

Because calls to `debug!` are removed by default, in most cases, don't worry
about adding "unnecessary" calls to `debug!` and leaving them in in code
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: "in in"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

These all produce `.dot` files. To view these files, install graphviz (e.g.
`apt-get install graphviz`) and then run the following commands:

```
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think ```bash?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep! Made it bash!

If you can get away with it (i.e. if it doesn't make your bug disappear),
passing `-C codegen-units=1` to rustc will make debugging easier.

If you want to play with the optimization pipeline, you can use the `opt` from
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps rephrase this sentence as "If you want to play with the optimization pipeline, you can use the opt tool from ./build/<host-triple>/llvm/bin/ with the the LLVM IR emitted by rustc."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed the first sentence and ended up with this paragraph!

If you want to play with the optimization pipeline, you can use the opt tool
from ./build/<host-triple>/llvm/bin/ with the the LLVM IR emitted by rustc.
Note that rustc emits different IR depending on whether -O is enabled, even
without LLVM's optimizations, so if you want to play with the IR rustc emits,
you should:

let me know if it's fine!

that rustc emits different IR depending on whether `-O` is enabled, even without
LLVM's optimizations, so if you want to play with the IR rustc emits,
you should:
```
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

```bash

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done!

@mark-i-m mark-i-m added the S-waiting-on-author Status: this PR is waiting for additional action by the OP label Apr 10, 2018
@cg-cnu
Copy link
Contributor Author

cg-cnu commented Apr 10, 2018

Hey @mark-i-m Fixed all the suggested changes and travis build is passing now! Waiting on one comment from @nikomatsakis Let me know if you have any other suggestions. Thanks! 🙂

Copy link
Member

@mark-i-m mark-i-m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :)

@mark-i-m
Copy link
Member

@cg-cnu Thanks :)

Let's wait a little while, and if Niko can't get around to it, we can add a note like "Note: this is copied from the forge. If anything needs updating, please open an issue or make a PR on the github repo." and merge...

@cg-cnu
Copy link
Contributor Author

cg-cnu commented Apr 10, 2018

Thanks @mark-i-m ! I just did a PR on rust-forge to remove debugging.md rust-lang/rust-forge#134

@mark-i-m mark-i-m added S-waiting-on-review Status: this PR is waiting for a reviewer to verify its content and removed S-waiting-on-review Status: this PR is waiting for a reviewer to verify its content S-waiting-on-author Status: this PR is waiting for additional action by the OP labels Apr 13, 2018
@mark-i-m
Copy link
Member

Ok, so Niko is going on (well-earned) vacation. Could add a note at the top of the document (maybe bolded or something)? Then I think we can just go ahead and merge.

@cg-cnu
Copy link
Contributor Author

cg-cnu commented Apr 15, 2018

Hey @mark-i-m Added note at the top of the page in bold! 🙂

@mark-i-m
Copy link
Member

Thanks!

@mark-i-m mark-i-m merged commit eb396e7 into rust-lang:master Apr 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: this PR is waiting for a reviewer to verify its content
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants