Remove global -Wno-return-type from CMakeLists.txt #9387
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
the
CLA Signed
✅ Deploy Preview for meta-velox canceled.
|
acvictor
changed the title
acvictor
changed the title
acvictor
changed the title
|
@pedroerp @mbasmanova can you please review this change? |
pedroerp
reviewed
Apr 8, 2024
|
I am absolutely for the removal of global warning suppression, it seems pragmas are the way to go here as adding the warning to the target would also potentially shadow actual issues... in that regard it would also be nice to not disable the warning globally for >12.2 (though just using clang in at least one ci job would also be a workaround...) |
kgpai
reviewed
Apr 9, 2024
acvictor
changed the title
|
@mbasmanova @pedroerp can you please give this another review? |
pedroerp
reviewed
Apr 9, 2024
Thank you for the review! |
pedroerp
reviewed
Apr 9, 2024
pedroerp
approved these changes
Apr 9, 2024
pedroerp
added
the
ready-to-merge
|
@mbasmanova has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@acvictor Would you rebase? |
majetideepak
approved these changes
Apr 9, 2024
…r/removeNoRetrun
Done, thanks! |
|
@mbasmanova has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@mbasmanova merged this pull request in 22279f9. |
|
Conbench analyzed the 1 benchmark run on commit There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
yanngyoung
pushed a commit
to yanngyoung/velox
that referenced
this pull request
Apr 12, 2024
…#9387) Summary: This PR makes a change to remove the globally applied `-Wno-return-type`, fixes some violations. This PR originally facebookincubator#4618 disabled it. In the case of a code bug, it may become difficult to diagnose the actual cause. An example of this is ``` #include <stdio.h> bool test1() { printf("Called inner\n"); } void test() { test1(); } int main(int argc, char* argv[]) { printf("Called test\n"); test(); printf("Finished test\n"); return 0; } ``` which crashes when compiled with -O2 due to bad codegen. Pull Request resolved: facebookincubator#9387 Reviewed By: pedroerp Differential Revision: D55933467 Pulled By: mbasmanova fbshipit-source-id: da748fc5df19adb3d6e295973cc12e75ac43f2c1
Joe-Abraham
pushed a commit
to Joe-Abraham/velox
that referenced
this pull request
Jun 7, 2024
…#9387) Summary: This PR makes a change to remove the globally applied `-Wno-return-type`, fixes some violations. This PR originally facebookincubator#4618 disabled it. In the case of a code bug, it may become difficult to diagnose the actual cause. An example of this is ``` #include <stdio.h> bool test1() { printf("Called inner\n"); } void test() { test1(); } int main(int argc, char* argv[]) { printf("Called test\n"); test(); printf("Finished test\n"); return 0; } ``` which crashes when compiled with -O2 due to bad codegen. Pull Request resolved: facebookincubator#9387 Reviewed By: pedroerp Differential Revision: D55933467 Pulled By: mbasmanova fbshipit-source-id: da748fc5df19adb3d6e295973cc12e75ac43f2c1
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR makes a change to remove the globally applied
-Wno-return-typeand fixes some violations. PR #4618 originally disabled it. In the case of a code bug, it may become difficult to diagnose the actual cause. An example of this iswhich crashes when compiled with -O2 due to bad codegen.