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

NativeAOT-LLVM: Add void to .c functions that have no parameters - something about K&R functions?? #2004

Merged
merged 3 commits into from
Oct 1, 2022

Conversation

yowl
Copy link
Contributor

@yowl yowl commented Sep 29, 2022

This PR fixes this warning which seems to be new in later versions of clang. Interestingly runtime doesn't have this fix, so don't know how that is working, maybe its not on the later versions of clang yet. If the header declares the args as void the .c needs to do the same to avoid the warning. At least that's my observation...

E:/GitHub/runtimelab/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.c(509,42): error G0E77CA38: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes] [E:\GitHub\runtimelab\src\libraries\Native\build-native.proj]

https://reviews.llvm.org/D122895

Might be relevant, there's a mention of void in the comments.

@yowl yowl changed the title NativeAOT-LLVM: more sig sync NativeAOT-LLVM: Add void to .c functions that have no parameters - something about K&R functions?? Sep 29, 2022
@yowl
Copy link
Contributor Author

yowl commented Sep 29, 2022

cc @SingleAccretion

@jkotas
Copy link
Member

jkotas commented Sep 29, 2022

dotnet/runtime "fix" for this problem was to disable the warning: https://github.com/dotnet/runtime/pull/70693/files#diff-7f8d8bfbe22b648f50efce6df1dddb9fb53e13efa083a0f54c7a3f2ac5e5bb15R153 . Would you like to do the same in this branch to avoid growing unnecessary delta?

Also, I am happy to take this fix and delete the warning disable in dotnet/runtime if you would like to submit it there. Thanks!

@jkotas
Copy link
Member

jkotas commented Oct 1, 2022

For the feature/NativeAOT-LLVM branch, we should just disable the warning, and wait for the full fix to come with merge from main.

@yowl Could you please update this PR to just disable the warnings?

@yowl
Copy link
Contributor Author

yowl commented Oct 1, 2022

Sure, this looks ok locally, hopefully it will pass.

@jkotas jkotas merged commit b9e0806 into dotnet:feature/NativeAOT-LLVM Oct 1, 2022
@yowl yowl deleted the sync-sigs branch October 1, 2022 22:51
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