Fix segfault when processing types with unresolved lifetimes in generic arguments#4436
Fix segfault when processing types with unresolved lifetimes in generic arguments#4436v1bh475u wants to merge 3 commits intoRust-GCC:masterfrom
Conversation
v1bh475u
force-pushed
the
fix/typecheck-unresolved-lifetime-segfault
branch
2 times, most recently
from
b9b1d7c to
d4b0c14
Compare
|
The build fails due to filename. Will fix it but need help for the changelog. After we are certain about the changelog, I will do a final rebase to finish all the changes. |
| generic_segment.get_generic_args ()); | ||
| // regions_from_generic_args returns empty vector on error | ||
| if (!generic_segment.get_generic_args ().get_lifetime_args ().empty () | ||
| && regions.empty ()) |
There was a problem hiding this comment.
This change doesnt seem right to me, if the regions are empty and no lifetimes are specified i feel like we should continue on anyway it shouldnt make us return early.
There was a problem hiding this comment.
This handles situation when when the lifetimes are specified and we get no regions. This is due to the fact that whenever regions_from_generic_args is unable to resolve any lifetime, it logs error and returns empty vector.
gccrs/gcc/rust/typecheck/rust-typecheck-context.cc
Lines 607 to 611 in 6eec7e7
Hence, I am checking if we had lifetimes in generic args but didn't receive any regions implying that there is some unresolved region.
There was a problem hiding this comment.
It'd probably be best to make regions_from_generic_args return a tl::optional, with tl::nullopt indicating an error.
There was a problem hiding this comment.
That would require a lot of changes in the codebase. There are a lot of places where it is always expected to return a value instead of having optional value. Is this what we are aiming to do here?
| { | ||
| TyTy::BaseType *base = TypeCheckType::Resolve (type.get_base_type ()); | ||
|
|
||
| if (base->get_kind () == TyTy::TypeKind::ERROR) |
There was a problem hiding this comment.
i think remove the new line in between base and this check this seems like the fix.
you can also do:
base->isTyTy::ErrorType () is a nicerway to do this if statement.
There was a problem hiding this comment.
I am unable to find this function. Do you mean the following?
gccrs/gcc/rust/typecheck/rust-tyty.h
Line 302 in 6eec7e7
There was a problem hiding this comment.
That's probably what phil meant
gcc/rust/ChangeLog: * typecheck/rust-hir-type-check-type.cc (TypeCheckType::resolve_root_path): check for unresolved lifetimes. Signed-off-by: vibhatsu <maulikbarot2915@gmail.com>
gcc/rust/ChangeLog: * typecheck/rust-hir-type-check-type.cc (TypeCheckType::visit): propagate error type in ReferenceType resolution Signed-off-by: vibhatsu <maulikbarot2915@gmail.com>
gcc/testsuite/ChangeLog: * rust/compile/unresolved-lifetime-in-generic-arg.rs: New test. Signed-off-by: vibhatsu <maulikbarot2915@gmail.com>
v1bh475u
force-pushed
the
fix/typecheck-unresolved-lifetime-segfault
branch
from
d4b0c14 to
e2c1bf5
Compare
3 participants
make check-rustpasses locallyclang-formatgcc/testsuite/rust/Fixes #3611
When resolving type paths with generic arguments, if a lifetime fails to resolve,
regions_from_generic_args()returns an empty vector. Previously, this empty vector was passed toSubstMapper::Resolve, creating types with uninitialized Region data. Later, variance analysis would crash when processing these malformed types.Fix:
resolve_root_path(): check ifregions_from_generic_args()failed and returnErrorTypeimmediately instead of creating malformed typesvisit(ReferenceType&): propagateErrorTypewhen encountered