Skip to content

add define to supress macro warnings Wimplicit-fallthrough for Clang - #2

Merged
MaximeModIo merged 2 commits into
masterfrom
disable_fallthroughWarningsCoroutine
Mar 24, 2023
Merged

MaximeModIo merged 2 commits into
masterfrom
disable_fallthroughWarningsCoroutine

Conversation

@MaximeModIo

Copy link
Copy Markdown

Link to task : https://app.clickup.com/t/9003015469/GF_CPP-731

Supress the warnings in the coroutine macro that was causing +400 warnings.

@MaximeModIo
MaximeModIo requested review from stephenwhittle and removed request for stephenwhittle March 2, 2023 05:16

#include "coroutine.hpp"

#if defined(__clang__)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do we need something that's compiler-portable here rather than just the GCC pragma? Should we be using Hedley to handle the suppression? I'm not sure that this pragma will work on PS or Switch which use clang.
Given that we include yield.hpp at the top and unyield.hpp at the bottom, I'm wondering if we should make our own wrappers for those files in the SDK itself which handle these warnings and can use Hedley to handle this more portably.

@MaximeModIo MaximeModIo Mar 20, 2023

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Hedley dont contains anything to handle the warning suppresion for Implicit-fallthough, only HEDLEY_FALL_THROUGH but i dont work for macro.

I also tried to add the push/diagnostic ignored wimplicit in yield.hpp and pop in unyield.hpp , it will also supress the warnings who are not contained in the macro, do we really want that ?

what i will suggest, will be to modify the macro i created to make it more compiler-portable.

What do you think @stephenwhittle ?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Yep, making it more compiler portable will be fine and then this will be approved

@MaximeModIo

MaximeModIo commented Mar 23, 2023

Copy link
Copy Markdown
Author

Update, @stephenwhittle , tested it on NX and Linux

@stephenwhittle
stephenwhittle self-requested a review March 24, 2023 01:36
@MaximeModIo
MaximeModIo merged commit e160bb6 into master Mar 24, 2023
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