Build with -strict-sequence #927
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Is this available on all compiler versions? What are the benefits of using this option? |
|
This PR is motivated by a proposal to enable The -strict-sequence option has been there since ocaml 3.12.0 it seems, which should cover all versions supported by batteries. |
|
Thanks for the explanations. |
|
Ah; does the v3 branch correspond to a new version of batteries and will eventually get merged into master? Or has it been branched from master? I'll prepare a separate PR that applies the current patch to the v3 branch. |
|
I am not sure about @rixed 's plan for the v3 branch. In the discussion about ocaml/ocaml#1971 you If possible, I would prefer that we use this version. |
It might be simpler to merge this PR into master and then to merge master into v3 actually?
I can add type annotations if you want, yes.
I don't understand this distinction though. If ignore is needed then this necessarily means that we have a function that returns a value that we do not care about and that we want to throw away. |
|
I meant: ignoring a unit is usually very different than ignoring anything else. |
I was not aware of this use case. What is the point of writing All the instances of ignore in this PR are in the second case: something "strange" (throwing away a value) was happening implicitly because of |
I have no plan regarding v3. I think we should release the next 2.x version, then rebase v3 onto master, then fix all tests, then release v3. I can help with some of that if needed. |
UnixJunkie
added a commit
that referenced
this pull request
Jan 9, 2020
* Changelog: add review information for #768 * remove exceptions in BatVect.{exists,for_all} * remove exceptions in BatVect.find Implementing BatVect.find_opt makes this more efficient (we have at most one allocation for the result, instead of using exception handlers for each left subtree), but the function is not exposed publicly. * BatVect: expose find_opt and harmonize variable names in .mli section * remove exceptions in BatVect.Make.{for_all,exists} * remove exceptions in BatVect.Make.find * tests for BatVect.Make functions * expose BatVect.Make.find_opt * less exceptions in batVect: changelog entry * Makefile: introduce a QTEST_SEED variable to set qtest random seed To reproduce a failure that may depend on the seed 1234, you may use make test QTEST_SEED=1345 * Fix some issues in the README * Fix compilation with OCaml 4.05.0 * Fixes based on @gasche's comments * Add support for new APIs * Update for compatibility with qcheck 0.6 Unfortunately this does break backwards compatibility with qcheck 0.5, so there is now an explicit opam dependency on qcheck >= 0.6. Fixes #756. * Simplify dependency restriction on qtest * batText: add tests and fix a bug in `rindex{,_from}` * batText: add tests for contains_from and fix documentation bug (exception name) * batText: add tests to rcontains_from and fix bug in passing * battext changes: ChangeLog entry * batNum: handle negatives in of_float_string * ChangeLog entry on BatNum.of_float_string * BatNum.of_float_string: extra testcase * Backport new 4.05 functions (#779) * Backport the new option-returning *_opt functions from 4.05.0 * Bigarray.Array0 cannot be backported from 4.05 (it depends on a runtime change) * batList: backport compare_length{s,_with} to <4.5 versions * Printexc.raise_with_backtrace cannot be backported from 4.05 This is a new primitive from the runtime. * Change entry for #777, #779 * batBytes: minor compatibility fix * release summary (#782) * add a "num" dependency to mirror upstream opam file (#783) See ocaml/opam-repository#9341 Note that, if/when "num" is split off the standard distribution, there will be the question of keeping it or not in Batteries. It could be come a separate package (batteries-num, depending on batteries) for example. * next release will be 2.7.0 * simplification to the opam file so that it is easier to compare with the one in the opam repository * added note explaining that BatX is also Batteries.X * substitued the for a * added explanation for case when Batteries is opened * added example illustrating the BatX -> Batteries.X mapping. For an example, I think it's better to use module that's not listed in the "Extensions to the Standard Library" section. That section has its own specialized message about opening Batteries. Using a module from this section as an illustration has a slight risk of suggesting to users that the BatX->Batteries.X point applies only to this section. * safe-string: make Lexing safe * safe-string: make Big_int safe * safe-string: make Buffer safe * safe-string: make Digest safe * safe-string: make Genlex safe * safe-string: make IO safe * safe-string: make Format safe * safe-string: make Int32 safe * safe-string: make Marshal safe * safe-string: make Pervasives safe * safe-string: make Printf safe * safe-string: make String safe (but String.Cap) * safe-string: adapt String.Cap * safe-string: make Unix safe * safe-string: make Base64 safe * safe-string: make BitSet safe * safe-string: make Substring safe * safe-string: make Text safe * _tags: actually enforce safe_string * safe-string: make BatBytes safe * -safe-string: Changelog * [minor] exclude compattest.ml from qtest (Otherwise you cannot run the tests for an OCaml version that does not pass the compat-test.) * add 4.06-only Array.Floatarray primitive module * add 4.06-only Uchar.t functions in Buffer It would be nicer to make sure that BatUChar is a superset of the stdlib Uchar module, and that the two types are compatible, but that requires more work. * 4.06 Bigarray functions * 4.06 Unix functions * 4.06: Changes entry * fix more FloatArray failures in the test targets * added BatString.count_char * added count_string * safe-string: fix compatibility with OCaml <=4.02 The safe-string compatibility fixes rely on the Bytes package, which we convincingly mock under OCaml<=4.02, but also on bytes-adapted functions or just convenience functions that are useful for code porting: Buffer.add_subbytes, String.init, etc. Before this commit, the codebase would simply not build on OCaml<=4.02 because of this. What the commit does is to introduce a new module, BatBytesCompat, meant to contain backward-compatibility implementation of those functions. * a prebaked patch to try our inline tests on older OCaml versions The following command should get Batteries (temporarily) testable with old qtest versions: git apply build/make-our-inline-tests-compatible-with-older-OCaml-versions.patch * mention count_string * Revert "added count_string" This reverts commit 1171462. The idea is to do the next minor release without the count_* functions that are not yet ready for inclusion. The commit should be merged back right after the release. * missing @SInCE tags * update scripts/replace_since.sh * refresh @SInCE NEXT_RELEASE tags with new version * bump _oasis version number * Update setup.ml based on _oasis * Revert "mention count_string" This reverts commit 0ae9a35. This has to be pushed again after v2.8.0, in the right Changelog section * batString: safe-string-ready map_first_char * first commit after v2.8.0 * Revert "Revert "added count_string"" This reverts commit 10e2dd1. * Revert "Revert "mention count_string"" This reverts commit bc10ba1. * move count_string's ChangeLog entry into the new section * BatInt.Safe_int.mul performance improvements The fast-path was made less expensive, by getting rid of absolute value computation (which incurs branching). The downside is that the fast-path supports only non-negative integers from now on. * Add ChangeLog entry * disable print_string_cap_ro/rw to match #673 * Fix: incorrect behaviour of Map.union In case of binding conflicts, [Map.union m1 m2] should prefer the value from [m2], as stated in documentation. * Add a test case for Map.union * Add ChangeLog entry * added BatSeq.{to_buffer|to_string|of_string} * Cosmetic tweaks in BatMap.mli * Document that Map.extract can raise Not_found. (#825) * added BatArray.split * updated ChangeLog * Clarified documentation for `next` in LazyList (#810) * Revised docs for seq, unfold, from_loop in LazyList (#806) seq: Revised basic description to fit first part of from_loop doc, which seems clearer. Also added note that you can create an infinite list by passing a function that constantly returns true. from_loop: Gave additional detail about what the next function is supposed to do, and noted that its result pair might contain different types. Added a simple example where the types are the same. (This example could be done more simpy using seq. I still don't understand the purpose of from_loop.) I moved seq before from_loop because it's a more basic version of this kind of operation. from_loop is more specialized, and complicated, but its name might suggest that it's the normal way of making a lazy list by repeated application of a function. Users should see seq first before trying to work out what from_loop is for. * improve MultiPMap documentation (#827) * simpler (and faster?) implementation of BatArray.partition (#829) * simpler and faster implementation of BatArray.partition (at the cost of using more memory) * updated changelog * removed previously added exception types * updated ocamldoc for BatSeq.of_string * Compile Batteries with -no-alias-deps The size cost of using batteries has always been a problem. OCaml 4.02 introduced --no-deps-libs for this exact purpose (see http://caml.inria.fr/pub/docs/manual-ocaml/extn.html#s%3Amodule-alias). I couldn't find any discussion relative to this but I'm well aware this is unlikely to not have been considered already. Simple test: ``` % cat toto1.ml open Batteries let () = Printf.printf "hello\n" % cat toto2.ml let () = BatPrintf.printf "hello\n" % for f in toto1 toto2 ; do \ ocamlfind ocamlopt $f.ml -o $f.opt -package batteries -linkpkg ; done % ls -l toto*opt ``` Before the patch: ``` -rwxr-xr-x 1 rixed 5272428 Feb 7 05:43 toto1.opt* -rwxr-xr-x 1 rixed 2006588 Feb 7 05:43 toto2.opt* ``` After: ``` -rwxr-xr-x 1 rixed 3098452 Feb 7 05:38 toto1.opt* -rwxr-xr-x 1 rixed 2006588 Feb 7 05:38 toto2.opt* ``` That's a 40% reduction in size of a program using only BatPrintf from Batteries, still bigger than a program using explicitly only BatPrintf because of batPervasives, I assume. There are several ways i can imagine this patch would break other programs: - it might break the build of some programs relying on some of their direct dependencies to link in others of their direct dependencies, which I would qualify as a bug in those builds; - this could prevent the dynamic linkage of some plugins if it expects the whole of batteries to be available from the main program; - programs relying on any side effects of some otherwise unused batteries module initialization code, but that sounds very unlikely. Note that I'm not familiar with this option so there might be others, more obvious issues. I'm confident you will know :-) Note: I have no idea if/how the "pack" option that seems to be present in the setup.ml could be used for the same purpose. * Fix: Map.update was broken Searching for the node to update was broken, as the keys were compared in wrong order. * Fix: Map.update did not throw an exception on failure The docstring says "@raise Not_found if [k1] is not bound in [m]", but there was no raise if [k1] and [k2] were different. * Fix: Set.update did not throw an exception on failure The docstring says "@raise Not_found if [x] is not in [s]", but there was no raise if [x] and [y] were different. * Fix typo * Simplify Set.cartesian_product * Remove a redundant exception from BatOption.ml * Simplify Text.concat * Fix typos (#838) * Fix: Array.insert error message improvement The function now throws a more relevant message on invalid indices instead of the generic [invalid_arg "index out of bounds]. The assertion is now documented. * Implementation of Array.insert now uses unsafe_{get,set} * Do not depend upon bisect any longer Rational: bisect is not very actively maintained any more and code coverage have not been useful (or is not useful any more). * Corrected documentation for batList {last, hd} (#840) * Fix docstring of String.right (#849) * Fix docstring of String.right * BatInt.Safe_int.mul performance improvements The previous overflow test from CERT C is notable for avoiding an overflow while checking for overflow; hovewer, since overflows are perfectly defined in OCaml, a shorter test from Hacker's Delight is a better option. * Update ChangeLog * A clean-up of invalid_arg uses This patch makes uses of `invalid_arg` more uniform, fixing the following kinds of irregularities: * Lack of module name in a message (e.g. `Enum.arg_min` threw "arg_min"); * Lack of function name in a message (e.g. `List.reduce` threw "Empty list"); * Wrong module name in a message (e.g. `PathGen` functions threw messages with "Path"); * Wrong function name in a message (e.g. `List.fold_left2` threw a message with "fold_right2"); * Minor style issues: capitalization, punctuation, "Bat" prefixes; * Use of `raise (Invalid_argument "...")` instead of `invalid_arg "..."`. * Add ChangeLog entry * Restructuring of Heap.del_min Utility functions [find_min_tree] and [del_min_tree] are extended with an additional argument so as to make it more obvious to the reader that error messages are correct. * Optimization of List.unique_cmp A set should be more efficient than [bool PMap.t]. * Add ChangeLog entry * Fix docstrings of List.append and List.concat Stack usage estimates are invalid. * BatString.ml should use unsafe versions of set and get * batString: annotate unsafe_{get,set} with static reasoning in comments (partial) * BatString.ml: annotate unsafe_{get,set} with static assertions * Remove mention of IO.flush_out that does not exist. * List.mli: remove erroneous mentions of Different_list_size * List.ml: clean-up invalid_arg messages * Add ChangeLog entry * Add String.cut (#856) * Add `String.index_after_n : char -> int -> string -> int` * Add ChangeLog entry for `String.index_after_n` * fix index_after_n doc * Add String.cut * Update documentation for String.{index_after_n,cut} * Remove spurious empty line before BatString.index_after_n * Add Changelog entry for String.cut * Update #856 Changelog with PR number * Clarify documentation of String.{index_after_n,cut} * Rename String.cut to cut_on_char * better exception string in BatString.index_after_n * better ocamldoc for BatHashtbl.find_default (#859) * better ocamldoc for BatHashtbl.find_default so much better with an example * better ocamldoc for find_default * Rename cut to cut_on_char in Changelog * Fix build on OCaml < 4.05 * Fix a regression in BatInt.Safe_int.mul * Make doc of String.index_from a bit clearer (#864) * Map.equal used (=) instead of argument * Changelog entry for #865 * Adding ocamlfind plugin support (#867) * changelog update for PR 867 * Update batVect.mli Typo in description of sub. Ordering r m n required to be consistent with function definition * batInnerWeaktbl: separate interface from Hashtbl for compatibility * 4.07 compatibility fixes * testsuite: remove test_interface.ml, subsumed by 'make test-compat' * beef up Travis checking * prepare release 2.9.0 * Update setup.ml based on _oasis * prepare the 2.9.0 release * Update setup.ml based on _oasis * first commit after 2.9.0 * extend the Changelog (#877) * Fix Dllist.skip skip had the same behavior no matter the sign of the parameter * batNum: fix the testsuite * fix a batNum.of_float_sting bug on inputs in ]-1; 0[. (fixes #886) * added BatList.fold_while (#889) * added BatList.fold_while * Fixing basic typos (#890) * Gc: add new 4.08 fields * Array: reorder type and include to avoid error * re-export Bigarray.*.map_file functions removed in 4.08 * Int32: incorrect primitive names * Lexing: no support for disabling position-tracking * Unix: we don't support the new (link ?follow) parameter on older OCamls * add CI testing for 4.08 * Corrected BatDeque.is_empty documentation. * Hashtbl merge (#892) * Addition of Hashtbl.merge and merge_all Closes #891 * BatString.count_char uses a while loop instead of fold * Current module name seems Extlib, not Extlibcompat (#873) https://github.com/ocaml-batteries-team/batteries-included/blob/master/src/extlib.ml * Add lazylist equal predicate (#811) added LazyList.equal * updated ChangeLog * get rid of >=4.02 syntax in BatHashtbl.merge * BatHashtbl: avoid |> when possible * some more files need preprocessing Hashtbl.statistics is from 4.00.0 and was used in batInnerWeaktbl.ml{i} * BatArray: 4.07 compat * BatBuffer: 4.07 and 4.08 compat * BatBytes: 4.07 and 4.08 compat * BatInt32: 4.08 compat * BatInt64: 4.08 compat * BatLexing: 4.08 compat * BatList: 4.08 compat * BatNativeint: 4.08 compat * BatPrintexc: 4.08 compat * BatQueue: 4.07 and 4.08 compat Note to other maintainers: I am not sure take_opt and peek_opt should be introduced by preprocessing. Maybe we want them added to all OCaml versions? * BatString: 4.08 compat * BatSys: 4.08 compat * BatUnix: 4.08 compat * BatString: to_seq{i} and of_seq are from OCaml 4.07, not 4.08 also added NEXT_RELEASE tag for them * added missing NEXT_RELEASE tags in many places * next release will be 2.10.0 * Update setup.ml based on _oasis * updated opam file with bits from the one in opam-repository * repaired upload-docs target in Makefile and make it more readable too * start of next release * Fix compilation by BER MetaOCaml (#909) Set.Infix used to have operators `<.` and `>.` (for strict subset and strict superset), but `>.` is also used as a code block end-quote by BER. It's been decided to jettison the whole BatSet.Infix module, as a simple and future proof fix. Ref #908 * Add BatFilename (#910) Also added `Filename.split_extension` Closes #445 * s/RELASE/RELEASE (#911) typos * Improve toplevel printability of predefined sets (#912) Redefining the modules (instead of using those predefined in BatSet) was causing the printers for BatSet.Int and friends useless. Closes #750 * Remove BatOo (#915) As it is undocumented, unsupported and unsafe. Closes #848 * Make BatSeq.Exceptionless.combine really exceptionless (#917) Now, BatSeq.Exceptionless.combine returns the longest sequence possible (stopping when one of the input sequence ends). Closes #418 * BatString.split_on_char and nsplit must never return an empty list (#918) BatString.split_on_char behavior matches the one of Legacy.String.split_on_char, and that nsplit behavior matches the one of split_on_char. Closes #845 Closes #846 * Add Legacy.Result for OCaml >= 4.8.0 (#913) Now, users can pattern match Result.t returned from some library using Legacy.Result.Error while Batteries is opened. Ref #839 * List.sum [] is now 0 (#916) List.sum and List.fsum default to 0 (or 0.0) in case the list is empty. Previously, this was raising Invalid_argument. Closes #519 * Remove Legacy.Sort for OCaml >= 4.08 (#926) Closes #925 * Build with -strict-sequence (#927) * with_locked_file (#904) * high level file locking file interface * with_file_lock: now call user function with the opened file * Release v2.11.0 (#931) * Fix batFilename for various old OCaml versions split_extension was relying on remove_extension and extension functions that were introduced in OCaml 4.04.0. Also, an ocaml.deprecated annotation needed to be protected by some preprocessing. Tested with most of supported compiler versions. * Prepare Changelog and NEXT_RELEASE placeholder for v2.11.0 * Update setup.ml based on _oasis Closes #928 * make Unix.sleepf available under all OCaml versions (#930) * make Unix.sleepf available across all OCaml versions * the '|>' operator was not available in old versions of the stdlib (e.g. OCaml 4.0.0) * BatBytes was missing unsafe_blit_string's signature to compile on 4.09.0 * provide signatures of new functions from 4.09.0 in BatPrintexc * BatSys: change in argv declaration for ocaml >= 4.09.0 * updated opam file to reflect more the one from opam-repository * bits for next release Co-authored-by: Gabriel Scherer <gabriel.scherer@gmail.com> Co-authored-by: Tej Chajed <tchajed@mit.edu> Co-authored-by: Anton Yabchinskiy <arn@bestmx.net> Co-authored-by: Max Mouratov <mmouratov@gmail.com> Co-authored-by: Milo <iitalics@gmail.com> Co-authored-by: Thibault Suzanne <thi.suzanne@gmail.com> Co-authored-by: Marshall Abrams <mars0i@users.noreply.github.com> Co-authored-by: sapristi <megabigfoutoir@gmail.com> Co-authored-by: FkHina <36575050+FkHina@users.noreply.github.com> Co-authored-by: Ralf Vogler <ralf.vogler@gmail.com> Co-authored-by: Arlen Cox <arlencox@gmail.com> Co-authored-by: Francois Berenger <unixjunkie@sdf.org> Co-authored-by: yaseersheriff <yaseer.sheriff@gmail.com> Co-authored-by: Virgile Robles <virgile.robles@protonmail.ch> Co-authored-by: Fourchaux <jprodi04@gmail.com> Co-authored-by: Zachary Palmer <zepalmer@users.noreply.github.com> Co-authored-by: Catalin Hritcu <catalin.hritcu@gmail.com> Co-authored-by: Armaël Guéneau <Armael@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR enables the
-strict-sequenceflag when building batteries. A few fixes were needed.BatIOin anIncubatormodule, I opted to restrict the type in the interface;ignoreso as to preserve the library interfaces.