Enforce intrinsic signatures during the type checking#4587
Conversation
P-E-P
left a comment
P-E-P
left a comment
There was a problem hiding this comment.
I'm not 100% sold on this approach as I feel it duplicates a lot of work (you had to implement all those "to_string" functions and we get another representation for types).
However, work has been done and is here, ready to be used. This is a low priority issue and it works as is, it doesn't make sense to work on another way which could very well be unsatisfactory. I'm willing to merge this once the few minor comments have been addressed.
|
|
||
| extern "rust-intrinsic" { | ||
| pub fn ctlz<T>(x: T) -> u32; | ||
| pub fn ctlz<T>(x: T) -> T; |
There was a problem hiding this comment.
Good catch, this was changed since 1.49.
|
|
||
| Fn_MutPtrU8, // fn(*mut u8) | ||
| Fn_MutPtrU8MutPtrU8, // fn(*mut u8, *mut u8) | ||
| }; |
There was a problem hiding this comment.
Interesting approach, I wonder if it wouldn't be easier to "build" the expected signature type like we do with builtin macros within the compiler and then make sure both function match ?
Anyway this is just a random idea thrown out of thin air, I'm not asking you to reimplement that thing differently.
There was a problem hiding this comment.
I actually thought about doing it that way initially, but decided to go with this approach as it was the fastest solution for a low-priority issue. I agree it would definitely be a much more natural fit, though. Would you like me to open an issue to document this technical debt?
This patch adds builtin intrinsic names to Values::Intrinsics class and updates the BuiltinContext mappings to use these constants instead of hardcoded raw strings. gcc/rust/ChangeLog: * backend/rust-builtins.cc (BuiltinsContext::register_rust_mappings): Use Values::Intrinsics constants instead of raw strings. * util/rust-intrinsic-values.h (class Intrinsics): Add missing builtin intrinsics. Signed-off-by: Enes Cevik <enes@nsvke.com>
This patch adds a validation mechanism to the typechecker. This mechanism verifies intrinsic declarations against predefined signature rules. If a signature is incorrect, the compiler emits the proper diagnostic and prevents the malformed intrinsic from reaching the backend. gcc/rust/ChangeLog: * Make-lang.in: Add rust-hir-type-check-intrinsic.o. * typecheck/rust-hir-type-check-implitem.cc (TypeCheckTopLevelExternItem::visit): Check intrinsic signature. * typecheck/rust-hir-type-check-intrinsic.cc: New file. * typecheck/rust-hir-type-check-intrinsic.h: New file. gcc/testsuite/ChangeLog: * rust/compile/ctlz.rs: Fix incorrect intrinsic signature. * rust/compile/ctlz_nonzero.rs: Likewise. * rust/compile/cttz.rs: Likewise. * rust/compile/cttz_nonzero.rs: Likewise. * rust/compile/issue-4245.rs: Likewise. * rust/compile/torture/intrinsics-3.rs: Likewise. * rust/execute/torture/ctlz.rs: Likewise. * rust/execute/torture/ctlz_i16.rs: Likewise. * rust/execute/torture/ctlz_i32.rs: Likewise. * rust/execute/torture/ctlz_i64.rs: Likewise. * rust/execute/torture/ctlz_i8.rs: Likewise. * rust/execute/torture/ctlz_nonzero.rs: Likewise. * rust/execute/torture/ctlz_nonzero_i16.rs: Likewise. * rust/execute/torture/ctlz_nonzero_i32.rs: Likewise. * rust/execute/torture/ctlz_nonzero_i64.rs: Likewise. * rust/execute/torture/ctlz_nonzero_i8.rs: Likewise. * rust/execute/torture/ctlz_nonzero_u16.rs: Likewise. * rust/execute/torture/ctlz_nonzero_u32.rs: Likewise. * rust/execute/torture/ctlz_nonzero_u64.rs: Likewise. * rust/execute/torture/ctlz_nonzero_u8.rs: Likewise. * rust/execute/torture/ctlz_u16.rs: Likewise. * rust/execute/torture/ctlz_u32.rs: Likewise. * rust/execute/torture/ctlz_u64.rs: Likewise. * rust/execute/torture/ctlz_u8.rs: Likewise. * rust/execute/torture/cttz.rs: Likewise. * rust/execute/torture/cttz_nonzero.rs: Likewise. * rust/compile/intrinsic-checker.rs: New test. Signed-off-by: Enes Cevik <enes@nsvke.com>
|
I've addressed the minor comments. To be honest, I'm not 100% sold on it either 😅. But I agree it's best to not sink more time into this low-priority issue. |
|
yeah i think there are a few issues here i get what its trying to do though. But since i rebased ontop of this its breaking my GATS work i'm gonna have to make it a bit more permissive.. but also the to_string implementations are not needed here |
3 participants
This patch adds a validation mechanism to the typechecker. This mechanism
verifies intrinsic declarations against predefined signature rules. If
a signature is incorrect, the compiler emits the proper diagnostic and
prevents the malformed intrinsic from reaching the backend.