Skip to content

Parse arbitrary precision integers ..#170

Closed
hhugo wants to merge 3 commits intoocaml:trunkfrom
hhugo:lex1
Closed

Parse arbitrary precision integers ..#170
hhugo wants to merge 3 commits intoocaml:trunkfrom
hhugo:lex1

Conversation

@hhugo
Copy link
Contributor

@hhugo hhugo commented Apr 19, 2015

.. and allow any char of [A-Za-z] as modifier for ints (previously 'l','L','n') and floats.
This give more freedom to ppx rewritters (what about a ppx for zarith or literal for complex)

Checks are performed when translating from Parsetree to Typedtree.
Unknow_literal is raised if the modifier is not recognized ([lLn]?)
Integer_overflow is raised as before.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may fail

@Octachron
Copy link
Member

This patch looks really interesting. Would it not make sense to have the same kind of support for float litteral? I can see some applications for qualified float litterals: complex number, single or quadruple precision litteral for instance. Most of these applications are not general enough to be supported within the compiler but would make nice ppx extensions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was an error here because the argument i of the function was shadowed by the pattern matching,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

15 years old typo !!

@hhugo
Copy link
Contributor Author

hhugo commented Apr 21, 2015

@Octachron, I've updated the PR to add modifier on floats.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The minus sign is kept inside the string. So there is a lot of string processing for this first character. Is it not easier to remove at lexing the minus sign from the string and keep the information in a separate bool?

@samoht
Copy link
Member

samoht commented Apr 21, 2015

It could maybe be helpful if you add a few tests exercising the new code?

@Drup
Copy link
Contributor

Drup commented May 7, 2015

I like the patch. I read the implementation and I think it's good.

I agree with @bobot on the minus sign.

I think there should be a function from Parsetree.constant to Asttypes.constant that do not need the typing env. It would be very useful for ppx writer. Returning an option would work well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would really benefit from using ocamllex's named sub-strings:

| (int_literal as lit) (literal_modifier as modif) { INT (lit, Some modif) }

@hhugo
Copy link
Contributor Author

hhugo commented May 11, 2015

I would prefer to keep the whole literal (minus sign included) inside the string. In most cases, it does not add any string processing as we already parse every int literal as negative numbers. (prepending "-" if not already negative, https://github.com/ocaml/ocaml/pull/170/files#diff-4408a3671b05f01d8b0dbe9df336e08bL173)

@hhugo
Copy link
Contributor Author

hhugo commented May 13, 2015

PR ready for camlp4 camlp4/camlp4#91

utils/misc.ml Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a comment for why you need this complicated function and a link to the related bts bug?

@gasche
Copy link
Member

gasche commented Jul 26, 2015

This one looks ready to merge. What is the status of camlp4/camlp4#90 ?

@hhugo
Copy link
Contributor Author

hhugo commented Jul 27, 2015

Rebased on top of trunk.
Ready to merge unless one wants to add tests as proposed by @samoht

@gasche
Copy link
Member

gasche commented Jul 27, 2015

Tests are always good. I'd love tests!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that the format here is apparently meant to give re-parseability, you should reprint PConst rather than Const.

@gasche
Copy link
Member

gasche commented Jul 27, 2015

If you write tests, it would be nice to include one checking that -1073741824 is still correctly parsed as an integer literal (This corresponds to min_int for 32 machines.), to avoid the issue described by this comment in MPR#4210.

@gasche
Copy link
Member

gasche commented Jul 27, 2015

I have one worry with this patch: there is an ambiguity (resolved by the longest-lexeme rule) between literal modifiers and hexadecimal literals. For example, 32f would be parsed as a literal modifier, but 0x32f is a valid hexadecimal literal.

Wouldn't it be safer (for now) to either restrict literal modifiers to the non-hexadecimal cases (but this wouldn't be consistent with nlL which are valid for hexadecimal literals), or to letters strictly after f or F?

@hhugo
Copy link
Contributor Author

hhugo commented Jul 27, 2015

How safer would that be ?
It seams to be similar to 1.2e and 1.2e3

@dbuenzli
Copy link
Contributor

.. and allow any char of [A-Za-z] as modifier for ints

This may break existing programs, right ?

@gasche
Copy link
Member

gasche commented Jul 27, 2015

Not directly, in the sense that programs that relied on 123foo to parse were made illegal by #155 . That said, #155 has not yet been part of a release, so both changes together will break some programs, yes.

@bobzhang
Copy link
Member

bobzhang commented Aug 3, 2015

So this patch is divided into two parts:

  1. make parsetree using string literals (more loyal to user input)

  2. extending lexer to support arbitrary modifiers

    With regard to 1, can we make changes to Asttypes.constant directly instead of creating a new type, Asttypes.constant is also used by Lambda, I am currently working on a project compiling lambda to readable javascript in the module level (still in the early days http://bobzhang.github.io/js-demo/), we could benefit such changes as well.

@hhugo
Copy link
Contributor Author

hhugo commented Oct 27, 2015

I've just rebased this PR onto trunk. Are people happy the current state ?

@bobzhang, you proposal of updating Asttypes.constant and not introducing a new type does not really make sense to me. One might still want to propagate constant as string to lambda but I would not do it inside this PR.

@gasche
Copy link
Member

gasche commented Nov 15, 2015

This looks good to merge, but I'm still mildly non-plussed by the interaction between modifiers in the a-f range and hexa literals. I assigned @damiendoligez so that he takes care of merging.

@hhugo
Copy link
Contributor Author

hhugo commented Nov 24, 2015

@gasche, thinking more about it, I now agree with your concern.
How do you feel about restricting the range for modifiers to ['g'-'z''G'-'Z'] ?

@gasche
Copy link
Member

gasche commented Nov 25, 2015

I think that this would be a good, safe thing to do -- we can always extend the feature later with more experience.

@damiendoligez
Copy link
Member

Merged with [g-zG-Z].

@alainfrisch
Copy link
Contributor

The new test min_int.ml cannot be compiled on 32-bit architectures. I've disabled it for now.

If someone insists on keeping it, please tweak the test suite to exclude this test (or create the 32-bit counterpart).

@alainfrisch alainfrisch reopened this Dec 4, 2015
@gasche
Copy link
Member

gasche commented Dec 4, 2015

We should have a warning on integer literals that do not fit on 32 machines, in the same spirit as the marshalling flag.

@alainfrisch
Copy link
Contributor

Note that there is already "ocamlc -compat-32", which triggers an error when the bytecode is not compatible with 32-bit.

@damiendoligez
Copy link
Member

I've "fixed" the test. Instead of adding heavy shell plumbing to compile a different source depending on the platform, I just changed it to the 32-bit min_int. This means the test is mostly useless on 64-bit, but the 32-bit platforms will test the corner case.

@hhugo hhugo deleted the lex1 branch December 22, 2015 10:16
kayceesrk added a commit to kayceesrk/ocaml that referenced this pull request Apr 9, 2018
@bobzhang
Copy link
Member

bobzhang commented Nov 3, 2018

curious do we have any ppx make use of such extension

@Drup
Copy link
Contributor

Drup commented Nov 3, 2018

@bobzhang ocaml/Zarith#4

mshinwell pushed a commit to mshinwell/ocaml that referenced this pull request Jun 17, 2020
lthls added a commit to lthls/ocaml that referenced this pull request Sep 23, 2020
lthls added a commit to lthls/ocaml that referenced this pull request Sep 23, 2020
lthls added a commit to lthls/ocaml that referenced this pull request Sep 24, 2020
chambart pushed a commit to chambart/ocaml-1 that referenced this pull request Sep 9, 2021
* Remove hack for .depend in runtime/dune

Port and extend flambda-backend/ocaml#460

* Remove reference to runtime/.depend from runtime/dune

runtime/.depend removed by ocaml#9332
stedolan pushed a commit to stedolan/ocaml that referenced this pull request Dec 13, 2021
* Remove hack for .depend in runtime/dune

Port and extend flambda-backend/ocaml#460

* Remove reference to runtime/.depend from runtime/dune

runtime/.depend removed by ocaml#9332
chambart pushed a commit to chambart/ocaml-1 that referenced this pull request Feb 1, 2022
23a7f73 flambda-backend: Fix some Debuginfo.t scopes in the frontend (ocaml#248)
33a04a6 flambda-backend: Attempt to shrink the heap before calling the assembler (ocaml#429)
8a36a16 flambda-backend: Fix to allow stage 2 builds in Flambda 2 -Oclassic mode (ocaml#442)
d828db6 flambda-backend: Rename -no-extensions flag to -disable-all-extensions (ocaml#425)
68c39d5 flambda-backend: Fix mistake with extension records (ocaml#423)
423f312 flambda-backend: Refactor -extension and -standard flags (ocaml#398)
585e023 flambda-backend: Improved simplification of array operations (ocaml#384)
faec6b1 flambda-backend: Typos (ocaml#407)
8914940 flambda-backend: Ensure allocations are initialised, even dead ones (ocaml#405)
6b58001 flambda-backend: Move compiler flag -dcfg out of ocaml/ subdirectory (ocaml#400)
4fd57cf flambda-backend: Use ghost loc for extension to avoid expressions with overlapping locations (ocaml#399)
8d993c5 flambda-backend: Let's fix instead of reverting flambda_backend_args (ocaml#396)
d29b133 flambda-backend: Revert "Move flambda-backend specific flags out of ocaml/ subdirectory (ocaml#382)" (ocaml#395)
d0cda93 flambda-backend: Revert ocaml#373 (ocaml#393)
1c6eee1 flambda-backend: Fix "make check_all_arches" in ocaml/ subdirectory (ocaml#388)
a7960dd flambda-backend: Move flambda-backend specific flags out of ocaml/ subdirectory (ocaml#382)
bf7b1a8 flambda-backend: List and Array Comprehensions (ocaml#147)
f2547de flambda-backend: Compile more stdlib files with -O3 (ocaml#380)
3620c58 flambda-backend: Four small inliner fixes (ocaml#379)
2d165d2 flambda-backend: Regenerate ocaml/configure
3838b56 flambda-backend: Bump Menhir to version 20210419 (ocaml#362)
43c14d6 flambda-backend: Re-enable -flambda2-join-points (ocaml#374)
5cd2520 flambda-backend: Disable inlining of recursive functions by default (ocaml#372)
e98b277 flambda-backend: Import ocaml#10736 (stack limit increases) (ocaml#373)
82c8086 flambda-backend: Use hooks for type tree and parse tree (ocaml#363)
33bbc93 flambda-backend: Fix parsecmm.mly in ocaml subdirectory (ocaml#357)
9650034 flambda-backend: Right-to-left evaluation of arguments of String.get and friends (ocaml#354)
f7d3775 flambda-backend: Revert "Magic numbers" (ocaml#360)
0bd2fa6 flambda-backend: Add [@inline ready] attribute and remove [@inline hint] (not [@inlined hint]) (ocaml#351)
cee74af flambda-backend: Ensure that functions are evaluated after their arguments (ocaml#353)
954be59 flambda-backend: Bootstrap
dd5c299 flambda-backend: Change prefix of all magic numbers to avoid clashes with upstream.
c2b1355 flambda-backend: Fix wrong shift generation in Cmm_helpers (ocaml#347)
739243b flambda-backend: Add flambda_oclassic attribute (ocaml#348)
dc9b7fd flambda-backend: Only speculate during inlining if argument types have useful information (ocaml#343)
aa190ec flambda-backend: Backport fix from PR#10719 (ocaml#342)
c53a574 flambda-backend: Reduce max inlining depths at -O2 and -O3 (ocaml#334)
a2493dc flambda-backend: Tweak error messages in Compenv.
1c7b580 flambda-backend: Change Name_abstraction to use a parameterized type (ocaml#326)
07e0918 flambda-backend: Save cfg to file (ocaml#257)
9427a8d flambda-backend: Make inlining parameters more aggressive (ocaml#332)
fe0610f flambda-backend: Do not cache young_limit in a processor register (upstream PR 9876) (ocaml#315)
56f28b8 flambda-backend: Fix an overflow bug in major GC work computation (ocaml#310)
8e43a49 flambda-backend: Cmm invariants (port upstream PR 1400) (ocaml#258)
e901f16 flambda-backend: Add attributes effects and coeffects (#18)
aaa1cdb flambda-backend: Expose Flambda 2 flags via OCAMLPARAM (ocaml#304)
62db54f flambda-backend: Fix freshening substitutions
57231d2 flambda-backend: Evaluate signature substitutions lazily (upstream PR 10599) (ocaml#280)
a1a07de flambda-backend: Keep Sys.opaque_identity in Cmm and Mach (port upstream PR 9412) (ocaml#238)
faaf149 flambda-backend: Rename Un_cps -> To_cmm (ocaml#261)
ecb0201 flambda-backend: Add "-dcfg" flag to ocamlopt (ocaml#254)
32ec58a flambda-backend: Bypass Simplify (ocaml#162)
bd4ce4a flambda-backend: Revert "Semaphore without probes: dummy notes (ocaml#142)" (ocaml#242)
c98530f flambda-backend: Semaphore without probes: dummy notes (ocaml#142)
c9b6a04 flambda-backend: Remove hack for .depend from runtime/dune  (ocaml#170)
6e5d4cf flambda-backend: Build and install Semaphore (ocaml#183)
924eb60 flambda-backend: Special constructor for %sys_argv primitive (ocaml#166)
2ac6334 flambda-backend: Build ocamldoc (ocaml#157)
c6f7267 flambda-backend: Add -mbranches-within-32B to major_gc.c compilation (where supported)
a99fdee flambda-backend: Merge pull request ocaml#10195 from stedolan/mark-prefetching
bd72dcb flambda-backend: Prefetching optimisations for sweeping (ocaml#9934)
27fed7e flambda-backend: Add missing index param for Obj.field (ocaml#145)
cd48b2f flambda-backend: Fix camlinternalOO at -O3 with Flambda 2 (ocaml#132)
9d85430 flambda-backend: Fix testsuite execution (ocaml#125)
ac964ca flambda-backend: Comment out `[@inlined]` annotation. (ocaml#136)
ad4afce flambda-backend: Fix magic numbers (test suite) (ocaml#135)
9b033c7 flambda-backend: Disable the comparison of bytecode programs (`ocamltest`) (ocaml#128)
e650abd flambda-backend: Import flambda2 changes (`Asmpackager`) (ocaml#127)
14dcc38 flambda-backend: Fix error with Record_unboxed (bug in block kind patch) (ocaml#119)
2d35761 flambda-backend: Resurrect [@inline never] annotations in camlinternalMod (ocaml#121)
f5985ad flambda-backend: Magic numbers for cmx and cmxa files (ocaml#118)
0e8b9f0 flambda-backend: Extend conditions to include flambda2 (ocaml#115)
99870c8 flambda-backend: Fix Translobj assertions for Flambda 2 (ocaml#112)
5106317 flambda-backend: Minor fix for "lazy" compilation in Matching with Flambda 2 (ocaml#110)
dba922b flambda-backend: Oclassic/O2/O3 etc (ocaml#104)
f88af3e flambda-backend: Wire in the remaining Flambda 2 flags (ocaml#103)
678d647 flambda-backend: Wire in the Flambda 2 inlining flags (ocaml#100)
1a8febb flambda-backend: Formatting of help text for some Flambda 2 options (ocaml#101)
9ae1c7a flambda-backend: First set of command-line flags for Flambda 2 (ocaml#98)
bc0bc5e flambda-backend: Add config variables flambda_backend, flambda2 and probes (ocaml#99)
efb8304 flambda-backend: Build our own ocamlobjinfo from tools/objinfo/ at the root (ocaml#95)
d2cfaca flambda-backend: Add mutability annotations to Pfield etc. (ocaml#88)
5532555 flambda-backend: Lambda block kinds (ocaml#86)
0c597ba flambda-backend: Revert VERSION, etc. back to 4.12.0 (mostly reverts 822d0a0 from upstream 4.12) (ocaml#93)
037c3d0 flambda-backend: Float blocks
7a9d190 flambda-backend: Allow --enable-middle-end=flambda2 etc (ocaml#89)
9057474 flambda-backend: Root scanning fixes for Flambda 2 (ocaml#87)
08e02a3 flambda-backend: Ensure that Lifthenelse has a boolean-valued condition (ocaml#63)
77214b7 flambda-backend: Obj changes for Flambda 2 (ocaml#71)
ecfdd72 flambda-backend: Cherry-pick 9432cfdadb043a191b414a2caece3e4f9bbc68b7 (ocaml#84)
d1a4396 flambda-backend: Add a `returns` field to `Cmm.Cextcall` (ocaml#74)
575dff5 flambda-backend: CMM traps (ocaml#72)
8a87272 flambda-backend: Remove Obj.set_tag and Obj.truncate (ocaml#73)
d9017ae flambda-backend: Merge pull request ocaml#80 from mshinwell/fb-backport-pr10205
3a4824e flambda-backend: Backport PR#10205 from upstream: Avoid overwriting closures while initialising recursive modules
f31890e flambda-backend: Install missing headers of ocaml/runtime/caml (ocaml#77)
83516f8 flambda-backend: Apply node created for probe should not be annotated as tailcall (ocaml#76)
bc430cb flambda-backend: Add Clflags.is_flambda2 (ocaml#62)
ed87247 flambda-backend: Preallocation of blocks in Translmod for value let rec w/ flambda2 (ocaml#59)
a4b04d5 flambda-backend: inline never on Gc.create_alarm (ocaml#56)
cef0bb6 flambda-backend: Config.flambda2 (ocaml#58)
ff0e4f7 flambda-backend: Pun labelled arguments with type constraint in function applications (ocaml#53)
d72c5fb flambda-backend: Remove Cmm.memory_chunk.Double_u (ocaml#42)
9d34d99 flambda-backend: Install missing artifacts
10146f2 flambda-backend: Add ocamlcfg (ocaml#34)
819d38a flambda-backend: Use OC_CFLAGS, OC_CPPFLAGS, and SHAREDLIB_CFLAGS for foreign libs (#30)
f98b564 flambda-backend: Pass -function-sections iff supported. (#29)
e0eef5e flambda-backend: Bootstrap (#11 part 2)
17374b4 flambda-backend: Add [@@Builtin] attribute to Primitives (#11 part 1)
85127ad flambda-backend: Add builtin, effects and coeffects fields to Cextcall (#12)
b670bcf flambda-backend: Replace tuple with record in Cextcall (#10)
db451b5 flambda-backend: Speedups in Asmlink (#8)
2fe489d flambda-backend: Cherry-pick upstream PR#10184 from upstream, dynlink invariant removal (rev 3dc3cd7 upstream)
d364bfa flambda-backend: Local patch against upstream: enable function sections in the Dune build
886b800 flambda-backend: Local patch against upstream: remove Raw_spacetime_lib (does not build with -m32)
1a7db7c flambda-backend: Local patch against upstream: make dune ignore ocamldoc/ directory
e411dd3 flambda-backend: Local patch against upstream: remove ocaml/testsuite/tests/tool-caml-tex/
1016d03 flambda-backend: Local patch against upstream: remove ocaml/dune-project and ocaml/ocaml-variants.opam
93785e3 flambda-backend: To upstream: export-dynamic for otherlibs/dynlink/ via the natdynlinkops files (still needs .gitignore + way of generating these files)
63db8c1 flambda-backend: To upstream: stop using -O3 in otherlibs/Makefile.otherlibs.common
eb2f1ed flambda-backend: To upstream: stop using -O3 for dynlink/
6682f8d flambda-backend: To upstream: use flambda_o3 attribute instead of -O3 in the Makefile for systhreads/
de197df flambda-backend: To upstream: renamed ocamltest_unix.xxx files for dune
bf3773d flambda-backend: To upstream: dune build fixes (depends on previous to-upstream patches)
6fbc80e flambda-backend: To upstream: refactor otherlibs/dynlink/, removing byte/ and native/
71a03ef flambda-backend: To upstream: fix to Ocaml_modifiers in ocamltest
686d6e3 flambda-backend: To upstream: fix dependency problem with Instruct
c311155 flambda-backend: To upstream: remove threadUnix
52e6e78 flambda-backend: To upstream: stabilise filenames used in backtraces: stdlib/, otherlibs/systhreads/, toplevel/toploop.ml
7d08e0e flambda-backend: To upstream: use flambda_o3 attribute in stdlib
403b82e flambda-backend: To upstream: flambda_o3 attribute support (includes bootstrap)
65032b1 flambda-backend: To upstream: use nolabels attribute instead of -nolabels for otherlibs/unix/
f533fad flambda-backend: To upstream: remove Compflags, add attributes, etc.
49fc1b5 flambda-backend: To upstream: Add attributes and bootstrap compiler
a4b9e0d flambda-backend: Already upstreamed: stdlib capitalisation patch
4c1c259 flambda-backend: ocaml#9748 from xclerc/share-ev_defname (cherry-pick 3e937fc)
00027c4 flambda-backend: permanent/default-to-best-fit (cherry-pick 64240fd)
2561dd9 flambda-backend: permanent/reraise-by-default (cherry-pick 50e9490)
c0aa4f4 flambda-backend: permanent/gc-tuning (cherry-pick e9d6d2f)

git-subtree-dir: ocaml
git-subtree-split: 23a7f73
EmileTrotignon pushed a commit to EmileTrotignon/ocaml that referenced this pull request Jan 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.