Fix C++20 compiler warnings#44
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThis PR refactors the text drawing API throughout the framework to use explicit 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@framework/draw/painter.h`:
- Around line 22-23: Remove the extra leading blank line before the `#pragma` once
directive in framework/draw/painter.h so the file begins immediately with
"`#pragma` once"; this will satisfy the codestyle formatter and allow the CI
codestyle job to pass.
In `@framework/draw/utils/drawdatajson.cpp`:
- Around line 338-339: The deserializer currently assigns text.alignment and
text.textFlags only from obj["alignment"] and obj["textFlags"], losing legacy
values stored under a single "flags" field; update the DrawText deserialization
to detect a legacy obj.contains("flags") and, when present and
alignment/textFlags keys are missing, read int flags = obj["flags"].toInt() and
decompose that value into the appropriate Alignment and TextFlags bits before
assigning text.alignment and text.textFlags (use the same bit masks/enum mapping
used elsewhere for these enums to ensure compatibility).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 054f890f-696f-4ecc-8825-18e38464ca40
📒 Files selected for processing (15)
framework/audio/engine/internal/fx/reverb/circularsamplebuffer.hframework/draw/bufferedpaintprovider.cppframework/draw/bufferedpaintprovider.hframework/draw/internal/qpainterprovider.cppframework/draw/internal/qpainterprovider.hframework/draw/ipaintprovider.hframework/draw/painter.cppframework/draw/painter.hframework/draw/types/drawdata.hframework/draw/types/drawtypes.hframework/draw/utils/drawdatacomp.cppframework/draw/utils/drawdatajson.cppframework/draw/utils/drawdatapaint.cppframework/testflow/internal/draw/abpaintprovider.cppframework/uicomponents/qml/Muse/UiComponents/abstracttableviewmodel.cpp
| text.alignment = static_cast<Alignment>(obj["alignment"].toInt()); | ||
| text.textFlags = static_cast<TextFlags>(obj["textFlags"].toInt()); |
There was a problem hiding this comment.
Add legacy flags fallback in DrawText deserialization.
Line 338-339 only reads alignment and textFlags; older JSON containing only flags will silently deserialize with zeroed layout flags.
💡 Proposed compatibility fix
- text.alignment = static_cast<Alignment>(obj["alignment"].toInt());
- text.textFlags = static_cast<TextFlags>(obj["textFlags"].toInt());
+ const int legacyFlags = obj.value("flags").toInt();
+ text.alignment = obj.contains("alignment")
+ ? static_cast<Alignment>(obj.value("alignment").toInt())
+ : static_cast<Alignment>(legacyFlags);
+ text.textFlags = obj.contains("textFlags")
+ ? static_cast<TextFlags>(obj.value("textFlags").toInt())
+ : static_cast<TextFlags>(legacyFlags);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| text.alignment = static_cast<Alignment>(obj["alignment"].toInt()); | |
| text.textFlags = static_cast<TextFlags>(obj["textFlags"].toInt()); | |
| const int legacyFlags = obj.value("flags").toInt(); | |
| text.alignment = obj.contains("alignment") | |
| ? static_cast<Alignment>(obj.value("alignment").toInt()) | |
| : static_cast<Alignment>(legacyFlags); | |
| text.textFlags = obj.contains("textFlags") | |
| ? static_cast<TextFlags>(obj.value("textFlags").toInt()) | |
| : static_cast<TextFlags>(legacyFlags); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@framework/draw/utils/drawdatajson.cpp` around lines 338 - 339, The
deserializer currently assigns text.alignment and text.textFlags only from
obj["alignment"] and obj["textFlags"], losing legacy values stored under a
single "flags" field; update the DrawText deserialization to detect a legacy
obj.contains("flags") and, when present and alignment/textFlags keys are
missing, read int flags = obj["flags"].toInt() and decompose that value into the
appropriate Alignment and TextFlags bits before assigning text.alignment and
text.textFlags (use the same bit masks/enum mapping used elsewhere for these
enums to ensure compatibility).
|
|
||
| void drawText(const PointF& point, const String& text) override; | ||
| void drawText(const RectF& rect, int flags, const String& text) override; | ||
| void drawText(const RectF& rect, Alignment alignment, TextFlags textFlags, const String& text) override; |
There was a problem hiding this comment.
I think it's better to add a new method here...
and then delete the old one.
There was a problem hiding this comment.
Hm... but I got the corresponding MSS PR already ready: musescore/MuseScore#33504
There was a problem hiding this comment.
We can try to merge them quickly!
But it's better to go with compatibility, especially in simple cases like these. Add something new, mark something old as deprecated, and remove it after syncing.
cbjeukendrup
force-pushed
the
c++20-warning-fixes
branch
from
916f480 to
dfeda3e
Compare
2 participants
(without relying on C++20 features)