Skip to content

Implement readonly checker on HIR #3881

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sakupan102
Copy link
Contributor

No description provided.

@sakupan102 sakupan102 marked this pull request as draft July 7, 2025 13:13
@philberty
Copy link
Member

your probably missing the using statement in the visitor

@@ -0,0 +1,150 @@
// Copyright (C) 2020-2025 Free Software Foundation, Inc.
Copy link
Member

Choose a reason for hiding this comment

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

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

we can just use 2025 here since it is a new file :)

@@ -0,0 +1,51 @@
// Copyright (C) 2020-2025 Free Software Foundation, Inc.
Copy link
Member

Choose a reason for hiding this comment

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

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

{
auto ident_pattern
= static_cast<HIR::IdentifierPattern *> (*maybe_pattern);
if (!ident_pattern->is_mut () && assignment_map[*hir_id] > 1)
Copy link
Member

Choose a reason for hiding this comment

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

do we want to check if it is greater than 0 here?

Copy link
Contributor Author

@sakupan102 sakupan102 Jul 9, 2025

Choose a reason for hiding this comment

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

I would like to check if the value is greater than 1.
In this context, assignment_map[hir_id] == 1 means the variable has been initialized, while assignment_map[hir_id] == 0 means it has only been declared.
If the assignment count is greater than 1, the variable has been assigned more than once, so I want to emit an error if the variable is not mutable.

== HIR::Item::ItemKind::Static)
{
auto static_item = static_cast<HIR::StaticItem *> (*maybe_static_item);
if (!static_item->is_mut () && assignment_map[*hir_id] > 1)
Copy link
Member

Choose a reason for hiding this comment

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

same here

namespace Rust {
namespace HIR {

static std::map<HirId, int> assignment_map = {};
Copy link
Member

Choose a reason for hiding this comment

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

if we are only interested in whether we've seen the HirId or not, we can use a set here

Suggested change
static std::map<HirId, int> assignment_map = {};
static std::set<HirId> assignment_map = {};

I agree that if you need the count/number of times we've seen the HirId, then a map is better 👍

gcc/rust/ChangeLog:

	* hir/tree/rust-hir-visitor.cc (DefaultHIRVisitor::walk):
	Add check before calling  `get_trait_ref()`

Signed-off-by: Ryutaro Okada <[email protected]>
gcc/rust/ChangeLog:

	* hir/tree/rust-hir-visitor.cc (DefaultHIRVisitor::walk):
	Cast to base class reuse the base class's accept_vis method

Signed-off-by: Ryutaro Okada <[email protected]>
@sakupan102 sakupan102 marked this pull request as ready for review July 21, 2025 16:44
@powerboat9
Copy link
Collaborator

You can skip the #include "options.h" and assume flag_name_resolution_2_0 is true -- name resolution 1.0 is in the middle of being removed, so your code should only have to work with name resolution 2.0

@sakupan102 sakupan102 requested a review from CohenArthur July 22, 2025 01:35
gcc/rust/ChangeLog:

	* Make-lang.in (rust-readonly-check2.cc):
	Add read-only check on HIR
	* checks/errors/rust-readonly-check2.cc (ReadonlyChecker):
	Add read-only check on HIR
	* checks/errors/rust-readonly-check2.h (ReadonlyChecker):
	Add read-only check on HIR

Signed-off-by: Ryutaro Okada <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants