Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gccrs: add support for lang_item eq and PartialEq trait #3347

Merged
merged 1 commit into from
Jan 7, 2025

Conversation

philberty
Copy link
Member

The Eq and Partial Ord are very similar to the operator overloads we support for add/sub/etc... but they differ in that usually the function call name matches the name of the lang item. This time we need to have support to send in a new path for the method call on the lang item we want instead of just the name of the lang item.

NOTE: this test case doesnt work correctly yet we need to support the derive of partial eq on enums to generate the correct comparison code for that.

Fixes #3302

gcc/rust/ChangeLog:

* backend/rust-compile-expr.cc (CompileExpr::visit): handle partial_eq possible call
* backend/rust-compile-expr.h: handle case where lang item calls differ from name
* hir/tree/rust-hir-expr.cc (OperatorExprMeta::OperatorExprMeta): new helper
* hir/tree/rust-hir-expr.h: likewise
* typecheck/rust-hir-type-check-expr.cc (TypeCheckExpr::visit): handle partial_eq
(TypeCheckExpr::resolve_operator_overload): likewise
* typecheck/rust-hir-type-check-expr.h: likewise
* util/rust-lang-item.cc (LangItem::ComparisonToLangItem): map comparison to lang item
(LangItem::ComparisonToSegment): likewise
* util/rust-lang-item.h: new lang items PartialOrd and Eq
* util/rust-operators.h (enum class): likewise

gcc/testsuite/ChangeLog:

* rust/compile/cmp1.rs: New test.

Copy link
Member

@CohenArthur CohenArthur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

had a quick first look and it looks good, I'll review in more detail later

gcc/rust/util/rust-lang-item.cc Outdated Show resolved Hide resolved
@philberty
Copy link
Member Author

philberty commented Jan 6, 2025

For clarity this is gimple getting generated at the end of the day:

  try          
    {
      _c1 = <test::Book as test::PartialEq::<test::BookFormat>>::eq (&b1, &D.501);  
      _c2 = <test::BookFormat as test::PartialEq::<test::Book>>::eq (&D.502, &b2); 
      _c3 = test::PartialEq::ne<Book, Book> (&b1, &b2); 
      return D.503;
    }
  finally                                               
    {                                                   
      b2 = {CLOBBER(eos)};       
      b1 = {CLOBBER(eos)};                                                                                                                                                                                                       
    }

Copy link
Member

@CohenArthur CohenArthur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! thanks Philip!

gcc/rust/typecheck/rust-hir-type-check-expr.cc Outdated Show resolved Hide resolved
@CohenArthur
Copy link
Member

CohenArthur commented Jan 7, 2025

@philberty what is the part that should be derive(PartialEq) in the testcase?

@philberty
Copy link
Member Author

We need it on the enum because enum comparisons are not correct because we need that derive to do it using that enum discriminant intrinsic

The Eq and Partial Ord are very similar to the operator overloads
we support for add/sub/etc... but they differ in that usually the
function call name matches the name of the lang item. This time
we need to have support to send in a new path for the method call
on the lang item we want instead of just the name of the lang item.

NOTE: this test case doesnt work correctly yet we need to support
the derive of partial eq on enums to generate the correct comparison
code for that.

Fixes #3302

gcc/rust/ChangeLog:

	* backend/rust-compile-expr.cc (CompileExpr::visit): handle partial_eq possible call
	* backend/rust-compile-expr.h: handle case where lang item calls differ from name
	* hir/tree/rust-hir-expr.cc (OperatorExprMeta::OperatorExprMeta): new helper
	* hir/tree/rust-hir-expr.h: likewise
	* typecheck/rust-hir-type-check-expr.cc (TypeCheckExpr::visit): handle partial_eq
	(TypeCheckExpr::resolve_operator_overload): likewise
	* typecheck/rust-hir-type-check-expr.h: likewise
	* util/rust-lang-item.cc (LangItem::ComparisonToLangItem): map comparison to lang item
	(LangItem::ComparisonToSegment): likewise
	* util/rust-lang-item.h: new lang items PartialOrd and Eq
	* util/rust-operators.h (enum class): likewise

gcc/testsuite/ChangeLog:

	* rust/compile/nr2/exclude: nr2 cant handle this
	* rust/compile/cmp1.rs: New test.

Signed-off-by: Philip Herron <herron.philip@googlemail.com>
@philberty philberty added this pull request to the merge queue Jan 7, 2025
Merged via the queue into master with commit e891887 Jan 7, 2025
12 checks passed
@philberty philberty deleted the phil/partial-eq-1 branch January 10, 2025 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

libcore cmp.rs implementation
2 participants