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 some static analyzer warnings #8249

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

Conversation

tgoyne
Copy link
Member

@tgoyne tgoyne commented May 25, 2023

No description provided.

@tgoyne tgoyne self-assigned this May 25, 2023
@cla-bot cla-bot bot added the cla: yes label May 25, 2023
Base automatically changed from tg/drop-xcode-13 to master May 26, 2023 17:29
Copy link
Collaborator

@dianaafanador3 dianaafanador3 left a comment

Choose a reason for hiding this comment

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

LGTM!! just a few minors comments

@@ -11,6 +12,10 @@ Replace Xcode 14.3 binaries with 14.3.1, which has important bug fixes for Swift
### Fixed
* Allow support for implicit boolean queries on Swift's Type Safe Queries API
([#8212](https://github.com/realm/realm-swift/issues/8212)).
* `-[RLMAsymmetricObject createObject:withValue:]` was marked as having a
non-null return value despite always returning `nil` (since v10.29.0).
* Eliminate several clang static analyzer warnings which did not report actual
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you tag the issue which reported this?

}

if constexpr (is_any_v<C, Columns<Dictionary>> && is_any_v<T, Columns<StringData>, Columns<BinaryData>>) {
// Core only implements these for Columns<Mixed> due to Dictionary being Mixed internall
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// Core only implements these for Columns<Mixed> due to Dictionary being Mixed internall
// Core only implements these for Columns<Mixed> due to Dictionary being Mixed internal

do_add_diacritic_sensitive_string_constraint(operatorType, predicateOptions, std::forward<C>(column), value);
}

if constexpr (is_any_v<C, Columns<Dictionary>> && is_any_v<T, Columns<StringData>, Columns<BinaryData>>) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we just check if T is mixed and throw otherwise

@@ -1603,6 +1603,10 @@ - (void)testConvertToEmbeddedAddingMoreLinks {
XCTAssertNotNil(oldObject);
XCTAssertNotNil(newObject);
RLMObject *childObject = newObject[@"object"];
XCTAssertNotNil(childObject);
if (childObject == nil) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If the above assertion fails, is this even run?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. The rest of the test continues running even if an XCTest assertion fails.

@LowAmmo
Copy link

LowAmmo commented Aug 29, 2023

Was just about to start making similar changes when I saw this PR!

Thanks for doing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants