Skip to content
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

Demonstrate dropping support for libclang < 5.0 #1807

Closed
wants to merge 4 commits into from

Conversation

kulp
Copy link
Member

@kulp kulp commented Jun 20, 2020

In #1006 (comment) the idea was raised of possibly removing support for old libclang versions. The current draft PR exists mainly to help evaluate the impact of such an idea. If this idea is not worth pursuing now, the current PR can be closed, but perhaps at least a policy could be determined as to how long old libclang versions should be supported.

For reference:

  • libclang 3.8.0 was released on 08 Mar 2016
  • libclang 3.9.0 was released on 02 Sep 2016
  • libclang 4.0.0 was released on 13 Mar 2017
  • libclang 5.0.0 was released on 07 Sep 2017

The impact overall in bindgen seems quite small no longer seems small in terms of code size. However, the reduction in bug surface area might still make the change worthwhile, at least in stages. Presumably old issues such as #1006 could be closed.

@highfive
Copy link

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!

Copy link
Contributor

@emilio emilio left a comment

Choose a reason for hiding this comment

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

I'm generally ok with this. https://bugzilla.mozilla.org/show_bug.cgi?id=1579189 updated Firefox's min clang version to 5.0, and this removes some gross hacks.

I do wonder if any other users of bindgen are using such an old libclang...

src/clang.rs Outdated Show resolved Hide resolved
@kulp kulp force-pushed the strip-old-clang branch 3 times, most recently from 69a9355 to fbca047 Compare June 28, 2020 22:11
@kulp
Copy link
Member Author

kulp commented Jun 28, 2020

I reorganized the commits so that support for older clang versions is removed in order, so that we could pick the best place to stop if we did not want to jump to clang 5.0 immediately.

@bors-servo
Copy link

☔ The latest upstream changes (presumably a492a9e) made this pull request unmergeable. Please resolve the merge conflicts.

@bors-servo
Copy link

☔ The latest upstream changes (presumably 3e2566d) made this pull request unmergeable. Please resolve the merge conflicts.

@kulp

This comment was marked as resolved.

@bors-servo
Copy link

☔ The latest upstream changes (presumably 802561e) made this pull request unmergeable. Please resolve the merge conflicts.

@bors-servo
Copy link

☔ The latest upstream changes (presumably 4f714ab) made this pull request unmergeable. Please resolve the merge conflicts.

@bors-servo
Copy link

☔ The latest upstream changes (presumably fde75f6) made this pull request unmergeable. Please resolve the merge conflicts.

@bors-servo
Copy link

☔ The latest upstream changes (presumably b3e6a98) made this pull request unmergeable. Please resolve the merge conflicts.

@kulp
Copy link
Member Author

kulp commented Feb 20, 2022

Due to the merging of #1830 and #2155, the current PR has now been superseded by #2166, which is pending. Since the current PR was only intended to be a demonstration anyway, and it is full of chaff, I am closing it now.

@kulp kulp closed this Feb 20, 2022
@kulp kulp deleted the strip-old-clang branch March 15, 2022 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants