Closed dwblaikie closed 1 month ago
Are you using pre-commit (
pre-commit install
)? I think it would've automatically fixed this by runningclang-format
for you.
Ah, nope - missed that, sorry. Thanks for the pointer/reminder. Think I managed to run it on some of my stuff that's already committed (with pre-commit run --all-files
) to the branch - it caught one formatting error, but otherwise seems clean
Mostly this'll be a place to discuss the flag naming/functionality, and testing strategy. Should we have debug info on in all lowering tests? Should there be some separate directory just for debug info lowering test coverage? I'm open to ideas/suggestions/requests.
Not a comment on the patch, but maybe update the description before merging to summarize what your suggested starting point is for these questions, and what motivates that starting point?
(FWIW, not disagreeing with the current behavior, mostly looking to have a record of it.)
Mostly this'll be a place to discuss the flag naming/functionality, and testing strategy. Should we have debug info on in all lowering tests? Should there be some separate directory just for debug info lowering test coverage? I'm open to ideas/suggestions/requests.
Not a comment on the patch, but maybe update the description before merging to summarize what your suggested starting point is for these questions, and what motivates that starting point?
(FWIW, not disagreeing with the current behavior, mostly looking to have a record of it.)
Updated - hopefully that's something along the lines of what you're looking for.
This adds just the debug info metadata for Compilation Units (the top level container of debug info) - but without anything in them, LLVM won't emit them at all, so while this is testable at the IR level, it isn't observable at the object level until more debug info is added.
A couple of starting points in this patch:
--debug-info
, seems to match the naming/style of other flags in the carbon driver, though this is different from the naming conventions of clang/gcc) that enables debug info when lowering. Open to other names/approaches (on by default? historically debug info's been to large/expensive to do this, so sticking with that precedent for now).