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
There should be a warning for repeated variables even in curried arguments #6070
Comments
Comment author: @alainfrisch Currently, "fun p1 p2 -> e" is treated exactly as "fun p1 -> fun p2 -> e". What would be the exact criterion to trigger the warning? Should it collect such immediately nested abstractions and check that they bind disjoint set of variables? What about the following: let f x ?(a = x) x = a + x Here we have a duplicated variable, but the instances are both used. Personally, I find warning 27 quite useful (and it is turned into an error in LexiFi's code base). What's wrong with it? |
Comment author: kaustuv I think that if the user writes "fun x x -> x", then it is nearly always something strange and possibly unintentonal. This is also true for "fun x ?(a=x) x -> x". Warning 27 also complains about such as "let snd x y = y" and if it is an error then I have to constantly fiddle with initial underscore. I am sure one can get used to it, but I would rather differentiate between "repeated uses of variable in function arguments" and "unused variable in function arguments" as potential sources of errors in my code. |
Comment author: @damiendoligez Maybe the criterion could be "the variable is shadowed before it is ever used", without regard for the shape of the expression. I don't much like a special warning just for function definitions. |
Comment author: kaustuv I think I am OK with that interpretation. In fact, that could just be a new case of "suspicious unused variable" (warning 26). Although, enlarging the scope of existing warnings risks breaking existing code that mark those warnings as errors. |
This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc. |
How could we implement @damiendoligez 's suggestion? |
This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc. |
This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc. |
What shall we do according to you, @garrigue?
My understanding of the issue is that there is something we would like
to tackle, so the issue needs to stay open, but we don't know yet how to
best tackle it. Do you (and others) share this understanding?
One other question: is the original author still interested, nine years
later?
|
I have no strong opinion on this one. There is already a warning for unused variables, which would apply in this case, so this could be a sufficient answer. @damiendoligez would be nicer for a warning enabled by default, but this is extra work for something that is already covered by another warning. |
Okay I am going to close this and it can always be re-opened if somebody
feels the need to.
|
Original bug ID: 6070
Reporter: kaustuv
Status: acknowledged (set by @damiendoligez on 2013-07-17T14:57:26Z)
Resolution: open
Priority: normal
Severity: feature
Version: 4.00.1
Category: typing
Monitored by: @gasche @hcarty
Bug description
I've just spent half an hour bug hunting an issue that came from code of this form (greatly simplified):
Obviously, this function is equivalent to fun _ (E x) -> x, which is silently the wrong interpretation of my intentions.
Basically, I would have spotted the error right away if instead I had written
but, since first_msg was being used in fold_left, I had to write it curried. The nature of this bug in my code took me on a complete wild goose chase as I hadn't assumed that the bug would be in my printing routine for errors.
Can there be a warning for shadowed variables in the arguments of curried function definitions? Note that this needs to be different from warning 27, which is way too annoying even to turn on, far less mark as error.
The text was updated successfully, but these errors were encountered: