-
Notifications
You must be signed in to change notification settings - Fork 162
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
[small] tidy up warnings #2323
[small] tidy up warnings #2323
Conversation
Please enable Also I think, now this shows warnings in macos. For example, from the build step in https://github.com/lcompilers/lpython/actions/runs/6168547819/job/16741232055?pr=2323: /Users/runner/work/lpython/lpython/lpython-0.19.0-220-g8e845514a/src/libasr/runtime/lfortran_intrinsics.c:1994:22: warning: format specifies type 'long *' but the argument has type 'int64_t *' (aka 'long long *') [-Wformat]
scanf("%ld", p);
~~~ ^
%lld
/Users/runner/work/lpython/lpython/lpython-0.19.0-220-g8e845514a/src/libasr/runtime/lfortran_intrinsics.c:2008:30: warning: format specifies type 'long *' but the argument has type 'int64_t *' (aka 'long long *') [-Wformat]
fscanf(filep, "%ld", p);
~~~ ^
%lld
/Users/runner/work/lpython/lpython/lpython-0.19.0-220-g8e845514a/src/libasr/runtime/lfortran_intrinsics.c:1994:22: warning: format specifies type 'long *' but the argument has type 'int64_t *' (aka 'long long *') [-Wformat]
scanf("%ld", p);
~~~ ^
%lld
/Users/runner/work/lpython/lpython/lpython-0.19.0-220-g8e845514a/src/libasr/runtime/lfortran_intrinsics.c:2008:30: warning: format specifies type 'long *' but the argument has type 'int64_t *' (aka 'long long *') [-Wformat]
fscanf(filep, "%ld", p);
~~~ ^
%lld |
Ah, I see. You are right; there are warning messages in macos but not in ubuntu. I think it's related to the compilers. Just to confirm this, I checked my previous PR; there were warning messages in ubuntu but not in macos. Please see: https://github.com/lcompilers/lpython/actions/runs/6164474892/job/16730254578 I also checked on the Godbolt and found that I mainly referred to this hack from stack-overflow. Thanks! |
Please also enable |
The runtime library must be build with Then we need to fix it. Yes, this |
See: lpython/src/libasr/runtime/lfortran_intrinsics.c Lines 2598 to 2602 in df414fb
|
Interesting! Thanks for sharing. I have updated it using Macros. EDIT: I see there are warnings on unused variables, I'm looking into it. Will update it soon. Thanks |
Hi, @certik @Shaikh-Ubaid @Thirumalai-Shaktivel, In LPython CI (3.10, ubuntu-latest), we are currently getting the following warning: /home/runner/work/lpython/lpython/lpython-0.19.0-232-g05cd6c4a7/src/libasr/runtime/lfortran_intrinsics.c: In function ‘_lfortran_read_int32’:
/home/runner/work/lpython/lpython/lpython-0.19.0-232-g05cd6c4a7/src/libasr/runtime/lfortran_intrinsics.c:1972:9: warning: ignoring return value of ‘scanf’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
1972 | scanf("%d", p);
| ^~~~~~~~~~~~~~ I stumbled upon an interesting discussion here. I found it is the expected behavior of the I initially thought we might be getting this warning due to the unused variable What do you think we should do in these cases to remove the warning? I'd appreciate your response. Thank you! |
I think we can try |
I think we can also disable the warning related to the unused result using the flag |
We can, but I think those are good warnings, and we should only disable them when we know the result value was meant to be ignored with the |
Thanks, @Thirumalai-Shaktivel and @Shaikh-Ubaid, for sharing two other ways to disable the warning message.
Got it! Thanks, @certik, for clarifying it. It sounds totally reasonable to me. This PR is ready by my side. Please let me know if there's anything to be improved in this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks good. We should probably use a different ifdef for apple (I am not sure if this define can be off sometimes for apple), but for now this is good.
Thanks! |
Thank you so much @khushi-411. Great work! I appreciate it. |
The above seems to be a correct approach. Please see https://chat.openai.com/share/51fe5b13-86a1-4ebe-809d-a71d7d284cab. It seems for |
For the unused result warnings, it seems we also have the possibility/approach of casting the result to $ cat examples/main.c
#include <stdio.h>
__attribute__((warn_unused_result))
int custom_function(int value) {
return value * 2;
}
int main() {
custom_function(5); // This will trigger a warning for unused result
return 0;
} $ clang -Wunused-result -o unused_result examples/main.c
examples/main.c:9:5: warning: ignoring return value of function declared with 'warn_unused_result' attribute [-Wunused-result]
custom_function(5); // This will trigger a warning for unused result
^~~~~~~~~~~~~~~ ~
1 warning generated. Casting the result to int main() {
(void) custom_function(5);
return 0;
} $ clang -Wunused-result -o unused_result examples/main.c
$ PS: It seems to work on mac. We might need to check on Linux. |
Ah, right. My bad, I missed it. Thanks for pointing it out, @Shaikh-Ubaid! :-)
Nice approach! I tested it on Ubuntu, and it appears clang is happy, but gcc still throws the same warning msg. Please see: (lp) khushi@khushi:~/Documents/lpython$ clang -Wunused-result -o result main.c
(lp) khushi@khushi:~/Documents/lpython$
(lp) khushi@khushi:~/Documents/lpython$ gcc -Wunused-result -o result main.c
main.c: In function ‘main’:
main.c:9:12: warning: ignoring return value of ‘custom_function’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
9 | (void) custom_function(5); // This will trigger a warning for unused result
| ^~~~~~~~~~~~~~~~~~ |
Hi, the PR removes warnings during build time and tests. Please see:
and
cc @certik @czgdp1807