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

Fix unchecked optional access in compile subcommand #4756

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

ottmar-zittlau
Copy link

Hi,

I fixed a small issue that I found inside the "compile subcommand" component:
The program can be crashed by running bazel run -- toolchain:carbon compile --dump-mem-usage "non-existing-file.carbon" - i.e. by activating the memory usage dump flag and passing a non-existing file.

Best regards,
oz

@github-actions github-actions bot requested a review from jonmeow January 5, 2025 12:21
Copy link
Contributor

@chandlerc chandlerc left a comment

Choose a reason for hiding this comment

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

Fix looks great -- would you be up for adding a test case?

This seems like a thing effectively tested with unittests here: https://github.com/carbon-language/carbon-lang/blob/trunk/toolchain/driver/driver_test.cpp

@ottmar-zittlau ottmar-zittlau force-pushed the fix/Unchecked-optional-access-in-compile-subcommand branch from be5c066 to 331f3eb Compare January 5, 2025 14:45
@ottmar-zittlau
Copy link
Author

Yes - I added a test for this specific flag only.
I thought about repeating the test also for the other flags, but didn't want to stretch this patch unnecessarily.
Let me know, if you think this would make sense.

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