Skip to content

Fix bugs in dialyzer task #49

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 6 commits into
base: master
Choose a base branch
from

Conversation

UncleGrumpy
Copy link
Collaborator

Fixes several bugs in the atomvm dialyzer task. Beam paths are correctly discovered when using the AtomVM/build directory as the atomvm_root. A bug which caused the plt check to always fail, causing a complete rebuild was fixed. The full atomvm version is used in the base PLT name so PLTs from different branches can be used without loosing the previous ones. The task will now abort when an error occurs rather than try to keep going. Improved error messages, stack traces are only printed (with better formatting) when running rebar3 with DEBUG=1.

Fixes a bug when checking the base PLT which caused it to always be rebuilt.

Signed-off-by: Winford <[email protected]>
Keep the full version when naming the base PLT file to allow greating base PLTs for differnt git
commits (or different branches) when working on the same base version.

Signed-off-by: Winford <[email protected]>
@UncleGrumpy UncleGrumpy force-pushed the fix_dialyzer_bugs branch 2 times, most recently from 54b4fb1 to 8bf9af3 Compare June 18, 2025 04:43
@UncleGrumpy UncleGrumpy requested a review from pguyot June 18, 2025 05:28
Copy link
Collaborator

@pguyot pguyot left a comment

Choose a reason for hiding this comment

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

Unrelated to these changes, there is a problem with the paths.
I needed to comment or move these lines:
rebar_paths:unset_paths([plugins], State),
rebar_paths:set_paths([deps], State),

Also, with atomvm_benchmark project, when building the app PLT I got a lot of undefined functions that should be in AtomVM PLT.

benchmark.erl:0: Unknown function erlang:get_module_info/1
benchmark.erl:0: Unknown function erlang:get_module_info/2
benchmark.erl:15:24: Unknown function atomvm:platform/0
benchmark.erl:19:21: Unknown function io:format/2

And subsequent calls work, but I have a failure again if I delete the benchmark.plt file.

@UncleGrumpy
Copy link
Collaborator Author

Unrelated to these changes, there is a problem with the paths. I needed to comment or move these lines: rebar_paths:unset_paths([plugins], State), rebar_paths:set_paths([deps], State),

I forgot to investigate those closer, it turns out they should not have been used at all. I removed them.

Also, with atomvm_benchmark project, when building the app PLT I got a lot of undefined functions that should be in AtomVM PLT.

benchmark.erl:0: Unknown function erlang:get_module_info/1
benchmark.erl:0: Unknown function erlang:get_module_info/2
benchmark.erl:15:24: Unknown function atomvm:platform/0
benchmark.erl:19:21: Unknown function io:format/2

And subsequent calls work, but I have a failure again if I delete the benchmark.plt file.

I will look into this and see what I did wrong.

@UncleGrumpy UncleGrumpy force-pushed the fix_dialyzer_bugs branch 3 times, most recently from ea789d2 to ada37af Compare June 18, 2025 23:54
@UncleGrumpy
Copy link
Collaborator Author

It turned out that I just needed to ignore warnings when generating the app PLT. The app PLT was being created properly but also issuing warnings when being built that should be ignored (as long as they aren't errors) until the actual analysis is done after making sure the base and application PLT are generated or checked if they are out of date.

@pguyot
Copy link
Collaborator

pguyot commented Jun 19, 2025

With the latest commit (ada37af), I get the following warning

     ┌─ _build/default/plugins/atomvm_rebar3_plugin/src/atomvm_dialyzer_provider.erl:
     │
 222 │  do_build_plt(Config, State) ->
     │               ╰── Warning: variable 'Config' is unused


Other than that, a quick test shows it behaves as expected 💯
For example:

  • it relates if I use a function that is not declared in AtomVM libs but exists with OTP (I tested with io:rows/0).
  • it generates an error if I use a function that that does exist in either AtomVM libs or OTP (I tested with io:formatz/2)
  • it detects type error within my module (I tried to match with a tuple a value in a function that was the result of either erlang:system_time(microsecond) or erlang:system_time(millisecond) * 1000).
  • it detects contract errors for NIFs (it detects a call to erlang:system_time(millisecondz)

It does not detect some items related to types that we should add. For example it didn't detect a call to erlang:system_flag(schedulers_onlinez, 1), but I guess that's because we don't define the flags that are valid in the spec (at least in the AtomVM version that is installed on my machine).

Thank you! It shoud prove a very valuable tool.

@UncleGrumpy
Copy link
Collaborator Author

With the latest commit (ada37af), I get the following warning

     ┌─ _build/default/plugins/atomvm_rebar3_plugin/src/atomvm_dialyzer_provider.erl:
     │
 222 │  do_build_plt(Config, State) ->
     │               ╰── Warning: variable 'Config' is unused

Thank you I missed that at the beginning since I ran in diagnostic mode the last few times and didn't scroll back far enough when I got the results I was looking for. Fixed now.

@pguyot
Copy link
Collaborator

pguyot commented Jun 19, 2025

I confirm the warning is gone with 076adff

{error, no_beams_found};
Ebins ->
Ebins
end.

% @private
libs_not_found() ->
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks like lists:duplicate/2 to me.

Libs =
case [filelib:wildcard(Base ++ "/lib/" ++ atom_to_list(Lib) ++ "*/ebin") || Lib <- ?LIBS] of
[] ->
[filename:join(Base, atom_to_list(Lib2) ++ "/src/beams") || Lib2 <- ?LIBS];
NotFound ->
Copy link
Collaborator

Choose a reason for hiding this comment

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

This logic could be simplified or improved.
Do we want to add for each lib in ?LIBS either ebin or src/beams?

I would suggest doing a foldl on the list instead than matching with [[], [], ..., []].

Fixes a bug where the beam files are not found when using source builds from Atomvm/build directory
as the atomvm_root path.

Signed-off-by: Winford <[email protected]>
Removes unnecessary path changes in the do/1.

Signed-off-by: Winford <[email protected]>
Fixes several instances where the task would try to complete after encountering an error. Improves
output when errors do occur by printing an informative message under noral opperation, and only
print the full stacktrace when DEBUG=1 is used. When displayed stacktraces are formatted better by
starting them on a new line.

Signed-off-by: Winford <[email protected]>
Removes unnecessary checks for warnings when building the application PLT for the first time, these
should be ignored until the actual analysis after both the base and application PLT are checked or
created.

Signed-off-by: Winford <[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.

2 participants