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

core: surface original error when raising verification error #1192

Merged
merged 1 commit into from Jun 26, 2023

Conversation

superlopuh
Copy link
Member

No description provided.

@superlopuh superlopuh added the core xDSL core (ir, textual format, ...) label Jun 26, 2023
@superlopuh superlopuh self-assigned this Jun 26, 2023
@codecov
Copy link

codecov bot commented Jun 26, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.10 🎉

Comparison is base (740c1dd) 88.65% compared to head (09f1aaf) 88.75%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1192      +/-   ##
==========================================
+ Coverage   88.65%   88.75%   +0.10%     
==========================================
  Files         160      161       +1     
  Lines       22464    22455       -9     
  Branches     3393     3409      +16     
==========================================
+ Hits        19916    19931      +15     
+ Misses       2004     1981      -23     
+ Partials      544      543       -1     
Impacted Files Coverage Δ
xdsl/ir/core.py 88.37% <100.00%> (ø)
xdsl/utils/diagnostic.py 84.00% <100.00%> (ø)

... and 7 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Collaborator

@webmiche webmiche left a comment

Choose a reason for hiding this comment

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

Can you add a test showing what this does exactly? Looks like a good idea o/w 👍

@superlopuh
Copy link
Member Author

I had a look into writing the test, and it's a bit hard to do. The main difference is in the python output, you get two stack traces, one leading to the underlying error, and one from the newly raised verification error.

@webmiche
Copy link
Collaborator

Yes, that's what I expected. I guess one way of doing that would be spawning a subprocess and getting its output. Or maybe something with ast.eval? 🤔

Not sure we want either of those in the codebase though. Do you have something nice/small that I can jsut run locally to see the change?

Copy link
Collaborator

@math-fehr math-fehr left a comment

Choose a reason for hiding this comment

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

Nice!

@superlopuh
Copy link
Member Author

>>> b = TestOp(operands=((),), result_types=((),), regions=((Return(),),))
>>> b.verify()
Traceback (most recent call last):
  File "/Users/sasha/Developer/xdslproject/xdsl/xdsl/ir/core.py", line 816, in verify
    self.verify_()
  File "/Users/sasha/Developer/xdslproject/xdsl/xdsl/irdl.py", line 1784, in verify_
    op_def.verify(self)
  File "/Users/sasha/Developer/xdslproject/xdsl/xdsl/irdl.py", line 1179, in verify
    trait.verify(op)
  File "/Users/sasha/Developer/xdslproject/xdsl/xdsl/traits.py", line 59, in verify
    raise VerifyException(
xdsl.utils.exceptions.VerifyException: 'func.return' expects parent op 'func.func'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/sasha/Developer/xdslproject/xdsl/xdsl/ir/core.py", line 812, in verify
    region.verify()
  File "/Users/sasha/Developer/xdslproject/xdsl/xdsl/ir/core.py", line 1690, in verify
    block.verify()
  File "/Users/sasha/Developer/xdslproject/xdsl/xdsl/ir/core.py", line 1392, in verify
    operation.verify()
  File "/Users/sasha/Developer/xdslproject/xdsl/xdsl/ir/core.py", line 818, in verify
    self.emit_error(
  File "/Users/sasha/Developer/xdslproject/xdsl/xdsl/ir/core.py", line 989, in emit_error
    diagnostic.raise_exception(message, self, exception_type, underlying_error)
  File "/Users/sasha/Developer/xdslproject/xdsl/xdsl/utils/diagnostic.py", line 42, in raise_exception
    raise exception_type(message + "\n\n" + f.getvalue()) from underlying_error
xdsl.utils.exceptions.VerifyException: Operation does not verify: 'func.return' expects parent op 'func.func'

"test.op"() ({
  "func.return"() : () -> ()
  ^^^^^^^^^^^^^-----------------------------------------------------------
  | Operation does not verify: 'func.return' expects parent op 'func.func'
  ------------------------------------------------------------------------
}) : () -> ()


>>> 

@superlopuh
Copy link
Member Author

As of right now, main only returns the second error and call stack, which is less useful

Copy link
Collaborator

@webmiche webmiche left a comment

Choose a reason for hiding this comment

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

Okay, thanks 👍

@superlopuh superlopuh merged commit aefa483 into main Jun 26, 2023
12 checks passed
@superlopuh superlopuh deleted the sasha/misc/underlying_error branch June 26, 2023 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core xDSL core (ir, textual format, ...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants