ClojureScript

no arity warnings on recursive calls

Details

  • Type: Defect Defect
  • Status: Open Open
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: 1.7.145
  • Fix Version/s: 1.9.671
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code

Description

If a function recursively invokes itself within its own body the invoke will not be checked for arity mismatch.

Activity

Hide
Samuel Miller added a comment -

Took some time to look at this issue. Originally thought "Do what loop/recur does" but that does not take into account multi-arity. It seems like maybe the best option is to somehow use the second pass of the analyze(analyze-fn-methods-pass2). The entire information about the function is present and the warning section of the code gets triggered but because of no-warn is ignored. Any other ideas for a solution to this?

Show
Samuel Miller added a comment - Took some time to look at this issue. Originally thought "Do what loop/recur does" but that does not take into account multi-arity. It seems like maybe the best option is to somehow use the second pass of the analyze(analyze-fn-methods-pass2). The entire information about the function is present and the warning section of the code gets triggered but because of no-warn is ignored. Any other ideas for a solution to this?
Hide
Samuel Miller added a comment -

So I am looking for feed back on this patch and I will try to explain the reasoning for each section.

The issue is that a function only knows about it's arity after it has been parsed once.
So we need to check arity issues on the second pass

First off, added two new variables.
-activate-second-pass-warnings:Boolean Basically if you want to have second-pass warnings turned on
-second-pass-cljs-warnings:Set Right now we only have :fn-arity but I figure might as well make it generic.

So first up if the modifications to the analyze-fn-methods-pass2 function.
Instead of using no-warn marco here we have some new functionality.
The goal is to turn everything off except the second-pass warnings

So if activate-second-pass-warnings is false just use no-warn else it will use the new section of code.

The default-warning-handler was also modified. After checking if a warning is on, it checks if the warning is a second-pass warning and
if that warning can now be activated. If activate-second-pass-warnings is false AND a warning is still on that implies it is a second pass warning
in the second pass so we activate it.

Also I tried to keep all modifications in cljs.analyzer.

Originally I had the cljs-warnings :fn-arity to false and it would only be turned on in the second pass.
However the repl section just sets everything to true (and turns off select parts like ns errors).
So I decided to not touch those sections and instead keep how other files interface with the analyzer the same.

Show
Samuel Miller added a comment - So I am looking for feed back on this patch and I will try to explain the reasoning for each section. The issue is that a function only knows about it's arity after it has been parsed once. So we need to check arity issues on the second pass First off, added two new variables. -activate-second-pass-warnings:Boolean Basically if you want to have second-pass warnings turned on -second-pass-cljs-warnings:Set Right now we only have :fn-arity but I figure might as well make it generic. So first up if the modifications to the analyze-fn-methods-pass2 function. Instead of using no-warn marco here we have some new functionality. The goal is to turn everything off except the second-pass warnings So if activate-second-pass-warnings is false just use no-warn else it will use the new section of code. The default-warning-handler was also modified. After checking if a warning is on, it checks if the warning is a second-pass warning and if that warning can now be activated. If activate-second-pass-warnings is false AND a warning is still on that implies it is a second pass warning in the second pass so we activate it. Also I tried to keep all modifications in cljs.analyzer. Originally I had the cljs-warnings :fn-arity to false and it would only be turned on in the second pass. However the repl section just sets everything to true (and turns off select parts like ns errors). So I decided to not touch those sections and instead keep how other files interface with the analyzer the same.
Hide
Samuel Miller added a comment -

Just realized that I have the patch marked as .md instead of .patch

Show
Samuel Miller added a comment - Just realized that I have the patch marked as .md instead of .patch
Hide
Mike Fikes added a comment -

Patch no longer applies.

Show
Mike Fikes added a comment - Patch no longer applies.

People

Vote (5)
Watch (4)

Dates

  • Created:
    Updated: