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

Rename the builtin FloatType to LegacyFloatType, Error to ErrorInst #4555

Merged
merged 2 commits into from
Nov 19, 2024

Conversation

jonmeow
Copy link
Contributor

@jonmeow jonmeow commented Nov 19, 2024

This is for more clearly distinct names, and to make it a clearer transition from BuiltinInst for name conflicts. FloatType is also an instruction, and we have Carbon::Error (common/error.h). This avoids affecting tests, although the name is embedded in the builtin test.

In LegacyFloatType, Legacy because I was having trouble coming up with a more appropriate name. I'm not clear this is a FloatLiteralType at present, it needs some work to mirror IntLiteralType.

In ErrorInst, the suffix Inst was discussed as good and similar to BuiltinInst (although I'm trying to get rid of that).

@github-actions github-actions bot requested a review from zygoloid November 19, 2024 18:21
@jonmeow jonmeow force-pushed the legacy-float-type branch 2 times, most recently from 35b209c to 58ec234 Compare November 19, 2024 18:29
@jonmeow jonmeow changed the title Rename the builtin FloatType to LegacyFloatType Rename the builtin FloatType to LegacyFloatType, Error to ErrorInst Nov 19, 2024
@zygoloid zygoloid added this pull request to the merge queue Nov 19, 2024
Merged via the queue into carbon-language:trunk with commit 4a80d67 Nov 19, 2024
8 checks passed
@jonmeow jonmeow deleted the legacy-float-type branch November 19, 2024 21:18
bricknerb pushed a commit to bricknerb/carbon-lang that referenced this pull request Nov 28, 2024
…arbon-language#4555)

This is for more clearly distinct names, and to make it a clearer
transition from `BuiltinInst` for name conflicts. `FloatType` is also an
instruction, and we have `Carbon::Error` (common/error.h). This avoids
affecting tests, although the name is embedded in the builtin test.

In `LegacyFloatType`, `Legacy` because I was having trouble coming up
with a more appropriate name. I'm not clear this is a `FloatLiteralType`
at present, it needs some work to mirror `IntLiteralType`.

In `ErrorInst`, the suffix `Inst` was discussed as good and similar to
`BuiltinInst` (although I'm trying to get rid of that).
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