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

Fix no return static analysis error in SchedulerPriorityUtils.h #47911

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

acoates-ms
Copy link
Contributor

Summary:

In react-native-windows our static analysis tools report an error for timeoutForSchedulerPriority due to cases where it may not always return a value. This is an upstreaming of the patch we have to fix that error.

Changelog:

[INTERNAL] [FIXED] - Fix no return static analysis error in SchedulerPriorityUtils.h

Test Plan:

Building should be sufficient.

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Microsoft Partner: Microsoft Partner labels Nov 22, 2024
@facebook-github-bot facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Nov 22, 2024
@@ -50,6 +50,9 @@ static inline std::chrono::milliseconds timeoutForSchedulerPriority(
return std::chrono::seconds(10);
case SchedulerPriority::IdlePriority:
return std::chrono::minutes(5);
default:
react_native_assert(false && "Unsupported SchedulerPriority value");
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: could you place the return outside the switch/case? Otherwise this suppresses any Clang warnings for switch/case exhaustiveness if a new enum value is added.

Copy link
Contributor

Choose a reason for hiding this comment

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

We also do LOG(FATAL) in some places which is marked as [[noreturn]], and will run in release builds, but either should be fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Microsoft Partner: Microsoft Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants