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

Fill correct file name to OpDebugFunction and add line info for parameters' OpDebugDeclare #3513

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

qingyuanzNV
Copy link
Contributor

This patch fixes:

  1. OpDebugFunction's file entry should be the correct file instead of always main file
  2. OpDebugDeclare and OpDebugValue should be now attributed with an OpDebugLine pointing to the function definition line.

Copy link
Contributor

@arcady-lunarg arcady-lunarg left a comment

Choose a reason for hiding this comment

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

Could you add a test with debuginfo and include files? I think this will help demonstrate your change's functionality.

@qingyuanzNV
Copy link
Contributor Author

It turns out it's a non-trivial effort to add a test for this. I have to change the test infrastructure to pass in a customized includer and optionally pass in the shader name for non-semantic debug.

Btw, I noticed that the source text for main file is wrong for the test result. I didn't fix it because it looks like an old issue, not a regression by this PR.

@qingyuanzNV qingyuanzNV force-pushed the correct_debug_function_lineinfo branch from 46d9198 to 692c0a2 Compare February 26, 2024 05:45
@qingyuanzNV qingyuanzNV marked this pull request as draft February 27, 2024 06:13
@qingyuanzNV
Copy link
Contributor Author

Change to draft. The test infra part is increasingly complicated, and I think it's worth a dedicated PR. I'd like to extract the test infra part and commit that first.

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

2 participants