Skip to content

Reorder comments, function attributes, and macro def args #251

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

Merged
merged 28 commits into from
Dec 9, 2022

Conversation

elcritch
Copy link
Contributor

@elcritch elcritch commented Dec 8, 2022

Some changes I've wanted for a while. Still a bit of a WIP.

  • reorder comments to convert C-style / Doxygen prefixed comments to Nim-style postfixed comments
    • currently only tested on proc defs, and struct bodies
  • add in __attribute__ to list of declarations for functions to help with parsing function attributes
  • add --def arg to allow defining a macro from the command line
  • add --stdints pre-defined mangle to convert C stdint to Nim ints

Example usage:

c2nim \
  --strict --header \
  --stdints # gives: --mangle:'{u?}int{\d+}_t=$1int$2' \
  --reordercomments \
  --def:RCL_PUBLIC='__attribute__ ()' \
  --def:RCL_WARN_UNUSED='__attribute__ ()' \
    testsuite/tests/rcl_arguments.h

Example re-order comments:

  /** * allocator objects.  */
  void * state;

becomes:

  void * state; ##\
      ## allocator objects. 

Parsing __attributes__ and --def: makes it posible to handle the GCC C function pragmas style. For example --def:RCL_WARN_UNUSED='__attribute__ ()' and --def:RCL_PUBLIC='__attribute__ ()' will make the below parse:

// function defined with macros attributes:
RCL_PUBLIC RCL_WARN_UNUSED int rcl_arguments_get_count_unparsed(const rcl_arguments_t * args);
// expands to:
__attribute__(public) __attribute__(...) int rcl_arguments_get_count_unparsed(const rcl_arguments_t * args);

That last change is responsible for a good portion of C files not parsing properly. Eventually it'd be nice to add pragmas to the Nim procs so people could define them, but that's a project for another day.

@elcritch elcritch changed the title WIP: Reorder comments, function attributes, and macro def args Reorder comments, function attributes, and macro def args Dec 9, 2022
@elcritch
Copy link
Contributor Author

elcritch commented Dec 9, 2022

Ok, I think this is ready. The new stuff has tests. Also added:

  • --render:extranewlines adds in extra newlines before proc defs. This plumbs
    in renderFlags to the CLI parser

@Araq Araq merged commit 41c8ef1 into nim-lang:master Dec 9, 2022
--def:SYM='macro()' define a C macro that gets replaced with the given
definition. It's parsed by the lexer. Use it to fix
function attributes: ``--def:PUBLIC='__attribute__ ()'``
--reordercomments reorder C comments to match Nim's postfix style

Choose a reason for hiding this comment

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

does this really need to be an option? ie why not always do this (since it's needed to make the comments compatible with nim doc)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Two reasons: 1) to make it easier to get accepted 2) because I’d have to reorder the comments in all the tests.

Though perhaps it could default to on and use the option to disable it. Plus there may be cases it’s wrong. I sorta hack the new lines with a heuristic.

@@ -52,6 +56,7 @@ Options:
for example `--mangle:'{u?}int{\d+}_t=$1int$2'` to
convert C <stdint.h> to Nim equivalents
(multiple --mangle options are supported)
--stdints Mangle C stdint's into Nim style int's

Choose a reason for hiding this comment

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

this would feel more natural as a --no-stdints - ie the quality of the wrapper is significantly improved when these types are recognised and it's hard to imagine code that uses these names for anything else than the standard interpretation

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.

4 participants