Fixed #2500 #3390

zhanghe9702 · 2025-01-28T07:14:13Z

Add a seperate typecheck pass collect impl blocks, then report errors.

Signed-off-by: Zhang He [email protected]

ChangLog:

    * gcc/rust/typecheck/rust-hir-inherent-impl-check.h: new typecheck pass
    * gcc/rust/typecheck/rust-hir-type-check.cc: modify the function which test primitive type
    * gcc/rust/typecheck/rust-tyty.cc: the new typecheck pass entrypoint
    * gcc/testsuite/rust/compile/issue-2500-rustc.rs: testsuite case same with rustc
    * gcc/testsuite/rust/compile/issue-2500.rs: testsuite case

Thank you for making Rust GCC better!

If your PR fixes an issue, you can add "Fixes #issue_number" into this
PR description and the git commit message. This way the issue will be
automatically closed when your PR is merged. If your change addresses
an issue but does not fully fix it please mark it as "Addresses #issue_number"
in the git commit message.

Here is a checklist to help you with your PR.

GCC development requires copyright assignment or the Developer's Certificate of Origin sign-off, see https://gcc.gnu.org/contribute.html or https://gcc.gnu.org/dco.html
Read contributing guidlines
make check-rust passes locally
Run clang-format
Added any relevant test cases to gcc/testsuite/rust/

Note that you can skip the above if you are just opening a WIP PR in
order to get feedback.

*Please write a comment explaining your change. This is the message
that will be part of the merge commit.

powerboat9 · 2025-01-28T12:07:33Z

gcc/rust/typecheck/rust-hir-type-check.cc

 #include "rust-hir-inherent-impl-overlap.h"
+#include "rust-hir-inherent-impl-check.h"


Duplicate includes

dkm · 2025-01-28T12:55:47Z

gcc/rust/typecheck/rust-hir-inherent-impl-check.h

@@ -0,0 +1,81 @@
+// Copyright (C) 2020-2024 Free Software Foundation, Inc.


I believe we're in 2025 now 😅

Suggested change

// Copyright (C) 2020-2024 Free Software Foundation, Inc.

// Copyright (C) 2020-2025 Free Software Foundation, Inc.

Should it be changed from 2020-2024 to just 2025, since it's a new file?

CohenArthur

Great work, you really did a good job using our existing framework and checking for the impls :D It's missing one bit of logic with the lang items I think, but this is honestly impressive - well done!

CohenArthur · 2025-01-28T14:13:23Z

gcc/rust/typecheck/rust-hir-inherent-impl-check.h

+#include "rust-hir-item.h"
+#include "rust-hir-type-check-base.h"
+#include "rust-type-util.h"
+#include <vector>


This include should be removed, you should include "rust-system.h" instead if it is necessary

CohenArthur · 2025-01-28T14:13:55Z

gcc/rust/typecheck/rust-hir-inherent-impl-check.h

+  void scan ()
+
+  {


Suggested change

void scan ()

{

void scan ()

{

CohenArthur · 2025-01-28T14:20:07Z

gcc/rust/typecheck/rust-hir-inherent-impl-check.h

+      if (is_primitive_type_kind (impl_type->get_kind ()))
+	{
+	  possible_primitive_impl.push_back (impl);
+	}


I think this is a good start, but we must also check if the implementation is a lang item. In Rust 1.49, this is the implementation for the official impl of i8 for example:

#[lang = "i8"] impl i8 { int_impl! { i8, i8, u8, 8, -128, 127, "", "", 2, "-0x7e", "0xa", "0x12", "0x12", "0x48", "[0x12]", "[0x12]", "", "" } }

because the standard library does define methods for the i8 type so we still need to be able to do that somewhere.

Here is another primitive implementation if you want to take a look: https://github.com/rust-lang/rust/blob/e1884a8e3c3e813aada8254edfa120e85bf5ffca/library/core/src/ptr/mut_ptr.rs#L6

We can maybe address this in a later PR if this doesn't cause any regressions though

:), i'm not sure if all these failed regressions just because missing lang item check, so many! sorry for that! :) maybe some of them need modify , I will take the time to do some reseach about your suggestions, thx!

Yes some of the regressions are due to this, e.g. in gcc/testsuite/rust/compile/const-issue1440.rs where we define the primitive implementation for integers (which I assume will cause your error to trigger). but you'll need to fix the testcase as well to add the attribute

CohenArthur · 2025-01-28T14:20:19Z

gcc/rust/typecheck/rust-hir-inherent-impl-check.h

+  void report_error (HIR::ImplBlock *impl)
+
+  {


Suggested change

void report_error (HIR::ImplBlock *impl)

{

void report_error (HIR::ImplBlock *impl)

{

zhanghe9702 · 2025-01-31T15:13:42Z

make check-rust pass at local machine, i will fix code format tomorrow

Add a seperate typecheck pass collect impl blocks, then report errors. gcc/rust/ChangeLog: * typecheck/rust-hir-inherent-impl-check.h: new typecheck pass * typecheck/rust-hir-type-check.cc: modify the function which test primitive type * typecheck/rust-tyty.cc: the new typecheck pass entrypoint * util/rust-hir-map.cc: add the map lookup lang item by defid * util/rust-hir-map.h: add the map lookup lang item by defid gcc/testsuite/ChangeLog: * rust/compile/issue-2500-rustc.rs: testsuite case same with rustc * rust/compile/issue-2500.rs: testsuite case * rust/compile/const-issue1440.rs: add lang item attribute * rust/compile/issue-1005.rs: add lang item attribute * rust/compile/issue-1130.rs: add lang item attribute * rust/compile/issue-1235.rs: add lang item attribute * rust/compile/issue-1237.rs: add lang item attribute * rust/compile/issue-2190-2.rs: add lang item attribute * rust/compile/issue-2905-2.rs: add lang item attribute * rust/compile/iterators1.rs: add lang item attribute * rust/compile/macros/mbe/macro-issue1233.rs: add lang item attribute * rust/compile/macros/mbe/macro54.rs: add lang item attribute * rust/compile/torture/intrinsics-8.rs: add lang item attribute * rust/compile/torture/issue-1075.rs: add lang item attribute * rust/compile/torture/issue-1432.rs: add lang item attribute * rust/execute/torture/issue-1436.rs: add lang item attribute * rust/execute/torture/issue-2236.rs: add lang item attribute * rust/execute/torture/iter1.rs: add lang item attribute * rust/execute/torture/str-layout1.rs: add lang item attribute Signed-off-by: Zhang He <[email protected]>

philberty · 2025-02-03T20:50:51Z

gcc/rust/typecheck/rust-hir-inherent-impl-check.h

+      // filtering  trait-impl-blocks
+      if (impl->has_trait_ref ())
+	return true;
+      HirId impl_ty_id = impl->get_type ().get_mappings ().get_hirid ();


I think some line breaks here will help make this easier to read maybe after those returns

zhanghe9702 force-pushed the HEAD branch 2 times, most recently from fe57261 to c8b9399 Compare January 28, 2025 08:17

powerboat9 reviewed Jan 28, 2025

View reviewed changes

dkm reviewed Jan 28, 2025

View reviewed changes

CohenArthur requested changes Jan 28, 2025

View reviewed changes

CohenArthur requested a review from philberty January 28, 2025 14:21

zhanghe9702 force-pushed the HEAD branch from c8b9399 to 638befc Compare January 31, 2025 14:54

zhanghe9702 force-pushed the HEAD branch 3 times, most recently from ac5a768 to 714c863 Compare February 1, 2025 02:03

zhanghe9702 force-pushed the HEAD branch from 714c863 to 4a3a0e1 Compare February 1, 2025 02:12

philberty reviewed Feb 3, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed #2500 #3390

Fixed #2500 #3390

zhanghe9702 commented Jan 28, 2025

powerboat9 Jan 28, 2025

dkm Jan 28, 2025

powerboat9 Jan 28, 2025

CohenArthur left a comment

CohenArthur Jan 28, 2025

CohenArthur Jan 28, 2025

CohenArthur Jan 28, 2025

zhanghe9702 Jan 28, 2025

CohenArthur Jan 28, 2025

CohenArthur Jan 28, 2025

zhanghe9702 commented Jan 31, 2025

philberty Feb 3, 2025

		#include "rust-hir-inherent-impl-overlap.h"
		#include "rust-hir-inherent-impl-check.h"

		@@ -0,0 +1,81 @@
		// Copyright (C) 2020-2024 Free Software Foundation, Inc.

	// Copyright (C) 2020-2024 Free Software Foundation, Inc.
	// Copyright (C) 2020-2025 Free Software Foundation, Inc.

Fixed #2500 #3390

Are you sure you want to change the base?

Fixed #2500 #3390

Conversation

zhanghe9702 commented Jan 28, 2025

Note that you can skip the above if you are just opening a WIP PR in order to get feedback.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CohenArthur left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhanghe9702 commented Jan 31, 2025

Choose a reason for hiding this comment

Note that you can skip the above if you are just opening a WIP PR in
order to get feedback.