Skip to content

local_serializer.cc: fix lossy implicit conversion compiler warning #9433

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

Merged

Conversation

dconeybe
Copy link
Contributor

@dconeybe dconeybe commented Mar 9, 2022

Fix the following warning in local_serializer.cc:

Implicit conversion loses integer precision: 'std::vector<firebase::firestore::model::Segment>::size_type' (aka 'unsigned long') to 'pb_size_t' (aka 'unsigned int')

The fix is to explicitly cast to suppress the compiler warning. There should NEVER be a case where the size of the vector is greater than 2**32, so, in practice, this will never be a lossy conversion.

Fixes #9430

#no-changelog

Copy link
Member

@paulb777 paulb777 left a comment

Choose a reason for hiding this comment

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

Thanks for fixing. Please add a changelog describing that this fixes the warnings introduced in 8.13.0.

@google-oss-bot
Copy link

google-oss-bot commented Mar 9, 2022

Coverage Report 1

Affected Products

  • FirebaseFirestore-iOS-FirebaseFirestore.framework

    Overall coverage changed from 86.31% (e5152af) to 86.31% (c938a91) by +0.00%.

    FilenameBase (e5152af)Merge (c938a91)Diff
    leveldb_key.cc98.81%98.38%-0.43%
    leveldb_mutation_queue.cc92.08%93.58%+1.51%
    local_serializer.cc83.52%83.57%+0.05%

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/1xj7JK4D0k.html

@dconeybe
Copy link
Contributor Author

dconeybe commented Mar 9, 2022

Thanks for fixing. Please add a changelog describing that this fixes the warnings introduced in 8.13.0.

Done.

Copy link
Member

@ncooke3 ncooke3 left a comment

Choose a reason for hiding this comment

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

LGTM. No warnings in the pod linting CI 👍

@dconeybe dconeybe merged commit b3bbea9 into master Mar 9, 2022
@dconeybe dconeybe deleted the dconeybe/FixLocalSerializerImplicitConversionWarning branch March 9, 2022 16:00
@firebase firebase locked and limited conversation to collaborators Apr 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Firestore 8.13.0 - Warnings in Firestore/core/src/local/local_serializer.cc
5 participants