-
Notifications
You must be signed in to change notification settings - Fork 163
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
Build with -Werror on CI #7646
Build with -Werror on CI #7646
Conversation
@@ -114,7 +114,7 @@ std::optional<CollectionType> process_collection(const Property& property) | |||
else if (is_dictionary(property.type)) { | |||
return CollectionType::Dictionary; | |||
} | |||
return {}; | |||
return std::nullopt; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is working around a bug in GCC 9 which was fixed in 10.
@@ -142,7 +142,7 @@ void SyncManager::do_make_logger() | |||
REALM_ASSERT(m_logger_ptr); | |||
} | |||
|
|||
const std::shared_ptr<util::Logger>& SyncManager::get_logger() const | |||
std::shared_ptr<util::Logger> SyncManager::get_logger() const |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Returning a reference here exposed the field which was supposed to be guarded by the mutex.
@@ -3091,7 +3091,7 @@ TEMPLATE_TEST_CASE("client reset collections of links", "[sync][pbs][client rese | |||
{Remove{dest_pk_1}, Remove{dest_pk_2}}, {dest_pk_3, dest_pk_4}); | |||
} | |||
} | |||
else if constexpr (test_type_is_set) { | |||
if constexpr (test_type_is_set) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the else
here, GCC determined that this if
was unreachable without ever evaluating test_type_is_set
and so produced an unused variable warning. This is IMO a bug in GCC, but removing the else doesn't change anything.
1755db2
to
d010fad
Compare
Pull Request Test Coverage Report for Build thomas.goyne_324Details
💛 - Coveralls |
We used to have the Jenkins error parser that made the job fail if there were any warnings, but now we have nothing and warnings can slip through. This is a little worse as it makes the build fail immediately rather than letting it run the tests, but it's better than nothing.
We used to have the Jenkins error parser that made the job fail if there were any warnings, but now we have nothing and warnings can slip through. This is a little worse as it makes the build fail immediately rather than letting it run the tests, but it's better than nothing.