Adding support for `i32`, `i8` and `i64` to-`char` conversion #6425

viniciusfdasilva · 2025-11-24T11:40:14Z

This pull request adds support for integer-to-char conversion, allowing the compiler to correctly handle character casting, implementing part of the issue #5922.

import Core library "io";

fn Run() -> i32 {
	var i : i32 = 65;
	var ch: char = (i as char); // Support implemented!
	Core.PrintChar(ch); // Print 'A'
	return 0;
}

danakj · 2025-11-24T15:37:03Z

This pull request adds support for i32-to-char conversion, enabling the compiler to correctly handle character casting from 32-bit integers.
import Core library "io";

fn Run() -> i32 {
	var i : i32 = 65;
	var ch: char = (i as char); // Support implemented!
	Core.PrintChar(ch); // Print 'A'
	return 0;
}

I don't see i32 -> char listed in here as a builtin operation: #5922 (comment)

Could you clarify where in the design or a proposal or leads issue this is from?

danakj · 2025-11-24T15:38:16Z

This pull request adds support for i32-to-char conversion, enabling the compiler to correctly handle character casting from 32-bit integers.
import Core library "io";

fn Run() -> i32 {
	var i : i32 = 65;
	var ch: char = (i as char); // Support implemented!
	Core.PrintChar(ch); // Print 'A'
	return 0;
}
I don't see i32 -> char listed in here as a builtin operation: #5922 (comment)

Could you clarify where in the design or a proposal or leads issue this is from?

Oh I see it:

Explicit conversion to/from integers equivalent to what u8 supports.
Continuing no implicit conversions.

Could you add a link to the PR description?

danakj

Please add tests including both tests that now pass and tests that fail for bad values or types, and tests that pass/fail for making a "convert_char" builtin signature that is correct or malformed.

danakj · 2025-11-24T15:40:20Z

core/prelude/types/char.carbon

  fn Convert[self: Self]() -> Char = "char.convert_checked";
 }
+
+impl Int(32) as As(Char) {


Can we make this generic for all integer sizes, like it is for u8?

carbon-lang/core/prelude/types/uint.carbon

Line 45 in 7be6538

final impl forall [From:! IntLiteral(), To:! IntLiteral()] UInt(From) as As(UInt(To)) {

Hi @danakj

I had issues with this kind of implementation, but I managed to get support working for i32, i8, and i64! However, I’ll work on adapting it to what you suggested in the future.

danakj · 2025-11-24T15:40:45Z

toolchain/check/eval.cpp

-        for (const auto& constraint : info.extend_named_constraints) {
-          ResolveSpecificDeclForSpecificId(eval_context,
-                                           constraint.specific_id);
-        }
-        for (const auto& constraint : info.self_impls_named_constraints) {
-          ResolveSpecificDeclForSpecificId(eval_context,
-                                           constraint.specific_id);
-        }


This looks like a bad rebase?

This might be a branch update problem!
I'll fix that as well!
This code should be included.

I added this snippet again!

toolchain/check/eval.cpp

danakj · 2025-11-24T15:41:35Z

toolchain/check/eval.cpp

+//// Performs a conversion from integer to character type.
+// Performs a conversion between character types, diagnosing if the value
+// doesn't fit in the destination type.


This looks like a copy paste that needs to be cleaned up?

Yes!! I will remove this

danakj · 2025-11-24T15:42:21Z

toolchain/check/eval.cpp

+// Performs a conversion between character types, diagnosing if the value
+// doesn't fit in the destination type.
+static auto PerformCharConvert(Context& context, SemIR::LocId loc_id,
+                                      SemIR::InstId arg_id,


I suggest setting up pre-commit locally so things like formatting are fixed before uploading. https://github.com/carbon-language/carbon-lang/blob/trunk/docs/project/contribution_tools.md

Thank you! I used pre-commit in my new submition :)

toolchain/check/eval.cpp

danakj · 2025-11-24T15:45:01Z

toolchain/check/eval.cpp

+  // Values over 0x80 require multiple code units in UTF-8.
+  if (value >= 0x80) {


This surprised me, could you point me to where the design says this should happen, rather than converting anything up to 255?

Actually, it’s a restriction I initially included, but it may indeed not make sense to keep it.

Yes! I removed this code snippet! I don’t think it makes sense to add this check.

danakj · 2025-11-24T15:45:40Z

toolchain/sem_ir/builtin_function_kind.cpp

 constexpr BuiltinInfo ReadChar = {"read.char",
                                  ValidateSignature<auto()->AnySizedInt>};

+


Suggested change

toolchain/sem_ir/builtin_function_kind.cpp

toolchain/sem_ir/builtin_function_kind.def

CarbonInfraBot · 2025-11-24T15:55:07Z

core/prelude/types/char.carbon

+  fn Convert[self: Self]() -> Char = "int.convert_char";  
+}


[diff] _{reported by reviewdog 🐶}

Suggested change

fn Convert[self: Self]() -> Char = "int.convert_char";

}

fn Convert[self: Self]() -> Char = "int.convert_char";

}

CarbonInfraBot · 2025-11-24T15:55:08Z

toolchain/check/eval.cpp

                                      SemIR::InstId arg_id,
                                      SemIR::TypeId dest_type_id)
    -> SemIR::ConstantId {
+


[diff] _{reported by reviewdog 🐶}

Suggested change

CarbonInfraBot · 2025-11-24T15:55:08Z

toolchain/check/eval.cpp

+                                      SemIR::InstId arg_id,
+                                      SemIR::TypeId dest_type_id)


[diff] _{reported by reviewdog 🐶}

Suggested change

SemIR::InstId arg_id,

SemIR::TypeId dest_type_id)

SemIR::InstId arg_id, SemIR::TypeId dest_type_id)

CarbonInfraBot · 2025-11-24T15:55:08Z

toolchain/check/eval.cpp

+                                      SemIR::InstId arg_id,
+                                      SemIR::TypeId dest_type_id)
+    -> SemIR::ConstantId {
+


[diff] _{reported by reviewdog 🐶}

Suggested change

CarbonInfraBot · 2025-11-24T15:55:08Z

toolchain/check/eval.cpp

+
+  auto [is_signed, bit_width_id] =
+      context.sem_ir().types().GetIntTypeInfo(dest_type_id);
+


[diff] _{reported by reviewdog 🐶}

Suggested change

CarbonInfraBot · 2025-11-24T15:55:08Z

toolchain/check/eval.cpp

+                           {.type = arg.type_id, .value = arg_val},
+                           dest_type_id);
+  }
+


[diff] _{reported by reviewdog 🐶}

Suggested change

CarbonInfraBot · 2025-11-24T15:55:08Z

toolchain/check/eval.cpp

+    CARBON_DIAGNOSTIC(
+        NegativeIntInUnsignedType, Error,
+        "character value {0} too large for type {1}", TypedInt,
+        SemIR::TypeId);


[diff] _{reported by reviewdog 🐶}

Suggested change

CARBON_DIAGNOSTIC(

NegativeIntInUnsignedType, Error,

"character value {0} too large for type {1}", TypedInt,

SemIR::TypeId);

CARBON_DIAGNOSTIC(NegativeIntInUnsignedType, Error,

"character value {0} too large for type {1}", TypedInt,

SemIR::TypeId);

CarbonInfraBot · 2025-11-24T15:55:08Z

toolchain/check/eval.cpp


    // Integer conversions.
+    case SemIR::BuiltinFunctionKind::IntConvertChar: {
+       if (phase != Phase::Concrete) {


[diff] _{reported by reviewdog 🐶}

Suggested change

if (phase != Phase::Concrete) {

if (phase != Phase::Concrete) {

CarbonInfraBot · 2025-11-24T15:55:08Z

toolchain/sem_ir/builtin_function_kind.cpp

 constexpr BuiltinInfo ReadChar = {"read.char",
                                  ValidateSignature<auto()->AnySizedInt>};

+


[diff] _{reported by reviewdog 🐶}

Suggested change

CarbonInfraBot · 2025-11-24T15:55:09Z

toolchain/sem_ir/builtin_function_kind.cpp

+constexpr BuiltinInfo IntConvertChar = {"int.convert_char", 
+                                        ValidateSignature<auto(AnyInt)->CharCompatible>};


[diff] _{reported by reviewdog 🐶}

Suggested change

constexpr BuiltinInfo IntConvertChar = {"int.convert_char",

ValidateSignature<auto(AnyInt)->CharCompatible>};

constexpr BuiltinInfo IntConvertChar = {

"int.convert_char", ValidateSignature<auto(AnyInt)->CharCompatible>};

Co-authored-by: Dana Jansens <danakj@orodu.net>

Adding support for i32-to-char conversion

74abc7d

viniciusfdasilva requested a review from a team as a code owner November 24, 2025 11:40

viniciusfdasilva requested review from danakj and removed request for a team November 24, 2025 11:40

This comment was marked as outdated.

Sign in to view

github-actions bot added the toolchain label Nov 24, 2025

danakj reviewed Nov 24, 2025

View reviewed changes

CarbonInfraBot reviewed Nov 24, 2025

View reviewed changes

viniciusfdasilva and others added 10 commits November 24, 2025 13:07

Update toolchain/check/eval.cpp

c7c60a7

Co-authored-by: Dana Jansens <danakj@orodu.net>

Update toolchain/check/eval.cpp

6efd96c

Co-authored-by: Dana Jansens <danakj@orodu.net>

Implementing support i8, i32, and i64 to char

4423c5c

Applying suggestions from PR carbon-language#6425

088d976

Inserting a section mistakenly removed from eval.cpp

0f722b6

Removing restriction for greater equal than 128 integer

c17f6ad

pre-commit changes applied

7366715

Creating tests

0b49ba9

Formating test file

6da16ea

Adding NegativeIntInUnsignedTypeInCast diagnostic name

edefca7

viniciusfdasilva requested review from CarbonInfraBot and danakj November 25, 2025 19:34

viniciusfdasilva changed the title ~~Adding support for i32-to-char conversion~~ Adding support for i32, i8 and i64 to-char conversion Nov 26, 2025

viniciusfdasilva changed the title ~~Adding support for i32, i8 and i64 to-char conversion~~ Adding support for i32, i8 and i64 to-char conversion Nov 26, 2025

Merge branch 'carbon-language:trunk' into trunk

6cdd115

		// Values over 0x80 require multiple code units in UTF-8.
		if (value >= 0x80) {

		constexpr BuiltinInfo ReadChar = {"read.char",
		ValidateSignature<auto()->AnySizedInt>};

	SemIR::InstId arg_id,
	SemIR::TypeId dest_type_id)
	SemIR::InstId arg_id, SemIR::TypeId dest_type_id)


		auto [is_signed, bit_width_id] =
		context.sem_ir().types().GetIntTypeInfo(dest_type_id);

	if (phase != Phase::Concrete) {
	if (phase != Phase::Concrete) {

		constexpr BuiltinInfo IntConvertChar = {"int.convert_char",
		ValidateSignature<auto(AnyInt)->CharCompatible>};

Adding support for i32, i8 and i64 to-char conversion #6425

Are you sure you want to change the base?

Adding support for i32, i8 and i64 to-char conversion #6425

Conversation

viniciusfdasilva commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

danakj commented Nov 24, 2025

Uh oh!

danakj commented Nov 24, 2025

Uh oh!

danakj left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Adding support for `i32`, `i8` and `i64` to-`char` conversion #6425

Adding support for `i32`, `i8` and `i64` to-`char` conversion #6425

viniciusfdasilva commented Nov 24, 2025 •

edited

Loading