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

Handle unknown lines in DebugInfo #4252

Merged
merged 5 commits into from
Sep 4, 2024
Merged

Conversation

jonmeow
Copy link
Contributor

@jonmeow jonmeow commented Aug 26, 2024

Mainly, changes the default from -1 to 0 in DiagnosticLoc, still trying to keep reusing that. Nothing except for the lowered output is affected, so I think this is fine.

Also, have lowering consistently call GetDiagnosticLoc.

Pulls in one of the CHECKs suggested from #4251

Co-authored-by: David Blaikie dblaikie@gmail.com

@jonmeow
Copy link
Contributor Author

jonmeow commented Aug 26, 2024

FYI @dwblaikie

Copy link
Contributor

@dwblaikie dwblaikie left a comment

Choose a reason for hiding this comment

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

(not sure what the conventions are for approval - since the author has commit rights, whether you just wanted my domain-specific review, or if I should leave approval to someone with commit rights/some other authority?)

Change looks OK to me (one minor side comment/thought).

The output changes look OK/consistent with how Clang generates debug info source locations for global constructors. (the global ctor function itself has no line, but the instructions inside it do point to the relevant parts of the initializing subexpressions)

toolchain/diagnostics/diagnostic.h Outdated Show resolved Hide resolved
Copy link
Contributor

@dwblaikie dwblaikie left a comment

Choose a reason for hiding this comment

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

(thought I picked the approve button, but guess got sidetracked by my own rambling considerations in the previous message)

ooh, I did pick approve, but it doesn't highlight green because of my permissions - then I didn't notice the subtle grey checkmark. Don't mind me.

Copy link
Contributor

@dwblaikie dwblaikie left a comment

Choose a reason for hiding this comment

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

Looks good to me - thanks!

@jonmeow jonmeow requested review from zygoloid and removed request for josh11b August 29, 2024 17:57
@jonmeow jonmeow added this pull request to the merge queue Sep 4, 2024
Merged via the queue into carbon-language:trunk with commit 1412ecd Sep 4, 2024
8 checks passed
@jonmeow jonmeow deleted the debug-line branch September 4, 2024 21:36
dwblaikie added a commit to dwblaikie/carbon-lang that referenced this pull request Sep 6, 2024
Mainly, changes the default from -1 to 0 in DiagnosticLoc, still trying
to keep reusing that. Nothing except for the lowered output is affected,
so I think this is fine.

Also, have lowering consistently call GetDiagnosticLoc.

Pulls in one of the CHECKs suggested from carbon-language#4251

Co-authored-by: David Blaikie <dblaikie@gmail.com>
dwblaikie added a commit to dwblaikie/carbon-lang that referenced this pull request Sep 9, 2024
Mainly, changes the default from -1 to 0 in DiagnosticLoc, still trying
to keep reusing that. Nothing except for the lowered output is affected,
so I think this is fine.

Also, have lowering consistently call GetDiagnosticLoc.

Pulls in one of the CHECKs suggested from carbon-language#4251 

Co-authored-by: David Blaikie <dblaikie@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants