Skip to content

Conversation

AlexsanderDamaceno
Copy link

This change aims to improve, as stated in issue #83877, the way integer types are displayed to users when detecting overflows. Instead of showing, for example, Builtin.Int, it will show the integer type name as declared by the user for example Uint8.

… way integer types are displayed to users when detecting overflows.

Instead of showing, for example, Builtin.Int, it will show the integer type name as declared by the user.
@AlexsanderDamaceno
Copy link
Author

AlexsanderDamaceno commented Oct 6, 2025

@rintaro @xedin @AnthonyLatsis hey, could you do review on this?

Copy link
Member

@rintaro rintaro left a comment

Choose a reason for hiding this comment

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

Thank you for working on this!
I'm not really a person to review this area, but I think this is a matter of how UserDstTy is derived from Loc, rather than how to display the built-in type. For example, in the case of #83877, with your change, the diagnostic would display UInt64, but it should be UInt.
I would look into how to derive UserDstTy to UInt using Loc.getAsASTNode<>()

Also, please add test cases 🙇‍♂️

case 8: return Ctx.getInt8Type();
case 16: return Ctx.getInt16Type();
case 32: return Ctx.getInt32Type();
case 64: return Ctx.getInt64Type();
Copy link
Member

Choose a reason for hiding this comment

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

You'd need default:. Also there is (U)Int128.

// Since builtin types are sign-agnostic, print the signedness
// separately.
// separately

Copy link
Member

Choose a reason for hiding this comment

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

Unrelated change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants