Add option for `mangling_separator` in ExportConfig by adamski · Pull Request #502 · mozilla/cbindgen

adamski · 2020-04-01T14:27:53Z

Addresses #437

I'm not sure if mangling_separator should live in ExportConfig or elsewhere.
Also wondering if there is a neater way to pass mangling_separator down the call chain.

Tested with a cpp_compat config, without the new option and with.

Not able to run tests yet due to #499

emilio

Thanks! This looks good, mostly some nits. Try rebasing on master? The issue with the tempdir should be fixed.

emilio · 2020-04-04T19:29:39Z

src/bindgen/ir/enumeration.rs

    }

-    fn specialize(&self, generic_values: &[Type], mappings: &[(&Path, &Type)]) -> Self {
+    fn specialize(&self, generic_values: &[Type], mappings: &[(&Path, &Type)], mangling_separator: &Option<String>) -> Self {


Passing an Option<&str> is a bit more idiomatic, is it much more difficult?

Alternatively if passing the whole &Config down here is sensible I think that'd be slightly preferable, but no big deal.

emilio · 2020-04-04T19:30:47Z

src/bindgen/mangle.rs


    let mut mangled = name.to_owned();
+    let separator = match mangle_separator {
+        Some(s) => s.to_owned(),


If this was an Option<&str>, then you don't need to heap-allocate here.

You don't need here anyway though, this could be:

match mangle_separator { Some(s) => &**s, None => "_", }

emilio · 2020-04-04T19:32:26Z

src/bindgen/mangle.rs

    for (i, ty) in generic_values.iter().enumerate() {
        if i != 0 {
-            mangled.push_str("__"); // ,
+            mangled.push_str(&concat_separators(&separator, 2)); // ,


Having a temporary string here is a bit unfortunate, just:

for _ in 0..2 { mangled.push_str(separator); }

Or alternatively, something like:

fn extend_n(mangled: &mut String, separator: &str, n: usize) { // ... }

adamski · 2020-04-06T09:30:12Z

@eqrion Thanks for the review. I passed Config instead, seems to make more sense also to be able to address #503
I still can't test due to #505

src/bindgen/mangle.rs

emilio

Thanks, this looks good. This needs to get rustfmt'd to pass CI, and also, can we get a test for this? Let me know if you need help adding one.

adamski · 2020-08-18T15:25:00Z

Thanks, this looks good. This needs to get rustfmt'd to pass CI, and also, can we get a test for this? Let me know if you need help adding one.

I'll run rustfmt and push. I already added a couple of tests for it. Please see mangle.rs

emilio

I meant more on the line of integration tests. Those are usually preferred, but fair enough :)

emilio · 2020-08-18T19:45:19Z

https://travis-ci.org/github/eqrion/cbindgen/builds/719002633 looks green :)

2153: Bump cbindgen from 0.14.3 to 0.14.4 r=tomtau a=dependabot-preview[bot] Bumps [cbindgen](https://github.com/eqrion/cbindgen) from 0.14.3 to 0.14.4. <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/eqrion/cbindgen/blob/master/CHANGES">cbindgen's changelog</a>.</em></p> <blockquote> <h2>0.14.4</h2> <pre><code> * Allow to override the mangling separator ([#502](mozilla/cbindgen#502)) <ul> <li> <p>cbindgen now handles better having ZSTs in template parameters, and default template parameters (<a href="https://github-redirect.dependabot.com/eqrion/cbindgen/issues/563">#563</a>).</p> </li> <li> <p>Support for annotating nonnull pointers (<a href="https://github-redirect.dependabot.com/eqrion/cbindgen/issues/558">#558</a>)</p> </li> <li> <p>Fixed bitflags that overflow a signed integer (<a href="https://github-redirect.dependabot.com/eqrion/cbindgen/issues/556">#556</a>)</p> </li> <li> <p>Support for wildcard argument names (<a href="https://github-redirect.dependabot.com/eqrion/cbindgen/issues/550">#550</a>)</p> </li> <li> <p>Support for the never return type, with configurable annotation (<a href="https://github-redirect.dependabot.com/eqrion/cbindgen/issues/549">#549</a>)</p> </li> <li> <p>Properly reject arrays as function arguments (<a href="https://github-redirect.dependabot.com/eqrion/cbindgen/issues/540">#540</a>) </code></pre></p> </li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/eqrion/cbindgen/commit/83cdbd897f02eaecb090a3f216899dcc14b43959"><code>83cdbd8</code></a> Release v.0.14.4</li> <li><a href="https://github.com/eqrion/cbindgen/commit/030916e3996969d621b0c2102eb89cb889f6a25e"><code>030916e</code></a> Add option for <code>mangling_separator</code> in ExportConfig (<a href="https://github-redirect.dependabot.com/eqrion/cbindgen/issues/502">#502</a>)</li> <li><a href="https://github.com/eqrion/cbindgen/commit/d652d29ae28f9b83bead8feca98cdd30752e63fa"><code>d652d29</code></a> ir: Be less strict when instantiating opaque types.</li> <li><a href="https://github.com/eqrion/cbindgen/commit/6b4181540c146fff75efd500bfb75a2d60403b4c"><code>6b41815</code></a> config: Derive default where appropriate.</li> <li><a href="https://github.com/eqrion/cbindgen/commit/29bc8f45f68113d38a348fe04db62a7456844c68"><code>29bc8f4</code></a> ir: Represent references and const / mut ptrs using the same Type variant.</li> <li><a href="https://github.com/eqrion/cbindgen/commit/afd4a05d4e8d4797beb1111ff5b60e625a2ffe83"><code>afd4a05</code></a> update nonnull expectations</li> <li><a href="https://github.com/eqrion/cbindgen/commit/6fc7c77b0fcd57edae6c6ed11392b1d84ad10697"><code>6fc7c77</code></a> remove duplicated code, correct test names, change config param</li> <li><a href="https://github.com/eqrion/cbindgen/commit/41b7866c148fd7bbef17c3a9d2ba22768f486ec6"><code>41b7866</code></a> Add support for an optional attribute on pointers whose nullability can be in...</li> <li><a href="https://github.com/eqrion/cbindgen/commit/3527e0f1e3c4e8da4f235166b36cb093c2018af6"><code>3527e0f</code></a> Update expectation files</li> <li><a href="https://github.com/eqrion/cbindgen/commit/f41eefba459b6d6e4dc56f1ef6dacb8a2efcf24e"><code>f41eefb</code></a> Add fix for failing scenario</li> <li>Additional commits viewable in <a href="https://github.com/eqrion/cbindgen/compare/v0.14.3...v0.14.4">compare view</a></li> </ul> </details> <br /> [![Dependabot compatibility score](https://api.dependabot.com/badges/compatibility_score?dependency-name=cbindgen&package-manager=cargo&previous-version=0.14.3&new-version=0.14.4)](https://dependabot.com/compatibility-score/?dependency-name=cbindgen&package-manager=cargo&previous-version=0.14.3&new-version=0.14.4) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) - `@dependabot use these labels` will set the current labels as the default for future PRs for this repo and language - `@dependabot use these reviewers` will set the current reviewers as the default for future PRs for this repo and language - `@dependabot use these assignees` will set the current assignees as the default for future PRs for this repo and language - `@dependabot use this milestone` will set the current milestone as the default for future PRs for this repo and language - `@dependabot badge me` will comment on this PR with code to add a "Dependabot enabled" badge to your readme Additionally, you can set the following in your Dependabot [dashboard](https://app.dependabot.com): - Update frequency (including time of day and day of week) - Pull request limits (per update run and/or open at any time) - Out-of-range updates (receive only lockfile updates, if desired) - Security updates (receive only security updates, if desired) </details> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>

…jang Bump cbindgen from 0.14.2 to 0.14.6 Bumps [cbindgen](https://github.com/eqrion/cbindgen) from 0.14.2 to 0.14.6. <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/eqrion/cbindgen/blob/master/CHANGES">cbindgen's changelog</a>.</em></p> <blockquote> <h2>0.14.6</h2> <pre><code> * Fixed the builds with older versions of rustc. </code></pre> <h2>0.14.5</h2> <pre><code> * Add support to specify line ending style ([#568](mozilla/cbindgen#568)) * Add cbindgen:ptrs-as-arrays annotation to allow making function arguments C/C++ arrays. </code></pre> <h2>0.14.4</h2> <pre><code> * Allow to override the mangling separator ([#502](mozilla/cbindgen#502)) <ul> <li> <p>cbindgen now handles better having ZSTs in template parameters, and default template parameters (<a href="https://github-redirect.dependabot.com/eqrion/cbindgen/issues/563">#563</a>).</p> </li> <li> <p>Support for annotating nonnull pointers (<a href="https://github-redirect.dependabot.com/eqrion/cbindgen/issues/558">#558</a>)</p> </li> <li> <p>Fixed bitflags that overflow a signed integer (<a href="https://github-redirect.dependabot.com/eqrion/cbindgen/issues/556">#556</a>)</p> </li> <li> <p>Support for wildcard argument names (<a href="https://github-redirect.dependabot.com/eqrion/cbindgen/issues/550">#550</a>)</p> </li> <li> <p>Support for the never return type, with configurable annotation (<a href="https://github-redirect.dependabot.com/eqrion/cbindgen/issues/549">#549</a>)</p> </li> <li> <p>Properly reject arrays as function arguments (<a href="https://github-redirect.dependabot.com/eqrion/cbindgen/issues/540">#540</a>) </code></pre></p> </li> </ul> <h2>0.14.3</h2> <pre><code> * Introduce cbindgen:ignore comment annotation, to allow ignoring items or modules. ([#519](mozilla/cbindgen#519)) * Support for casts in constant expressions. ([#526](mozilla/cbindgen#526)) * Make a non-fatal error a warning message. ([#535](mozilla/cbindgen#535)) * Add a --metadata option to the CLI, to allow passing pre-computed cargo metadata. ([#538](mozilla/cbindgen#538)) </code></pre> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/eqrion/cbindgen/commit/e4da7d39a6d1481420440b37dcfe85d7ea4527d5"><code>e4da7d3</code></a> v0.14.6</li> <li><a href="https://github.com/eqrion/cbindgen/commit/1e46e53ae22b1d85bc794911dd750543e374bad7"><code>1e46e53</code></a> v0.14.5</li> <li><a href="https://github.com/eqrion/cbindgen/commit/fe656442a562ad1c7c19706d7b068492ecf58230"><code>fe65644</code></a> Document ptrs-as-arrays annotation</li> <li><a href="https://github.com/eqrion/cbindgen/commit/07c3aed03e6a3b53a580f8c6cfc4c9ea446104bb"><code>07c3aed</code></a> Add ptrs-as-arrays tests</li> <li><a href="https://github.com/eqrion/cbindgen/commit/ee4638ab10f4a7dcf9a8abd3b5fc2e2463dfcde5"><code>ee4638a</code></a> Add an annotation to represent pointers as arrays</li> <li><a href="https://github.com/eqrion/cbindgen/commit/73ea04c2b0f8926ebb1491ab311e591721d5dbd2"><code>73ea04c</code></a> Add badge to README</li> <li><a href="https://github.com/eqrion/cbindgen/commit/6b56082be8403c9dfe237cf0d4274369b7a83cd9"><code>6b56082</code></a> Replace travis with Github Actions</li> <li><a href="https://github.com/eqrion/cbindgen/commit/fec6bdad8cd9a0d37d506a6033ae3ee21c4f6989"><code>fec6bda</code></a> Fix clippy warnings</li> <li><a href="https://github.com/eqrion/cbindgen/commit/3ca0e75ed906ae505740657647b08e04cd844f14"><code>3ca0e75</code></a> Add support for specifying line ending style (<a href="https://github-redirect.dependabot.com/eqrion/cbindgen/issues/568">#568</a>)</li> <li><a href="https://github.com/eqrion/cbindgen/commit/83cdbd897f02eaecb090a3f216899dcc14b43959"><code>83cdbd8</code></a> Release v.0.14.4</li> <li>Additional commits viewable in <a href="https://github.com/eqrion/cbindgen/compare/v0.14.2...v0.14.6">compare view</a></li> </ul> </details> <br /> [![Dependabot compatibility score](https://api.dependabot.com/badges/compatibility_score?dependency-name=cbindgen&package-manager=cargo&previous-version=0.14.2&new-version=0.14.6)](https://dependabot.com/compatibility-score/?dependency-name=cbindgen&package-manager=cargo&previous-version=0.14.2&new-version=0.14.6) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) - `@dependabot use these labels` will set the current labels as the default for future PRs for this repo and language - `@dependabot use these reviewers` will set the current reviewers as the default for future PRs for this repo and language - `@dependabot use these assignees` will set the current assignees as the default for future PRs for this repo and language - `@dependabot use this milestone` will set the current milestone as the default for future PRs for this repo and language - `@dependabot badge me` will comment on this PR with code to add a "Dependabot enabled" badge to your readme Additionally, you can set the following in your Dependabot [dashboard](https://app.dependabot.com): - Update frequency (including time of day and day of week) - Pull request limits (per update run and/or open at any time) - Out-of-range updates (receive only lockfile updates, if desired) - Security updates (receive only security updates, if desired) </details>

adamski mentioned this pull request Apr 1, 2020

Allow renaming of generic types #503

Open

emilio requested changes Apr 4, 2020

View reviewed changes

adamski added 3 commits August 14, 2020 17:33

Add option for mangling_separator in ExportConfig

858d3d8

Add option for mangling_separator in ExportConfig

d0a2226

Fix build issues and refactor after rebase

fa22397

adamski force-pushed the adamski/mangling-separator branch from 169960e to fa22397 Compare August 14, 2020 22:02

adamski added 2 commits August 14, 2020 23:11

Refactor after rebase

937e3ba

Fix whitespace/alignment issues

90557ff

adamski requested a review from emilio August 14, 2020 22:17

emilio reviewed Aug 15, 2020

View reviewed changes

src/bindgen/mangle.rs Outdated Show resolved Hide resolved

src/bindgen/mangle.rs Outdated Show resolved Hide resolved

adamski added 4 commits August 15, 2020 13:23

Code formatting

0f73c70

Refactor to Option<&str> arguments and add test

0254d60

Reformat function call to multiline

86a6909

Fix typo and keep naming consistent

035d704

adamski requested a review from emilio August 17, 2020 11:52

adamski mentioned this pull request Aug 17, 2020

Capitalize primitives #564

Closed

emilio reviewed Aug 17, 2020

View reviewed changes

Run rustfmt

3270aec

adamski requested a review from emilio August 18, 2020 15:39

emilio approved these changes Aug 18, 2020

View reviewed changes

emilio merged commit 030916e into mozilla:master Aug 18, 2020

emilio mentioned this pull request Feb 23, 2021

Possible to remove the _ in mangle_path for C? #437

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add option for `mangling_separator` in ExportConfig#502

Add option for `mangling_separator` in ExportConfig#502
emilio merged 10 commits intomozilla:masterfrom
adamski:adamski/mangling-separator

adamski commented Apr 1, 2020

Uh oh!

emilio left a comment

Uh oh!

emilio Apr 4, 2020

Uh oh!

emilio Apr 4, 2020

Uh oh!

emilio Apr 4, 2020

Uh oh!

adamski commented Apr 6, 2020

Uh oh!

Uh oh!

Uh oh!

emilio left a comment

Uh oh!

adamski commented Aug 18, 2020

Uh oh!

emilio left a comment

Uh oh!

emilio commented Aug 18, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

adamski commented Apr 1, 2020

Uh oh!

emilio left a comment

Choose a reason for hiding this comment

Uh oh!

emilio Apr 4, 2020

Choose a reason for hiding this comment

Uh oh!

emilio Apr 4, 2020

Choose a reason for hiding this comment

Uh oh!

emilio Apr 4, 2020

Choose a reason for hiding this comment

Uh oh!

adamski commented Apr 6, 2020

Uh oh!

Uh oh!

Uh oh!

emilio left a comment

Choose a reason for hiding this comment

Uh oh!

adamski commented Aug 18, 2020

Uh oh!

emilio left a comment

Choose a reason for hiding this comment

Uh oh!

emilio commented Aug 18, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants