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

Removed obsolete objects #2968

Merged
merged 1 commit into from
May 6, 2024
Merged

Removed obsolete objects #2968

merged 1 commit into from
May 6, 2024

Conversation

braw-lee
Copy link
Contributor

Fixes #2967

Created this PR to check if my new setup was working for PRs or not,
I can delete this if you want to save this issue as good-first-pr for first contributions.

Comment on lines +1318 to +1314
// lookup compiled functions since it may have already been compiled
fn_expr = resolve_method_address (fntype, receiver, expr.get_locus ());
Copy link
Member

Choose a reason for hiding this comment

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

hmmm, I feel like there used to be/should be a method cache lookup here which does not exist for some reason? maybe it would be better to comment out the two objects and add a FIXME comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh ok, fixme comment something like this?

// FIXME : method cache lookup
// comment deleted lines
// here
code...

Copy link
Member

Choose a reason for hiding this comment

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

yes something like this would be good :) but could you investigate that piece of code? was there method lookup at some point here that was removed, or was it just never added?

Copy link
Contributor Author

@braw-lee braw-lee Apr 24, 2024

Choose a reason for hiding this comment

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

I checked with git blame, what I found was that the 2nd object was passed to the below function first, but later function didn't need it, and the above 2 lines were left unchanged

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i asked @philberty about these 2 lines, he told me to go for it
Philip, do you remember what Arthur is talking about?

Copy link
Member

Choose a reason for hiding this comment

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

that's fine then, we can go ahead and remove them :D thanks for checking!

@CohenArthur CohenArthur added this pull request to the merge queue Apr 24, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Apr 24, 2024
gcc/rust/ChangeLog:

	* backend/rust-compile-expr.cc (CompileExpr::visit): Lines
	removed as the objects are unused.

Signed-off-by: Kushal Pal <kushalpal109@gmail.com>
@P-E-P P-E-P added this pull request to the merge queue May 6, 2024
Merged via the queue into Rust-GCC:master with commit 3b9a040 May 6, 2024
9 checks passed
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.

Obsolete objects, can be removed safely
3 participants