-
Notifications
You must be signed in to change notification settings - Fork 854
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 correct line number to OpDebugFunction and OpDebugScope for function #3357
Add correct line number to OpDebugFunction and OpDebugScope for function #3357
Conversation
chaocNV
commented
Oct 12, 2023
- Pull OpDebugFunction, OpDebugScope and OpDebugVariable for params out of makeFunctionEntry.
- Put above in a separate function called setupDebugFunctionEntry, which also accept line number and set it correctly in builder.
- Call setupDebugFunctionEntry in makeFunction. Also special case handle entry function since it's created ealier elsewhere.
@arcady-lunarg @ncesario-lunarg Could you help review and if looks good, merge it? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thinks looks good on the surface, but it would be good to get @jeremy-lunarg's input as I believe he was the original implementer of this (I think?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of spelling corrections but LGTM. Thanks!
95cdcf5
to
c5d52f8
Compare
All review comment resolved. Could you help merge if it looks good to you? |
@chaocNV if you are able to run the tests, can you please take a look at this failing test: https://github.com/KhronosGroup/glslang/actions/runs/6623976670/job/17992038094?pr=3357#step:11:3400 It looks like the test result file just needs to be updated. |
c5d52f8
to
7234b36
Compare
…ion: 1. Pull OpDebugFunction, OpDebugScope and OpDebugVariable for params out of makeFunctionEntry. 2. Put above in a separate function called setupDebugFunctionEntry, which also accept line number and set it correctly in builder. 3. Call setupDebugFunctionEntry in makeFunction. Also special case handle entry function since it's created ealier elsewhere.
7234b36
to
979423d
Compare
All fixed. Thanks for pointing out |