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

[stdlib] Refactor format_int.mojo #3348

Open
wants to merge 1 commit into
base: nightly
Choose a base branch
from

Conversation

helehex
Copy link
Contributor

@helehex helehex commented Aug 1, 2024

Refactor format_int.mojo to use Formatter, and take care of corresponding TODO's.

@helehex helehex requested a review from a team as a code owner August 1, 2024 15:57
@helehex
Copy link
Contributor Author

helehex commented Aug 1, 2024

Oh I forgot about something.
I thought about removing the _format_int() functions in favor of the _write_int() ones, but decided it was best to keep both.

…ponding TODO's.

Signed-off-by: Max Brylski <helehex@gmail.com>
@helehex
Copy link
Contributor Author

helehex commented Aug 1, 2024

Ok, I added those tests back in.

@@ -591,9 +591,7 @@ struct _ObjectImpl(
)
return
if self.is_func():
writer.write(
"Function at address " + hex(int(self.get_as_func().value))
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Question Is there a way we might be able to keep the hex(..) formatting here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Followup Never mind, I see you've changed the formatting of UnsafePointer to use hex by default as well.

That seems reasonable to me, though it would be great to call that out in the description as well :)

@ConnorGray
Copy link
Collaborator

!sync

@modularbot modularbot added the imported-internally Signals that a given pull request has been imported internally. label Aug 5, 2024
@JoeLoser
Copy link
Collaborator

FYI this is still hitting some compiler bugs internally in the elaborator, so added the "blocked" label on this PR for now.

@JoeLoser JoeLoser added the blocked Blocked by another issue that must be resolved first label Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Blocked by another issue that must be resolved first imported-internally Signals that a given pull request has been imported internally.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants