rescript-association / reanalyze

Experimental analyses for ReScript and OCaml: globally dead values/types, exception analysis, and termination analysis.
MIT License
277 stars 20 forks source link

False alarms for native projects that generate new files #174

Closed Zeta611 closed 2 years ago

Zeta611 commented 2 years ago

Hi, reanalyze currently emits false alarms for native (OCaml) projects that generate new .ml files when built.

This MWE demonstrates this using ocamlyacc and ocamllex. In this example, reanalyze warns about a dead constructor that is actually visited by a generated pareser.

This is because reanalyze tries to find a file lib/parser.ml, while it is only present in the build target directory. For projects using dune, this directory defaults to _build/default/.

Zeta611 commented 2 years ago

Here is a pull request #175 that enables users to specify build target directory, though I'm not quite sure if this is a scalable solution. This does work for the MWE I provided.

cristianoc commented 2 years ago

It is already possible to specify paths, inclusions, exclusions from the command line, to look in specific folders and select only certain things. See the CLI arguments.

Zeta611 commented 2 years ago

Thank you for the quick response.

I'm a bit confused--is it possible to look inside the build target directory to remove the false warnings I mentioned above? I am unable to do so using the -unsuppress flag by adding the build target directory.

I only get a correct result when I navigate inside _build/default and change the current working directory, but not in the root project directory as guided in the README file; is this an intended behavior?

LimitEpsilon commented 2 years ago

Hi, I was trying out reanalyze with @Zeta611 earlier this day, and I also have some understanding about this issue.

The problem that @Zeta611 is pointing out is not the inability to specify folders to find .cmt files in.

Rather, it is a problem when Sys.file_exists is evaluated in src/FindSourceFile.ml. The path to the source file(.ml or .mli) is a relative path, and therefore is resolved relative to the current working directory.

Thus, when reanalyze is executed in the project root, even if we specify _build/default, for example, the source files are not found. Or is this already covered in the CLI arguments?

cristianoc commented 2 years ago

If I understand correctly. What if you go into _build/default then run the executable from there? Does it have the behaviour you need?

LimitEpsilon commented 2 years ago

Yes, going into _build/default and executing reanalyze works!

cristianoc commented 2 years ago

Is the option provided still useful? I don't mind merging as it's a simple change.

Zeta611 commented 2 years ago

Specifying a build target path is more straightforward than having to manually navigate inside the build target directory, so the new option is useful in dune projects IMHO.

cristianoc commented 2 years ago

PR merged.