checkapi: use ValueString to compare const values
Comparison based on Literal() was a work around because checkapi could't
rely on type/ref resolution. Now, checkapi can use more robust
ValueString() to compare const values.
This reverts 02a11bee998bda59cd420e2c8dd57f3684ee288a and
fdaae1d5ff8601ba0c22d3ebda463a68f85dfded partially.
Bug: n/a
Test: aidl_unittests
Change-Id: I223843cdfe44b7f129b28cf1e4c1d313225db4e6
diff --git a/aidl_checkapi.cpp b/aidl_checkapi.cpp
index 91ed49a..ae3b63b 100644
--- a/aidl_checkapi.cpp
+++ b/aidl_checkapi.cpp
@@ -41,18 +41,10 @@
using std::string;
using std::vector;
-struct DumpForEqualityVisitor : DumpVisitor {
- DumpForEqualityVisitor(CodeWriter& out) : DumpVisitor(out) {}
-
- void DumpConstantValue(const AidlTypeSpecifier&, const AidlConstantValue& c) {
- out << c.Literal();
- }
-};
-
static std::string Dump(const AidlDefinedType& type) {
string code;
CodeWriterPtr out = CodeWriter::ForString(&code);
- DumpForEqualityVisitor visitor(*out);
+ DumpVisitor visitor(*out);
type.DispatchVisit(visitor);
out->Close();
return code;
@@ -154,8 +146,8 @@
const auto new_c = found->second;
compatible &= are_compatible_types(old_c->GetType(), new_c->GetType());
- const string old_value = old_c->GetValue().Literal();
- const string new_value = new_c->GetValue().Literal();
+ const string old_value = old_c->ValueString(AidlConstantValueDecorator);
+ const string new_value = new_c->ValueString(AidlConstantValueDecorator);
if (old_value != new_value) {
AIDL_ERROR(newer) << "Changed constant value: " << older.GetCanonicalName() << "."
<< old_c->GetName() << " from " << old_value << " to " << new_value << ".";
@@ -227,21 +219,14 @@
static bool HasZeroEnumerator(const AidlEnumDeclaration& enum_decl) {
return std::any_of(enum_decl.GetEnumerators().begin(), enum_decl.GetEnumerators().end(),
[&](const unique_ptr<AidlEnumerator>& enumerator) {
- return enumerator->GetValue()->Literal() == "0";
+ return enumerator->GetValue()->ValueString(
+ enum_decl.GetBackingType(), AidlConstantValueDecorator) == "0";
});
}
-static bool EvaluatesToZero(const AidlEnumDeclaration& enum_decl, const std::string& value) {
- if (value == "") return true;
- // Because --check_api runs with "valid" AIDL definitions, we can safely assume that
- // the value is formatted as <scope>.<enumerator>.
- auto enumerator_name = value.substr(value.find_last_of('.') + 1);
- for (const auto& enumerator : enum_decl.GetEnumerators()) {
- if (enumerator->GetName() == enumerator_name) {
- return enumerator->GetValue()->Literal() == "0";
- }
- }
- AIDL_FATAL(enum_decl) << "Can't find " << enumerator_name << " in " << enum_decl.GetName();
+static bool EvaluatesToZero(const AidlEnumDeclaration& enum_decl, const AidlConstantValue* value) {
+ if (value == nullptr) return true;
+ return value->ValueString(enum_decl.GetBackingType(), AidlConstantValueDecorator) == "0";
}
static bool are_compatible_parcelables(const AidlDefinedType& older, const AidlTypenames&,
@@ -277,15 +262,14 @@
const auto& new_field = new_fields.at(i);
compatible &= are_compatible_types(old_field->GetType(), new_field->GetType());
- string old_value = old_field->GetDefaultValue() ? old_field->GetDefaultValue()->Literal() : "";
- string new_value = new_field->GetDefaultValue() ? new_field->GetDefaultValue()->Literal() : "";
-
+ const string old_value = old_field->ValueString(AidlConstantValueDecorator);
+ const string new_value = new_field->ValueString(AidlConstantValueDecorator);
if (old_value == new_value) {
continue;
}
// For enum type fields, we accept setting explicit default value which is "zero"
auto enum_decl = new_types.GetEnumDeclaration(new_field->GetType());
- if (old_value == "" && enum_decl && EvaluatesToZero(*enum_decl, new_value)) {
+ if (old_value == "" && enum_decl && EvaluatesToZero(*enum_decl, new_field->GetDefaultValue())) {
continue;
}
@@ -390,8 +374,10 @@
compatible = false;
continue;
}
- const string old_value = old_enum_map[name]->Literal();
- const string new_value = new_enum_map[name]->Literal();
+ const string old_value =
+ old_enum_map[name]->ValueString(older.GetBackingType(), AidlConstantValueDecorator);
+ const string new_value =
+ new_enum_map[name]->ValueString(newer.GetBackingType(), AidlConstantValueDecorator);
if (old_value != new_value) {
AIDL_ERROR(newer) << "Changed enumerator value: " << older.GetCanonicalName() << "::" << name
<< " from " << old_value << " to " << new_value << ".";
diff --git a/aidl_dumpapi.h b/aidl_dumpapi.h
index 6d000dd..f30c54a 100644
--- a/aidl_dumpapi.h
+++ b/aidl_dumpapi.h
@@ -28,7 +28,7 @@
void DumpMembers(const AidlDefinedType& dt);
void DumpComments(const AidlCommentable& c);
void DumpAnnotations(const AidlAnnotatable& a);
- virtual void DumpConstantValue(const AidlTypeSpecifier& type, const AidlConstantValue& c);
+ void DumpConstantValue(const AidlTypeSpecifier& type, const AidlConstantValue& c);
void Visit(const AidlInterface& t) override;
void Visit(const AidlParcelable& t) override;
diff --git a/aidl_unittest.cpp b/aidl_unittest.cpp
index 196e396..0218561 100644
--- a/aidl_unittest.cpp
+++ b/aidl_unittest.cpp
@@ -3757,6 +3757,20 @@
err);
}
+TEST_F(AidlTest, EnumDefaultShouldBeEnumerators_RejectsNumericValue) {
+ io_delegate_.SetFileContents("a/p/Enum.aidl", "package p; enum Enum { FOO = 1, BAR = 2}");
+ io_delegate_.SetFileContents("a/p/Foo.aidl", R"(
+package p;
+import p.Enum;
+parcelable Foo {
+ Enum e = 1;
+})");
+ CaptureStderr();
+ auto options = Options::From("aidl -I a --lang java -o out -h out a/p/Foo.aidl");
+ EXPECT_EQ(1, aidl::compile_aidl(options, io_delegate_));
+ EXPECT_THAT(GetCapturedStderr(), HasSubstr("Invalid value (1) for enum p.Enum"));
+}
+
TEST_P(AidlTest, DefaultWithEmptyArray) {
io_delegate_.SetFileContents("a/p/Foo.aidl", "package p; parcelable Foo { p.Bar[] bars = {}; }");
io_delegate_.SetFileContents("a/p/Bar.aidl", "package p; parcelable Bar { }");