Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0007616OCamlmiddle end (typedtree to clambda)public2017-08-31 13:452017-10-02 14:16
Assigned To 
Platformlinux x86_64OSfedora 26OS Version4.12.8-300.fc26
Product Version4.04.2 
Target VersionFixed in Version 
Summary0007616: Compiler emits Warning 59 on array assignment
DescriptionCompiler emits warning 59 being emitted when I assign array to ref.
`x := [||];` works fine, but `x := CCArray.filter_map filter !x;`
(CCArray.filter_map from containers package [1]) emits warning 59 (A potential assignment to a non-mutable value was detected)

1. [^]
Steps To Reproduce1) compiler: 4.04.2 + flambda

2) set -O3 flag

3) compile a trivial program containing `x := CCArray.filter_map filter !x;`
Additional InformationTrivial example with the corresponded Makefile is attached
TagsNo tags attached.
Attached Filesgz file icon test.tar.gz [^] (512 bytes) 2017-08-31 13:45

- Relationships

-  Notes
octachron (developer)
2017-09-01 16:01

Interestingly, the warning is not triggered with 4.05.0+flambda but rears its head again with trunk.
xclerc (reporter)
2017-09-04 12:35

The problem is related to the definition of `Simple_value_approx.is_definitely_immutable`,
that considers `Value_block` approximations as immutable. The following commit fixes the
definition: [^]

(Note that the function is used only to trigger the warning.)

However, it would entail a small regression, as examplified by some of the tests present
in `testsuite/tests/warnings/`. The warnings for the mutation of a
tuple would disappear. Indeed, as approximations for tuples and arrays are shared, I do not
see how flambda could generate a warning for the former but not for the latter.

It thus depends on whether we prefer false positives or false negative. One problem with
this particular false positive though is that the user cannot easily get rid of it (apart
from disabling the warning).
octachron (developer)
2017-09-11 00:01

False positives sound more benign than false negatives. Nevertheless, the flambda part of the manual currently only warns about false negatives for this warning. I think it would help users to at least clarify this point in the manual, and add a reference inside the warning message towards this extended explanation. It is also a bit unfortunate that this warning cannot be disabled locally with warning attributes.
xclerc (reporter)
2017-09-11 18:30
edited on: 2017-09-11 18:35

Indeed. Moreover, the manual proves me wrong: there is a way to
silence the warning. The reporter may amend `filter_map` [1] as

let filter_map f a =
  let rec aux acc i =
    if i = Array.length a
    then (
      let a' = Array.of_list acc in
      reverse_in_place (Sys.opaque_identity a');
    ) else match f a.(i) with
      | None -> aux acc (i+1)
      | Some x -> aux (x::acc) (i+1)
  in aux [] 0

[1] [^]

lpw25 (developer)
2017-09-15 14:05

Haven't actually tested, but this PR should fix it: [^]
xclerc (reporter)
2017-09-15 14:48

Tested - the PR fixes the problem on this instance.
doligez (administrator)
2017-10-02 14:16

Fixed by GPR#1339

- Issue History
Date Modified Username Field Change
2017-08-31 13:45 freyr New Issue
2017-08-31 13:45 freyr File Added: test.tar.gz
2017-09-01 16:01 octachron Note Added: 0018212
2017-09-01 16:03 octachron Status new => acknowledged
2017-09-04 12:35 xclerc Note Added: 0018213
2017-09-11 00:01 octachron Note Added: 0018235
2017-09-11 18:30 xclerc Note Added: 0018236
2017-09-11 18:35 xclerc Note Edited: 0018236 View Revisions
2017-09-11 18:35 xclerc Note Edited: 0018236 View Revisions
2017-09-15 14:05 lpw25 Note Added: 0018261
2017-09-15 14:48 xclerc Note Added: 0018262
2017-10-02 14:16 doligez Note Added: 0018449
2017-10-02 14:16 doligez Status acknowledged => resolved
2017-10-02 14:16 doligez Resolution open => fixed

Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker