Hi Philip,
On Thu, Sep 30, 2021 at 11:46:30AM +0100, Philip Herron wrote:
> Your patch was 99% of the way there to fix the type
> resolution so I finished it off for you:
>
> https://github.com/Rust-GCC/gccrs/pull/698/files
And you added the actual backend code needed! So I'll probably say I
was maybe 45% there :) This is really useful. It works for me and will
enable writing a couple of real tests for byte strings (like we have
for str already, that aren't just lexer/parser checks, but which can
actually check the contents of the byte strings.
> The missing piece was that References and Array's are a type of covariant
> type so that an array type can look like this: [_, capacity], so the
> inference variable here is the variant so that we need to make sure it has
> its own implicit mapping id. You just needed to create one more mapping to
> get that implicit id so that the reference type similarly doesn't get into
> a loop of looking up itself. Creating implicit types like this could be
> made easier, so we should likely add some helpers for this scenario.
Thanks, my knowledge of mappings was a bit fuzzy. So even though we
know our own array type is fixed, we still need to attach a unique
mapping to match against other types which might not be fixed (yet).
There is indeed a lot of extra work you need to do and that you can
easily get wrong. Creating an empty mapping is a lot of work and it is
easy to get the details right. An new_empty_mapping helper method
would be great. Maybe it could even be hidden inside the expression or
type constructor.
I do have a question about why we also create these mappings for the
capacity. The capacity is always a fixed usize number. Not just in
this case, but always since it is constant expression in a const
context. So any comparison of the length will always be a simple
constant usize number check. Is using mappings not overkill in that
case?
We use a Bexpression for the capacity, created by
ConstFold::ConstFoldExpr::fold, shouldn't we check it actually folded
correctly? Currently when it cannot we just leave it in an error state
till it is too late.
e.g
pub fn main()
{
let _a: &[u8; 4 / 0];
_a = b"test";
}
Results in:
rust1: internal compiler error: in const_size_val_to_string, at
rust/rust-gcc.cc:210
0x794e67 Gcc_backend::const_size_val_to_string[abi:cxx11](Bexpression*)
/home/mark/src/gccrs/gcc/rust/rust-gcc.cc:210
0x794e67 Gcc_backend::const_size_val_to_string[abi:cxx11](Bexpression*)
/home/mark/src/gccrs/gcc/rust/rust-gcc.cc:208
0x8c51ef Rust::TyTy::ArrayType::capacity_string[abi:cxx11]() const
/home/mark/src/gccrs/gcc/rust/typecheck/rust-tyty.cc:1136
0x8c51ef Rust::TyTy::ArrayType::as_string[abi:cxx11]() const
/home/mark/src/gccrs/gcc/rust/typecheck/rust-tyty.cc:1129
0x8d0c8b Rust::TyTy::BaseCoercionRules::visit(Rust::TyTy::ArrayType&)
/home/mark/src/gccrs/gcc/rust/typecheck/rust-tyty-coercion.h:166
[...]
So it seems we really should check for is_error_expr earlier. And
maybe just get the integer value and store/compare that instead of the
Bexpression?
P.S. Don't let this stop you from committing the code as is, it does
work. We can rework it afterwards if the above comments make any
sense.
Cheers,
Mark
--
Gcc-rust mailing list
Gcc-rust@gcc.gnu.org
https://gcc.gnu.org/mailman/listinfo/gcc-rust
