FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix "Undefined symbols" linker error when DocumentChange::npos was used by dconeybe · Pull Request #474 · firebase/firebase-cpp-sdk · GitHub

/ firebase-cpp-sdk Public

Fix "Undefined symbols" linker error when DocumentChange::npos was used #474

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.

Sign up for GitHub

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
dconeybe merged 6 commits into from
Jun 21, 2021
Conversation 10 Commits 6 Checks 0 Files changed
Merged

Fix "Undefined symbols" linker error when DocumentChange::npos was used #474

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
77d3475
Fix "Undefined symbols" linker error when DocumentChange::npos was used.
dconeybe Jun 16, 2021
ee4fa77
Add change log entry
dconeybe Jun 16, 2021
3be0e69
Fix version number in change log entry (8.0.1 -> 8.1.0)
dconeybe Jun 16, 2021
544ae52
Merge remote-tracking branch 'origin/main' at 3803777c066d675909241ca…
dconeybe Jun 17, 2021
005ae9b
Merge 'origin/main' at 287f764682e9cba15cedb8da4492ee495fa09684
dconeybe Jun 21, 2021
1e1c41d
Add backticks around DocumentChange::npos.
dconeybe Jun 21, 2021
File filter

Filter by extension

Filter by extension .cc  (3) .md  (1) All 2 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
8 changes: 8 additions & 0 deletions firestore/integration_test/src/integration_test.cc
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,14 @@ TEST_F(FirebaseFirestoreBasicTest, TestQuery) {
"int", firebase::firestore::FieldValue::Integer(101))))));
}

TEST_F(FirebaseFirestoreBasicTest, TestDocumentChangeNpos) {
// This test may seem pointless, but it exists to avoid the long-standing
// latent bug that `npos` was not defined on non-Android platforms and
// would therefore fail to link if used.
EXPECT_EQ(firebase::firestore::DocumentChange::npos,
static_cast<std::size_t>(-1));
}

TEST_F(FirebaseFirestoreBasicTest,
TestInvalidatingReferencesWhenDeletingFirestore) {
delete firestore_;
Expand Down
8 changes: 8 additions & 0 deletions firestore/integration_test_internal/src/integration_test.cc
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,14 @@ TEST_F(FirebaseFirestoreBasicTest, TestQuery) {
"int", firebase::firestore::FieldValue::Integer(101))))));
}

TEST_F(FirebaseFirestoreBasicTest, TestDocumentChangeNpos) {
// This test may seem pointless, but it exists to avoid the long-standing
// latent bug that `npos` was not defined on non-Android platforms and
// would therefore fail to link if used.
EXPECT_EQ(firebase::firestore::DocumentChange::npos,
static_cast<std::size_t>(-1));
}

TEST_F(FirebaseFirestoreBasicTest,
TestInvalidatingReferencesWhenDeletingFirestore) {
delete firestore_;
Expand Down
2 changes: 2 additions & 0 deletions firestore/src/common/document_change.cc
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ using Type = DocumentChange::Type;
// Older NDK (r16b) fails to define this properly. Fix this when support for
// the older NDK is removed.
const std::size_t DocumentChange::npos = static_cast<std::size_t>(-1);
#else
constexpr std::size_t DocumentChange::npos;
Comment thread
dconeybe marked this conversation as resolved.
#endif // defined(ANDROID)

DocumentChange::DocumentChange() {}
Expand Down
5 changes: 5 additions & 0 deletions release_build_files/readme.md
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,11 @@ code.

## Release Notes

### 8.1.0
- Changes
- Firestore: Fixed a linker error when `DocumentChange::npos` was used.
([#474](https://github.com/firebase/firebase-cpp-sdk/pull/474)).

var-const Jun 18, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Optional: I think the changelog usually links to issues, not PRs, so this link could probably be omited.

dconeybe Jun 21, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

It looks like there is a mixture of issues and PRs linked to in previous change log entries. I see your point but I personally feel like adding a link to additional context is useful for future readers. If this issue had been discovered externally then there would be an issue number; however, since it was discovered by me, a developer on the Firestore team, no issue was created. IMO, providing this link adds value at no cost. If you feel strongly I can remove it, but I like having it here.

### 8.0.0
- Changes
- Analytics: Removed `SetCurrentScreen()` following its removal from iOS SDK
Expand Down
Back | FazBrowse Home | New Git URL