diff options
author | Jooyung Han <jooyung@google.com> | 2022-12-21 16:53:56 +0900 |
---|---|---|
committer | Jooyung Han <jooyung@google.com> | 2022-12-22 03:45:48 +0000 |
commit | 833fbf46f949dbb450e715dbdfe1411f56aa5d3a (patch) | |
tree | 0e8831f69adacd00b0250922daa3198270cc3773 | |
parent | 4449a1fc4e9f7122a33b132bf1fec0891dc1b1dd (diff) | |
download | aidl-833fbf46f949dbb450e715dbdfe1411f56aa5d3a.tar.gz |
--dumpapi doesn't inline constants
Due to constant inlining, some AIDL types referencing other type's
constants lose the type references in the snapshot. This causes CPP/NDK
clients relying on the fact of including headers of referenced types
break after *-freeze-api.
(See the REPRO in Iab358e15d4400fb43ec9d2812ac37c32af127323)
Now, constant references are not inlined when creating API dumps.
Bug: 262594867
Test: aidl_unittests
Test: m client-using-test-piece-3 # OK
m test-piece-3-freeze-api # OK
m client-using-test-piece-3 # OK
Change-Id: I4e8d8e03c70c6168942bcd95b7db39c29c65aeea
-rw-r--r-- | aidl_checkapi.cpp | 2 | ||||
-rw-r--r-- | aidl_dumpapi.cpp | 49 | ||||
-rw-r--r-- | aidl_dumpapi.h | 8 | ||||
-rw-r--r-- | aidl_language.h | 7 | ||||
-rw-r--r-- | aidl_unittest.cpp | 12 |
5 files changed, 66 insertions, 12 deletions
diff --git a/aidl_checkapi.cpp b/aidl_checkapi.cpp index c1d3c66b..584e5a66 100644 --- a/aidl_checkapi.cpp +++ b/aidl_checkapi.cpp @@ -44,7 +44,7 @@ using std::vector; static std::string Dump(const AidlDefinedType& type) { string code; CodeWriterPtr out = CodeWriter::ForString(&code); - DumpVisitor visitor(*out); + DumpVisitor visitor(*out, /*inline_constants=*/true); type.DispatchVisit(visitor); out->Close(); return code; diff --git a/aidl_dumpapi.cpp b/aidl_dumpapi.cpp index c1450813..df5be2fd 100644 --- a/aidl_dumpapi.cpp +++ b/aidl_dumpapi.cpp @@ -97,7 +97,11 @@ void DumpVisitor::DumpAnnotations(const AidlAnnotatable& a) { } void DumpVisitor::DumpConstantValue(const AidlTypeSpecifier& type, const AidlConstantValue& c) { - out << c.ValueString(type, AidlConstantValueDecorator); + if (inline_constants) { + out << c.ValueString(type, AidlConstantValueDecorator); + } else { + c.DispatchVisit(*this); + } } void DumpVisitor::Visit(const AidlInterface& t) { @@ -126,8 +130,11 @@ void DumpVisitor::Visit(const AidlEnumDeclaration& t) { out.Indent(); for (const auto& e : t.GetEnumerators()) { DumpComments(*e); - out << e->GetName() << " = "; - DumpConstantValue(t.GetBackingType(), *e->GetValue()); + out << e->GetName(); + if (e->IsValueUserSpecified() || inline_constants) { + out << " = "; + DumpConstantValue(t.GetBackingType(), *e->GetValue()); + } out << ",\n"; } out.Dedent(); @@ -173,6 +180,40 @@ void DumpVisitor::Visit(const AidlTypeSpecifier& t) { out << t.ToString(); } +// These Visit() methods are not invoked when inline_constants = true +void DumpVisitor::Visit(const AidlConstantValue& c) { + AIDL_FATAL_IF(inline_constants, AIDL_LOCATION_HERE); + out << c.Literal(); +} + +void DumpVisitor::Visit(const AidlConstantReference& r) { + AIDL_FATAL_IF(inline_constants, AIDL_LOCATION_HERE); + if (auto& ref = r.GetRefType(); ref) { + ref->DispatchVisit(*this); + out << "."; + } + out << r.GetFieldName(); +} + +void DumpVisitor::Visit(const AidlBinaryConstExpression& b) { + AIDL_FATAL_IF(inline_constants, AIDL_LOCATION_HERE); + // TODO(b/262594867) put parentheses only when necessary + out << "("; + b.Left()->DispatchVisit(*this); + out << " " << b.Op() << " "; + b.Right()->DispatchVisit(*this); + out << ")"; +} + +void DumpVisitor::Visit(const AidlUnaryConstExpression& u) { + AIDL_FATAL_IF(inline_constants, AIDL_LOCATION_HERE); + // TODO(b/262594867) put parentheses only when necessary + out << "("; + out << u.Op(); + u.Val()->DispatchVisit(*this); + out << ")"; +} + static string GetApiDumpPathFor(const AidlDefinedType& defined_type, const Options& options) { string package_as_path = Join(Split(defined_type.GetPackage(), "."), OS_PATH_SEPARATOR); AIDL_FATAL_IF(options.OutputDir().empty() || options.OutputDir().back() != '/', defined_type); @@ -209,7 +250,7 @@ bool dump_api(const Options& options, const IoDelegate& io_delegate) { if (!type->GetPackage().empty()) { (*writer) << "package " << type->GetPackage() << ";\n"; } - DumpVisitor visitor(*writer); + DumpVisitor visitor(*writer, /*inline_constants=*/false); type->DispatchVisit(visitor); } } else { diff --git a/aidl_dumpapi.h b/aidl_dumpapi.h index df8278a4..b4c146cf 100644 --- a/aidl_dumpapi.h +++ b/aidl_dumpapi.h @@ -22,7 +22,9 @@ namespace aidl { struct DumpVisitor : AidlVisitor { CodeWriter& out; - DumpVisitor(CodeWriter& out) : out(out) {} + bool inline_constants; + DumpVisitor(CodeWriter& out, bool inline_constants) + : out(out), inline_constants(inline_constants) {} void DumpType(const AidlDefinedType& dt, const string& type); void DumpMembers(const AidlDefinedType& dt); @@ -39,6 +41,10 @@ struct DumpVisitor : AidlVisitor { void Visit(const AidlVariableDeclaration& v) override; void Visit(const AidlConstantDeclaration& c) override; void Visit(const AidlTypeSpecifier& t) override; + void Visit(const AidlConstantValue& c) override; + void Visit(const AidlConstantReference& r) override; + void Visit(const AidlBinaryConstExpression& b) override; + void Visit(const AidlUnaryConstExpression& u) override; }; bool dump_api(const Options& options, const IoDelegate& io_delegate); diff --git a/aidl_language.h b/aidl_language.h index 9cec5704..e5e97748 100644 --- a/aidl_language.h +++ b/aidl_language.h @@ -727,7 +727,7 @@ class AidlConstantValue : public AidlNode { }; // Represents "<type>.<field>" which resolves to a constant which is one of -// - constant declartion +// - constant declaration // - enumerator // When a <type> is missing, <field> is of the enclosing type. class AidlConstantReference : public AidlConstantValue { @@ -765,6 +765,8 @@ class AidlUnaryConstExpression : public AidlConstantValue { traverse(*unary_); } void DispatchVisit(AidlVisitor& v) const override { v.Visit(*this); } + const std::unique_ptr<AidlConstantValue>& Val() const { return unary_; } + const std::string& Op() const { return op_; } private: bool evaluate() const override; @@ -790,6 +792,9 @@ class AidlBinaryConstExpression : public AidlConstantValue { traverse(*right_val_); } void DispatchVisit(AidlVisitor& v) const override { v.Visit(*this); } + const std::unique_ptr<AidlConstantValue>& Left() const { return left_val_; } + const std::unique_ptr<AidlConstantValue>& Right() const { return right_val_; } + const std::string& Op() const { return op_; } private: bool evaluate() const override; diff --git a/aidl_unittest.cpp b/aidl_unittest.cpp index 9dedca3c..7dc83aa0 100644 --- a/aidl_unittest.cpp +++ b/aidl_unittest.cpp @@ -2478,8 +2478,8 @@ TEST_F(AidlTest, ApiDumpWithEnums) { EXPECT_TRUE(io_delegate_.GetWrittenContents("dump/foo/bar/Enum.aidl", &actual)); EXPECT_EQ(string(kPreamble).append("package foo.bar;\n" "enum Enum {\n" - " FOO = 0,\n" - " BAR = 1,\n" + " FOO,\n" + " BAR = (FOO + 1),\n" "}\n"), actual); } @@ -2495,6 +2495,7 @@ TEST_F(AidlTest, ApiDumpWithEnumDefaultValues) { "import foo.bar.Enum;\n" "parcelable Foo {\n" " Enum e = Enum.FOO;\n" + " int n = Enum.FOO;\n" "}\n"); vector<string> args = {"aidl", "--dumpapi", "-I . ", "-o dump", "foo/bar/Foo.aidl"}; @@ -2507,6 +2508,7 @@ TEST_F(AidlTest, ApiDumpWithEnumDefaultValues) { EXPECT_EQ(string(kPreamble).append("package foo.bar;\n" "parcelable Foo {\n" " foo.bar.Enum e = foo.bar.Enum.FOO;\n" + " int n = foo.bar.Enum.FOO;\n" "}\n"), actual); } @@ -4981,7 +4983,7 @@ parcelable Foo { EXPECT_EQ("int e = 3", fields[0]->ToString()); } -TEST_P(AidlTest, EnumeratorIsConstantValue_CanDefineOtherEnumerator) { +TEST_F(AidlTest, EnumeratorIsConstantValue_CanDefineOtherEnumerator) { CaptureStderr(); const AidlDefinedType* type = Parse("a/p/Foo.aidl", R"( package a.p; @@ -4992,14 +4994,14 @@ enum Foo { STANDARD_BT601_625 = 2 << STANDARD_SHIFT, } )", - typenames_, GetLanguage()); + typenames_, Options::Language::JAVA); auto err = GetCapturedStderr(); EXPECT_EQ("", err); ASSERT_NE(type, nullptr); const auto& enum_type = type->AsEnumDeclaration(); string code; auto writer = CodeWriter::ForString(&code); - DumpVisitor visitor(*writer); + DumpVisitor visitor(*writer, /*inline_constants=*/true); visitor.Visit(*enum_type); writer->Close(); EXPECT_EQ(R"--(@Backing(type="int") |