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

RPYTHON: embed rpython code line as a comment in the generated C #4837

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mattip
Copy link
Member

@mattip mattip commented Jan 7, 2024

This started as a patch to #1220. I am trying to update it.

code = self.pycode
linenum = self.last_offset
src = self.graph.source.split('\n')[linenum]
text = '%s:%d : %s' % (code.co_name, linenum, src)
Copy link
Member Author

Choose a reason for hiding this comment

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

This part fails. self.pycode is the compiled bytecode, and the last_offset relates to a bytecode offset, not a code linenum. Do we have any information about the line number in the source code anywhere?

Copy link
Member

Choose a reason for hiding this comment

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

yes, you can call offset2lineno(code, self.last_offset) to get the line number. the function is in rpython.tool.error.

Comment on lines +401 to +404
except Exception as e:
print(e)
import pdb;pdb.set_trace()
raise
Copy link
Member Author

Choose a reason for hiding this comment

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

Remove this when the comment creation works

Suggested change
except Exception as e:
print(e)
import pdb;pdb.set_trace()
raise

@mattip mattip marked this pull request as draft January 7, 2024 10:24
@cfbolz
Copy link
Member

cfbolz commented Jan 7, 2024

cool! pitfalls that I can think off:

  • don't know what the rtyper does with the comments?
  • you should make sure that the comments get a weight of 0 in inlining decisions, by adding them to translator.backendopt.inline.OP_WEIGHTS.

you should probably add some unit tests to:

  • the flowspace to check that the right comments are generated
  • the rtyper to check that it doesn't remove the comments
  • the inlining tests to make sure that comments get inlined correctly, and that they don't influence inlining decisions.

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.

None yet

3 participants