checkapi -= new field defaults for unions
I don't think this requirement is intentional,
it's for parcelables.
A new server sending a new union field to an old server
is rejected. An old server couldn't send the new type,
so I don't think there is any need for the default
field for unions.
Bug: N/A (email thread)
Test: N/A
Change-Id: I4227795f14f8d6ce569fa3f5432ddaad5494a44b
diff --git a/aidl_checkapi.cpp b/aidl_checkapi.cpp
index 039c01b..a18a8f8 100644
--- a/aidl_checkapi.cpp
+++ b/aidl_checkapi.cpp
@@ -302,57 +302,60 @@
}
}
- for (size_t i = old_fields.size(); i < new_fields.size(); i++) {
- const auto& new_field = new_fields.at(i);
- if (new_field->HasUsefulDefaultValue()) {
- continue;
- }
-
- // enum can't be nullable, but it's okay if it has 0 as a valid enumerator.
- if (const auto& enum_decl = new_types.GetEnumDeclaration(new_field->GetType());
- enum_decl != nullptr) {
- if (HasZeroEnumerator(*enum_decl)) {
+ // New fields must have default values.
+ if (older.AsUnionDeclaration() == nullptr) {
+ for (size_t i = old_fields.size(); i < new_fields.size(); i++) {
+ const auto& new_field = new_fields.at(i);
+ if (new_field->HasUsefulDefaultValue()) {
continue;
}
- // TODO(b/142893595): Rephrase the message: "provide a default value or make sure ..."
- AIDL_ERROR(new_field) << "Field '" << new_field->GetName() << "' of enum '"
- << enum_decl->GetName()
- << "' can't be initialized as '0'. Please make sure '"
- << enum_decl->GetName() << "' has '0' as a valid value.";
- compatible = false;
- continue;
- }
+ // enum can't be nullable, but it's okay if it has 0 as a valid enumerator.
+ if (const auto& enum_decl = new_types.GetEnumDeclaration(new_field->GetType());
+ enum_decl != nullptr) {
+ if (HasZeroEnumerator(*enum_decl)) {
+ continue;
+ }
- // Old API versions may suffer from the issue presented here. There is
- // only a finite number in Android, which we must allow indefinitely.
- struct HistoricalException {
- std::string canonical;
- std::string field;
- };
- static std::vector<HistoricalException> exceptions = {
- {"android.net.DhcpResultsParcelable", "serverHostName"},
- {"android.net.ResolverParamsParcel", "resolverOptions"},
- };
- bool excepted = false;
- for (const HistoricalException& exception : exceptions) {
- if (older.GetCanonicalName() == exception.canonical &&
- new_field->GetName() == exception.field) {
- excepted = true;
- break;
+ // TODO(b/142893595): Rephrase the message: "provide a default value or make sure ..."
+ AIDL_ERROR(new_field) << "Field '" << new_field->GetName() << "' of enum '"
+ << enum_decl->GetName()
+ << "' can't be initialized as '0'. Please make sure '"
+ << enum_decl->GetName() << "' has '0' as a valid value.";
+ compatible = false;
+ continue;
}
- }
- if (excepted) continue;
- AIDL_ERROR(new_field)
- << "Field '" << new_field->GetName()
- << "' does not have a useful default in some backends. Please either provide a default "
- "value for this field or mark the field as @nullable. This value or a null value will "
- "be used automatically when an old version of this parcelable is sent to a process "
- "which understands a new version of this parcelable. In order to make sure your code "
- "continues to be backwards compatible, make sure the default or null value does not "
- "cause a semantic change to this parcelable.";
- compatible = false;
+ // Old API versions may suffer from the issue presented here. There is
+ // only a finite number in Android, which we must allow indefinitely.
+ struct HistoricalException {
+ std::string canonical;
+ std::string field;
+ };
+ static std::vector<HistoricalException> exceptions = {
+ {"android.net.DhcpResultsParcelable", "serverHostName"},
+ {"android.net.ResolverParamsParcel", "resolverOptions"},
+ };
+ bool excepted = false;
+ for (const HistoricalException& exception : exceptions) {
+ if (older.GetCanonicalName() == exception.canonical &&
+ new_field->GetName() == exception.field) {
+ excepted = true;
+ break;
+ }
+ }
+ if (excepted) continue;
+
+ AIDL_ERROR(new_field)
+ << "Field '" << new_field->GetName()
+ << "' does not have a useful default in some backends. Please either provide a default "
+ "value for this field or mark the field as @nullable. This value or a null value will "
+ "be used automatically when an old version of this parcelable is sent to a process "
+ "which understands a new version of this parcelable. In order to make sure your code "
+ "continues to be backwards compatible, make sure the default or null value does not "
+ "cause a semantic change to this parcelable.";
+ compatible = false;
+ }
}
compatible = are_compatible_constants(older, newer) && compatible;
