Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Integer literals #3

Closed
rsaill opened this issue Sep 1, 2022 · 3 comments
Closed

Integer literals #3

rsaill opened this issue Sep 1, 2022 · 3 comments

Comments

@rsaill
Copy link

rsaill commented Sep 1, 2022

When parsing and printing the following program

const unsigned long x = 18446744073709551615UL;

I get

unsigned long const x = -1;

While this is correct (I think) due to implicit conversion, I was expecting the original literal to be printed back.
After a quick look at the code, I think that there is a missing optional argument ?signed somewhere in the printer.

Also, I tried to implement a suffix_of_integer_literal in the same spirit as radix_of_integer_literal to extract, for instance, the 'UL' suffix above.
However, I remarked that it does not work when the literal comes from a macro expansion and the macro is defined in another file.
In this case, your radix_of_integer_literal returns None.
Do you think that there is another way to get the radix and the suffix in the particular case?

@thierry-martinez
Copy link
Owner

Thank you very much for reporting this! These bugs should be fixed in #4: you may checkout the branch https://github.com/thierry-martinez/clangml/tree/fix.3.integer_literal and run ./bootstrap.sh before ./configure to test it.

The printer should now print integer literals with the expected suffix. Moreover, radix_of_integer_literal now works even if the literal comes from a macro. Here is the new code of radix_of_integer_literal if you want to adapt it for suffix_of_integer_literal:

  let radix_of_integer_literal (expr : t) : radix option =
    let cursor = Ast.cursor_of_node expr in
    let start = get_range_start (get_cursor_extent cursor) in
    let tu = cursor_get_translation_unit cursor in
    let tokens = tokenize tu (get_range start start) in
    if Array.length tokens >= 1 then
      Some (radix_of_string (get_token_spelling tu tokens.(0)))
    else
      None

thierry-martinez added a commit to thierry-martinez/opam-repository that referenced this issue Sep 5, 2022
This commit pushes a new release of clangml.4.8.0, compatible with
the upcoming LLVM/Clang 15. Changes are:

- Support for Clang/LLVM 15.0.0

- `Ast.character_kind` and `Ast.string_kind` are now distinct
  types (aliases for `Clang.clang_ext_characterkind` and
  `Clang.clang_ext_stringkind` respectively.
  The constructor `Ordinary` replaced the former constructor `Ascii`
  for `string_kind`, to match the new convention used by
  `clang::StringLiteral::StringKind` from Clang 15.0.0.
  The constructor `Ascii` for `character_kind` is left unchanged.

- #1, #2: `Ast.Var` and `Ast.Function` constructors now have a `storage`
  field in addition to the computed `linkage`, exposing the value
  previously accessible via `cursor_get_storage_class`. The
  storage classes are now correctly printed by the printer.
  (reported by Ronan, rsaill/n47, thierry-martinez/clangml#1)

- #3, ocaml#4: fix `Clang.Expr.radix_of_integer_literal` when the literal
  comes from a macro expansion, and fix printing of unsigned/long
  integer literals with `Clang.Printer`.
  (reported by Ronan, rsaill/n47, thierry-martinez/clangml#3)
@rsaill
Copy link
Author

rsaill commented Sep 5, 2022

Fantastic! I can confirm that it works as expected now.

@thierry-martinez
Copy link
Owner

This issue (and the previous ones) are fixed in clangml release 4.8.0, which should appear in opam soon. Thank you for your contributions to this release!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants