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

Remove unused code #972

Merged
merged 1 commit into from
Nov 19, 2023
Merged

Remove unused code #972

merged 1 commit into from
Nov 19, 2023

Conversation

kraigher
Copy link
Collaborator

@kraigher kraigher commented Nov 4, 2023

This removes a bunch of dead code that I found when developing unused code detection into VHDL LS.

@LarsAsplund
Copy link
Collaborator

Nice feature! I like that it can see unused code in several steps. That one line will be unused once you remove another unused line.

I'm thinking such a check should be part of our lint job.

@LarsAsplund LarsAsplund merged commit c16718d into master Nov 19, 2023
22 of 29 checks passed
@LarsAsplund
Copy link
Collaborator

LarsAsplund commented Nov 20, 2023

After the merge I had had a look at what dead code VHDL LS did not find:

check.vhd:77
check.vhd:863
com_deprecated.vhd:912
com_types.vhd:454
common_log_pkg-body.vhd:16
external_integer_vector_pkg.vhd:28
external_integer_vector_pkg.vhd:29
external_integer_vector_pkg.vhd:30
external_integer_vector_pkg.vhd:36
external_integer_vector_pkg.vhd:37
external_integer_vector_pkg.vhd:43
external_string_pkg.vhd:28
external_string_pkg.vhd:29
external_string_pkg.vhd:30
external_string_pkg.vhd:36
external_string_pkg.vhd:37
external_string_pkg.vhd:43
integer_vector_ptr_pkg-body-2002p.vhd:59
integer_vector_ptr_pkg-body-93.vhd:13
location_pkg-body-2008m.vhd:8
run.vhd:389
run.vhd:426
run_tests.vhd:167
string_ptr_pkg-body-2002p.vhd:62
string_ptr_pkg-body-93.vhd:13
sync_pkg-body.vhd:43
tb_avalon_master.vhd:77
tb_avalon_slave.vhd:64
tb_check_relation.vhd:45
tb_check_relation.vhd:59
tb_com_codec.vhd:33
tb_com_codec.vhd:34
tb_com_codec.vhd:35
tb_com_codec.vhd:36
tb_com_codec.vhd:37
tb_com_codec.vhd:44
tb_com_codec.vhd:76
tb_com_codec.vhd:76
tb_com_codec.vhd:78
tb_wishbone_master.vhd:70
tb_wishbone_slave.vhd:63

@kraigher
Copy link
Collaborator Author

Are you basing you conclusion on the contents of this MR? This MR does not remove all dead code within VUnit that VHDL-LS finds. For example it also finds check.vhd:77 that I have not included as part of this MR. Also regarding check.vhd:21 it is the function result which is used a lot so that should not be unused.

Note that the VHDL-LS unused declaration detection only warns for things that are locally unused. Since VHDL does not have a public/private concept it does not assume that things in a package header are unused as it could be intended to be used by a third party. Adding support for globally unused things would require some sideband configuration of what should be considered private and public which I have not implemented. I still think the locally unused stuff is the most important and most likely to find bugs. An unused signal, port, variable of function argument is much more likely to be a bug than an unused function in a package.

@LarsAsplund
Copy link
Collaborator

Are you basing you conclusion on the contents of this MR? This MR does not remove all dead code within VUnit that VHDL-LS finds.

I did. I noticed one dead code line and then I created a report from another tool. I didn't check the code in that report but the line I noticed was this

@kraigher
Copy link
Collaborator Author

Yes buf on that line is unused code. But it cannot be removed since allocate has a side effect.

@LarsAsplund
Copy link
Collaborator

You're right and the comment in the other tool is that the buf is not used. I also had it setup poorly so it reported on many strange things. Updated the list above.

I still haven't looked at all of them but these lines look correctly reported:

$ git grep -ni null_storage
vunit/vhdl/data_types/src/integer_vector_ptr_pkg-body-2002p.vhd:59:    constant null_storage : storage_t := (integer'low, internal, integer'low);
vunit/vhdl/data_types/src/integer_vector_ptr_pkg-body-93.vhd:13:  constant null_storage : storage_t := (integer'low, internal, integer'low);
vunit/vhdl/data_types/src/string_ptr_pkg-body-2002p.vhd:62:    constant null_storage : storage_t := (integer'low, internal, integer'low);
vunit/vhdl/data_types/src/string_ptr_pkg-body-93.vhd:13:  constant null_storage : storage_t := (integer'low, internal, integer'low);

@kraigher
Copy link
Collaborator Author

Those lines are found by VHDL LS as well.

@eine eine deleted the remove-dead-code branch February 24, 2024 22:19
@umarcor
Copy link
Member

umarcor commented Mar 11, 2024

See #997.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants