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

Allow attributes in macro invocations #22

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

ids1024
Copy link

@ids1024 ids1024 commented Oct 15, 2024

Allows #[cfg] to be used to make certain definitions conditional. Such as for library version feature flags.

Needed for Smithay/wayland-rs#735 so it can have a feature flag for the libwayland version.

Seems to work, but still needs to be added to dlib-test, with CI to test with and without the feature.

Allows `#[cfg]` to be used to make certain definitions conditional. Such
as for library version feature flags.
There is still a "local ambiguity when calling macro `external_library`"
error. That appears to be a real bug.
Currently fails at macro expansion due to existing issue.
@ids1024
Copy link
Author

ids1024 commented Oct 16, 2024

Added some testing improvements. I see some issues trying to test statics: and varargs: (#23, #24), but I think the changes here are good, and aren't causing issues that don't already exist.

I don't think those block this from being merged and released. Though a semver breaking version of the crate will probably be needed at some point to fix those.

@ids1024 ids1024 marked this pull request as ready for review October 16, 2024 20:05
@@ -9,6 +9,7 @@ exclude = ["/dlib-test", "README.tpl"]
readme = "README.md"
keywords = ["dylib", "dlopen"]
categories = ["api-bindings"]
edition = "2021"
Copy link
Owner

Choose a reason for hiding this comment

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

Is this specifically needed for this change?

Copy link
Author

Choose a reason for hiding this comment

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

It shouldn't be necessary, no. It just seems a bit unusual at this point for a crate to be using the default edition.

I don't think there's any need to support Rust versions too old for the 2021 edition at this point.

Copy link
Owner

Choose a reason for hiding this comment

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

In that case, could you also update the CI accordingly? It's currently failing because of that change.

Copy link
Author

Choose a reason for hiding this comment

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

I've updated CI to the first version of Rust that supports the 2021 edition.

@elinorbgr
Copy link
Owner

Just to confirm, have you tested that this changes actually makes it possible to add a feature flag to Smithay/wayland-rs#735 ?

@ids1024
Copy link
Author

ids1024 commented Oct 31, 2024

Yes. Simply adding #[cfg(feature = "libwayland_1_22")] appears to work as expected in wayland-sys if it uses this version of dlib.

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.

2 participants