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

Match panics in compilation-mode #510

Merged
merged 1 commit into from
Feb 17, 2024
Merged

Conversation

luckysori
Copy link
Contributor

@luckysori luckysori commented Feb 11, 2024

Fixes #507.

If we encounter a panic when executing Rust code in compilation-mode, we now correctly highlight the location where the panic occurred.

For example:

thread 'main' panicked at src/main.rs:2:5:
explicit panic
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Here, src/main.rs:2:5 is highlighted. The developer is then able to execute compile-goto-error to jump to the correct spot in the code.

rust-compile.el Outdated
@@ -34,6 +34,12 @@ See `compilation-error-regexp-alist' for help on their format.")
"Specifications for matching code references in rustc invocations.
See `compilation-error-regexp-alist' for help on their format.")

(defvar rustc-panics-compilation-regexps
(let ((re "thread '[^']+' panicked at \\(\\(.*\\):\\([0-9]+\\):\\([0-9]+\\)\\)"))
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you should use the rustc-compilation-location variable already in scope.
Something like this

(let ((re (concat "thread '[^']+' panicked at " rustc-compilation-location)))
    (cons re '(2 3 4 nil 1)))
    ```

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the tip! I'll give it a go later.

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, you were right!

If we encounter a panic when executing Rust code in
`compilation-mode`, we now correctly highlight the location where the
panic occurred.

For example:

```
thread 'main' panicked at src/main.rs:2:5:
explicit panic
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```

Here, `src/main.rs:2:5` is highlighted. The developer is then able to
execute `compile-goto-error` to jump to the correct spot in the code.

Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
Co-authored-by: zlef <zachlefevre@gmail.com>
@@ -3618,11 +3619,13 @@ let b = 1;"
("file6.rs" "132" "34" compilation-error "file6.rs:132:34"))
(("file5.rs" "12" "34" compilation-info "file5.rs:12:34"))
((like-previous-one "82" back-to-indentation compilation-info "82")
(like-previous-one "132" back-to-indentation compilation-info "132")))
(like-previous-one "132" back-to-indentation compilation-info "132"))
(("src/file7.rs" "12" "34" nil "src/file7.rs:12:34")))
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've added a test now, but I'm not sure why this is nil and not compilation-error. Maybe someone more comfortable with Elisp can assist me.

Copy link
Contributor

@zachlefevre zachlefevre Feb 14, 2024

Choose a reason for hiding this comment

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

I am not very familiar with elisp either, so I could be wrong.
Here's what I think is happening here:
compilation-error is set

(defvar compilation-error "error"
  "Stem of message to print when no matches are found.")

And so an expression like ("file6.rs" "132" "34" compilation-error "file6.rs:132:34") is equivalent to ("file6.rs" "132" "34" error "file6.rs:132:34")'

That translates to type expression from the compilation-error-regexp-alist help doc. I'm getting that from this paragraph:

Each elt has the form (REGEXP FILE [LINE COLUMN TYPE HYPERLINK
HIGHLIGHT...]).  If REGEXP matches, the FILE’th subexpression
gives the file name, and the LINE’th subexpression gives the line
number.  The COLUMN’th subexpression gives the column number on
that line.

The definition of our regex is

defvar rustc-panics-compilation-regexps
   (let ((re (concat "thread '[^']+' panicked at " rustc-compilation-location)))
     (cons re '(2 3 4 nil 1)))
   "Specifications for matching panics in rustc invocations.
See `compilation-error-regexp-alist' for help on their format.")

And so we're capturing the:
2nd capture group for the file name
3rd capture group for the line number,
4th capture group for the column number,
nil for the type
1st capture group for the hyperlink.

We don't have an error or warning in our error message, like the others do, to parse out for the type, so our type is nil.

Copy link
Contributor

Choose a reason for hiding this comment

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

so I think nil is correct. That, or invent a new type like panic. I'm not really sure.

@zachlefevre
Copy link
Contributor

@luckysori This looks ready for review to me. Do you mind running @rustbot review? from https://rustc-dev-guide.rust-lang.org/contributing.html#waiting-for-reviews ?

@luckysori
Copy link
Contributor Author

@rustbot review

@rustbot
Copy link

rustbot commented Feb 16, 2024

Error: The feature shortcut is not enabled in this repository.
To enable it add its section in the triagebot.toml in the root of the repository.

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

@luckysori
Copy link
Contributor Author

Error: The feature shortcut is not enabled in this repository. To enable it add its section in the triagebot.toml in the root of the repository.

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

I guess it's not enabled on this repo, @zachlefevre.

@zachlefevre
Copy link
Contributor

@psibi Do you mind reviewing this?

@psibi
Copy link
Member

psibi commented Feb 17, 2024

Thanks @zachlefevre for the review and @luckysori for the PR and the test case! I have confirmed that it works for me, merging this.

@psibi psibi merged commit 8bbe70b into rust-lang:master Feb 17, 2024
14 of 15 checks passed
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.

compilation-error-regexp not matching panics
4 participants