refactor: make "Autofill" explicit
Extracted "Autofill" (setting backing-type for enums) into a separate
method which is called by AidlTypenames::Autofill().
Now the processing steps are as follows: (see diagnostics_unittest.cpp)
- Parser::Parse(): load a doc into AST
- parser.Resolve(): type/reference resolution(link references to targets)
- typenames.Autofill(): fill or fix AST to be valid
- CheckValid: is AST valid?
- .. further processing ..
Bug: none
Test: m aidl_unittests
Change-Id: I70117aa6df8301b8665194f4a0466df1f271aa67
diff --git a/aidl.cpp b/aidl.cpp
index d223ac3..ca84533 100644
--- a/aidl.cpp
+++ b/aidl.cpp
@@ -531,26 +531,8 @@
return AidlError::BAD_TYPE;
}
- typenames->IterateTypes([&](const AidlDefinedType& type) {
- AidlEnumDeclaration* enum_decl = const_cast<AidlEnumDeclaration*>(type.AsEnumDeclaration());
- if (enum_decl != nullptr) {
- // BackingType is filled in for all known enums, including imported enums,
- // because other types that may use enums, such as Interface or
- // StructuredParcelable, need to know the enum BackingType when
- // generating code.
- if (auto backing_type = enum_decl->BackingType(*typenames); backing_type != nullptr) {
- enum_decl->SetBackingType(std::unique_ptr<const AidlTypeSpecifier>(backing_type));
- } else {
- // Default to byte type for enums.
- auto byte_type =
- std::make_unique<AidlTypeSpecifier>(AIDL_LOCATION_HERE, "byte", false, nullptr, "");
- byte_type->Resolve(*typenames);
- enum_decl->SetBackingType(std::move(byte_type));
- }
- }
- });
- if (err != AidlError::OK) {
- return err;
+ if (!typenames->Autofill()) {
+ return AidlError::BAD_TYPE;
}
//////////////////////////////////////////////////////////////////////////
diff --git a/aidl_language.cpp b/aidl_language.cpp
index 256af2c..3de3e06 100644
--- a/aidl_language.cpp
+++ b/aidl_language.cpp
@@ -342,25 +342,8 @@
return GetAnnotation(annotations_, AidlAnnotation::Type::RUST_DERIVE);
}
-const AidlTypeSpecifier* AidlAnnotatable::BackingType(const AidlTypenames& typenames) const {
- auto annotation = GetAnnotation(annotations_, AidlAnnotation::Type::BACKING);
- if (annotation != nullptr) {
- auto annotation_params = annotation->AnnotationParams(AidlConstantValueDecorator);
- if (auto it = annotation_params.find("type"); it != annotation_params.end()) {
- const string& type = it->second;
-
- AIDL_FATAL_IF(type.size() < 2, this) << type;
- AIDL_FATAL_IF(type[0] != '"', this) << type;
- AIDL_FATAL_IF(type[type.length() - 1] != '"', this) << type;
- string unquoted_type = type.substr(1, type.length() - 2);
-
- AidlTypeSpecifier* type_specifier =
- new AidlTypeSpecifier(annotation->GetLocation(), unquoted_type, false, nullptr, "");
- type_specifier->Resolve(typenames);
- return type_specifier;
- }
- }
- return nullptr;
+const AidlAnnotation* AidlAnnotatable::BackingType() const {
+ return GetAnnotation(annotations_, AidlAnnotation::Type::BACKING);
}
bool AidlAnnotatable::IsStableApiParcelable(Options::Language lang) const {
@@ -1221,14 +1204,10 @@
const std::string& package, const std::string& comments)
: AidlDefinedType(location, name, comments, package, nullptr),
enumerators_(std::move(*enumerators)) {
- Autofill();
-}
-
-void AidlEnumDeclaration::SetBackingType(std::unique_ptr<const AidlTypeSpecifier> type) {
- backing_type_ = std::move(type);
-}
-
-void AidlEnumDeclaration::Autofill() {
+ // Fill missing enumerator values with <prev + 1>
+ // This can't be done in Autofill() because type/ref resolution depends on this.
+ // For example, with enum E { A, B = A }, B's value 'A' is a reference which can't be
+ // resolved if A has no value set.
const AidlEnumerator* previous = nullptr;
for (const auto& enumerator : enumerators_) {
if (enumerator->GetValue() == nullptr) {
@@ -1247,6 +1226,36 @@
}
}
+bool AidlEnumDeclaration::Autofill(const AidlTypenames& typenames) {
+ if (auto annot = BackingType(); annot != nullptr) {
+ // Autofill() is called before the grand CheckValid(). But AidlAnnotation::AnnotationParams()
+ // calls AidlConstantValue::ValueString() which requires CheckValid() to be called before. So we
+ // need to call CheckValid().
+ if (!annot->CheckValid()) {
+ return false;
+ }
+ auto annotation_params = annot->AnnotationParams(AidlConstantValueDecorator);
+ auto type = annotation_params.at("type");
+
+ AIDL_FATAL_IF(type.size() < 2, this) << type;
+ AIDL_FATAL_IF(type[0] != '"', this) << type;
+ AIDL_FATAL_IF(type[type.length() - 1] != '"', this) << type;
+ string unquoted_type = type.substr(1, type.length() - 2);
+
+ backing_type_ = std::make_unique<AidlTypeSpecifier>(annot->GetLocation(), unquoted_type, false,
+ nullptr, "");
+ } else {
+ // Default to byte type for enums.
+ backing_type_ =
+ std::make_unique<AidlTypeSpecifier>(AIDL_LOCATION_HERE, "byte", false, nullptr, "");
+ }
+ // Autofill() is called after type resolution, we resolve the backing type manually.
+ if (!backing_type_->Resolve(typenames)) {
+ AIDL_ERROR(this) << "Invalid backing type: " << backing_type_->GetName();
+ }
+ return true;
+}
+
std::set<AidlAnnotation::Type> AidlEnumDeclaration::GetSupportedAnnotations() const {
return {AidlAnnotation::Type::VINTF_STABILITY, AidlAnnotation::Type::BACKING,
AidlAnnotation::Type::HIDE, AidlAnnotation::Type::JAVA_PASSTHROUGH};
diff --git a/aidl_language.h b/aidl_language.h
index f19ef24..c4a61fc 100644
--- a/aidl_language.h
+++ b/aidl_language.h
@@ -256,7 +256,7 @@
const AidlAnnotation* UnsupportedAppUsage() const;
const AidlAnnotation* RustDerive() const;
- const AidlTypeSpecifier* BackingType(const AidlTypenames& typenames) const;
+ const AidlAnnotation* BackingType() const;
// ToString is for dumping AIDL.
// Returns string representation of annotations.
@@ -973,7 +973,7 @@
AidlEnumDeclaration& operator=(const AidlEnumDeclaration&) = delete;
AidlEnumDeclaration& operator=(AidlEnumDeclaration&&) = delete;
- void SetBackingType(std::unique_ptr<const AidlTypeSpecifier> type);
+ bool Autofill(const AidlTypenames&);
const AidlTypeSpecifier& GetBackingType() const { return *backing_type_; }
const std::vector<std::unique_ptr<AidlEnumerator>>& GetEnumerators() const {
return enumerators_;
@@ -990,11 +990,10 @@
const AidlEnumDeclaration* AsEnumDeclaration() const override { return this; }
private:
- void Autofill();
const std::string name_;
const std::vector<std::unique_ptr<AidlEnumerator>> enumerators_;
- std::unique_ptr<const AidlTypeSpecifier> backing_type_;
+ std::unique_ptr<AidlTypeSpecifier> backing_type_;
};
class AidlUnionDecl : public AidlParcelable {
diff --git a/aidl_typenames.cpp b/aidl_typenames.cpp
index 2e77976..109f99a 100644
--- a/aidl_typenames.cpp
+++ b/aidl_typenames.cpp
@@ -352,5 +352,21 @@
}
}
+bool AidlTypenames::Autofill() const {
+ bool success = true;
+ IterateTypes([&](const AidlDefinedType& type) {
+ // BackingType is filled in for all known enums, including imported enums,
+ // because other types that may use enums, such as Interface or
+ // StructuredParcelable, need to know the enum BackingType when
+ // generating code.
+ if (auto enum_decl = const_cast<AidlDefinedType&>(type).AsEnumDeclaration(); enum_decl) {
+ if (!enum_decl->Autofill(*this)) {
+ success = false;
+ }
+ }
+ });
+ return success;
+}
+
} // namespace aidl
} // namespace android
diff --git a/aidl_typenames.h b/aidl_typenames.h
index 2a6973f..9ba5745 100644
--- a/aidl_typenames.h
+++ b/aidl_typenames.h
@@ -90,6 +90,8 @@
const AidlParcelable* GetParcelable(const AidlTypeSpecifier& type) const;
// Iterates over all defined and then preprocessed types
void IterateTypes(const std::function<void(const AidlDefinedType&)>& body) const;
+ // Fixes AST after type/ref resolution before validation
+ bool Autofill() const;
private:
struct DefinedImplResult {
diff --git a/aidl_unittest.cpp b/aidl_unittest.cpp
index a3de25f..2ee75df 100644
--- a/aidl_unittest.cpp
+++ b/aidl_unittest.cpp
@@ -3039,8 +3039,6 @@
AidlError error;
const string expected_stderr =
"ERROR: p/TestEnum.aidl:2.1-51: Parameter foo not supported for annotation Backing. It must "
- "be one of: type\n"
- "ERROR: p/TestEnum.aidl:2.1-51: Parameter foo not supported for annotation Backing. It must "
"be one of: type\n";
CaptureStderr();
EXPECT_EQ(nullptr, Parse("p/TestEnum.aidl",