Fix optional trait parsing #2745
Conversation
gcc/rust/parse/rust-parse-impl.h
Outdated
| if (lexer.peek_token()->get_id() == QUESTION_MARK) { | ||
| lexer.skip_token(); | ||
| rust_error_at(lexer.peek_token()->get_locus(), | ||
| "%<?Trait%> is not permitted in supertraits"); | ||
| return nullptr; | ||
| } |
There was a problem hiding this comment.
yes, as @powerboat9 showed we should not be emitting this error at the parsing stage, but probably in the typechecker. you can see that it parses properly in the playground link, or with a nightly rustc with the -Z parse-only CLI option
There was a problem hiding this comment.
if you'd like any guidance for adding that check to the type checker please do say so :)
There was a problem hiding this comment.
Thanks for the feedback. I'll go ahead and try to add the check to the typechecker.
dme2
force-pushed
the
optional_trait_supertrait
branch
from
c21374d to
2bce504
Compare
|
Hi @CohenArthur, I know the formatting isn't passing yet, but does that latest commit look ok? |
| HIR::TraitBound &tb | ||
| = static_cast<HIR::TraitBound &> (*tp_bound.get ()); | ||
| if (tb.get_polarity () == BoundPolarity::AntiBound) |
There was a problem hiding this comment.
this is good, but the static cast might fail if the trait bound is a lifetime.
you can switch on tp_bound.get_bound_type() and check if it is a lifetime or a trait, and proceed to cast safely then. so something like:
| HIR::TraitBound &tb | |
| = static_cast<HIR::TraitBound &> (*tp_bound.get ()); | |
| if (tb.get_polarity () == BoundPolarity::AntiBound) | |
| if (tp_bound.get_bound_type() == LIFETIME) { | |
| HIR::TraitBound &tb | |
| = static_cast<HIR::TraitBound &> (*tp_bound.get ()); | |
| if (tb.get_polarity () == BoundPolarity::AntiBound) | |
| /* snip */ | |
| } |
etc. does that make sense?
There was a problem hiding this comment.
@CohenArthur, thank you for the review. Do you have an example of the when the static cast might fail?
This program seems to compile fine with the code as is:
#[lang = "sized"]
pub trait Sized {}
trait Trait<'a>: 'a {}There was a problem hiding this comment.
ah, my bad - the static cast won't fail, but the value will be undefined. so we still need to do the check.
gcc/rust/ChangeLog: * typecheck/rust-hir-type-check-item.cc (TypeCheckItem::visit): Check for ?Trait in visitor gcc/testsuite/ChangeLog: * rust/compile/issue-2725.rs: New test. Signed-off-by: Dave Evans <[email protected]>
dme2
force-pushed
the
optional_trait_supertrait
branch
from
2bce504 to
1429964
Compare
|
@dme2 What is the current state of this PR ? From what I've read it seems like there is only a minor format issue ? |
|
Closed in favor of #3055 (This is exactly the same PR with proper code format). Thank you for the initial work! |
Addresses #2725