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

feat: implement __has_declspec_attribute #514

Merged
merged 4 commits into from
Sep 30, 2023

Conversation

eightfilms
Copy link
Contributor

closes #419

CI seems to be failing, not sure if it's related to this?

src/target.zig Outdated
@@ -684,6 +684,7 @@ pub fn toLLVMTriple(target: std.Target, buf: []u8) []const u8 {
.watchos => "watchos",
.driverkit => "driverkit",
.shadermodel => "shadermodel",
.liteos => "liteos",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this in order to make it compile, but #510 fixes this.

Copy link
Owner

@Vexu Vexu left a comment

Choose a reason for hiding this comment

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

__has_declspec_attribute(<anything>) should return 0 when langopts.declspec_attrs == false.

CI failure is definitely because of llvm-17, I'll try to figure it out.

@eightfilms
Copy link
Contributor Author

eightfilms commented Sep 30, 2023

@Vexu Hmm, am I misundestanding the usage of

//aro-args -fdeclspec

at the top of the file? I was under the impression that including the args switches on the options in tests. If that's the case then the ifs would reflect accurately on the behaviour, no?

@Vexu
Copy link
Owner

Vexu commented Sep 30, 2023

No you understand correctly, I was talking about the case where -fdeclspec was not specified.

@eightfilms
Copy link
Contributor Author

Ah should I add a check there as well? I'm assuming you mean no_declspec.c

@Vexu
Copy link
Owner

Vexu commented Sep 30, 2023

Something like that yes.

@Vexu Vexu merged commit 05bedcb into Vexu:master Sep 30, 2023
3 checks passed
@Vexu
Copy link
Owner

Vexu commented Sep 30, 2023

Thanks!

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.

Implement __has_declspec_attribute
2 participants