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

Remove needless MRB_API #3216

Merged
merged 1 commit into from Sep 27, 2016
Merged

Remove needless MRB_API #3216

merged 1 commit into from Sep 27, 2016

Conversation

kou
Copy link
Contributor

@kou kou commented Sep 25, 2016

ref #3215

If a function (such as mrb_read_irep_file()) is declared without MRB_API
in header file (such as include/mruby/dump.h), implementation of the
function in source file (such as src/load.c) should also defined without
MRB_API.

If MRB_API is mismatch, Visual C++ reports link error with C2375 error
code: https://msdn.microsoft.com/en-us/library/5k6kw95a.aspx

ref mruby#3215

If a function (such as mrb_read_irep_file()) is declared without MRB_API
in header file (such as include/mruby/dump.h), implementation of the
function in source file (such as src/load.c) should also defined without
MRB_API.

If MRB_API is mismatch, Visual C++ reports link error with C2375 error
code: https://msdn.microsoft.com/en-us/library/5k6kw95a.aspx
@kou kou mentioned this pull request Sep 25, 2016
@matz matz merged commit 6bc4b47 into mruby:master Sep 27, 2016
2 checks passed
@matz
Copy link
Member

@matz matz commented Sep 27, 2016

merged. But mrb_str_strlen() should be MRB_API. I will fix.

@kou kou deleted the remove-needless-mrbapi branch Sep 27, 2016
@kou
Copy link
Contributor Author

@kou kou commented Sep 27, 2016

Thanks.
BTW, how about removing all MRB_API from **/*.c? They are optional. If we have MRB_API in **/*.h, we don't need to put MRB_API into **/*.c. If we only have MRB_API in **/*.h, we don't need to care about this problem.

@zzak
Copy link
Member

@zzak zzak commented Sep 27, 2016

👍 We shouldn't need extern only for declaration, not definition. But, I wonder if changing them all at once will have side-effects 💭

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.

None yet

3 participants