const-ref: support recursive references
Referenced constant expression can also refer another constant
expressions.
parcelable P {
const int A = B + 1;
const int B = C + 1;
const int C = 1;
}
This works with imported types.
parcelable P { const int A = Q.A + 1; }
parcelable Q { const int A = 1; }
Using this, "auto-fill" of enums got simplified.
enum E { A, B } == enum E { A = 0, B = A + 1 }
Note that circular references are not supported.
parcelable P { const int A = A + 1; } // error
Bug: 142893595
Bug: 174877216
Test: aidl_unittests
Change-Id: Ib187ec47c0184effd64568a9a3d57a2adf5aa4f4
diff --git a/aidl.cpp b/aidl.cpp
index b9ac26f..0f894c2 100644
--- a/aidl.cpp
+++ b/aidl.cpp
@@ -447,51 +447,25 @@
vector<string> import_paths;
ImportResolver import_resolver{io_delegate, input_file_name, options.ImportDirs(),
options.InputFiles()};
-
- vector<string> type_from_import_statements;
for (const auto& import : main_parser->ParsedDocument().Imports()) {
- if (!AidlTypenames::IsBuiltinTypename(import->GetNeededClass())) {
- type_from_import_statements.emplace_back(import->GetNeededClass());
+ if (AidlTypenames::IsBuiltinTypename(import->GetNeededClass())) {
+ continue;
}
- }
-
- // When referencing a type using fully qualified name it should be imported
- // without the import statement. To support that, add all unresolved
- // typespecs encountered during the parsing to the import_candidates list.
- // Note that there is no guarantee that the typespecs are all fully qualified.
- // It will be determined by calling FindImportFile().
- set<string> unresolved_types;
- for (const auto type : main_parser->GetUnresolvedTypespecs()) {
- if (!AidlTypenames::IsBuiltinTypename(type->GetName())) {
- unresolved_types.emplace(type->GetName());
- }
- }
- vector<string> import_candidates(type_from_import_statements);
- import_candidates.insert(import_candidates.end(), unresolved_types.begin(),
- unresolved_types.end());
- for (const auto& import : import_candidates) {
- if (typenames->IsIgnorableImport(import)) {
+ if (typenames->IsIgnorableImport(import->GetNeededClass())) {
// There are places in the Android tree where an import doesn't resolve,
// but we'll pick the type up through the preprocessed types.
// This seems like an error, but legacy support demands we support it...
continue;
}
- string import_path = import_resolver.FindImportFile(import);
+ string import_path = import_resolver.FindImportFile(import->GetNeededClass());
if (import_path.empty()) {
- if (typenames->ResolveTypename(import).is_resolved) {
- // Couldn't find the *.aidl file for the type from the include paths, but we
- // have the type already resolved. This could happen when the type is
- // from the preprocessed aidl file. In that case, use the type from the
- // preprocessed aidl file as a last resort.
+ if (typenames->ResolveTypename(import->GetNeededClass()).is_resolved) {
+ // This could happen when the type is from the preprocessed aidl file.
+ // In that case, use the type from preprocessed aidl file
continue;
}
-
- if (std::find(type_from_import_statements.begin(), type_from_import_statements.end(),
- import) != type_from_import_statements.end()) {
- // Complain only when the import from the import statement has failed.
- AIDL_ERROR(input_file_name) << "Couldn't find import for class " << import;
- err = AidlError::BAD_IMPORT;
- }
+ AIDL_ERROR(input_file_name) << "Couldn't find import for class " << import->GetNeededClass();
+ err = AidlError::BAD_IMPORT;
continue;
}
@@ -521,10 +495,36 @@
if (err != AidlError::OK) {
return err;
}
+
+ TypeResolver resolver = [&](const AidlDocument* doc, AidlTypeSpecifier* type) {
+ if (type->Resolve(*typenames)) return true;
+
+ const string unresolved_name = type->GetUnresolvedName();
+ const std::optional<string> canonical_name = doc->ResolveName(unresolved_name);
+ if (!canonical_name) {
+ return false;
+ }
+ const string import_path = import_resolver.FindImportFile(*canonical_name);
+ if (import_path.empty()) {
+ return false;
+ }
+ import_paths.push_back(import_path);
+
+ std::unique_ptr<Parser> import_parser = Parser::Parse(import_path, io_delegate, *typenames);
+ if (import_parser == nullptr) {
+ AIDL_ERROR(import_path) << "error while importing " << import_path << " for " << import_path;
+ return false;
+ }
+ if (!type->Resolve(*typenames)) {
+ AIDL_ERROR(type) << "Can't resolve " << type->GetName();
+ return false;
+ }
+ return true;
+ };
const bool is_check_api = options.GetTask() == Options::Task::CHECK_API;
// Resolve the unresolved type references found from the input file
- if (!is_check_api && !main_parser->Resolve()) {
+ if (!is_check_api && !main_parser->Resolve(resolver)) {
// Resolution is not need for check api because all typespecs are
// using fully qualified names.
return AidlError::BAD_TYPE;
@@ -546,10 +546,6 @@
byte_type->Resolve(*typenames);
enum_decl->SetBackingType(std::move(byte_type));
}
-
- if (!enum_decl->Autofill()) {
- err = AidlError::BAD_TYPE;
- }
}
});
if (err != AidlError::OK) {
diff --git a/aidl_const_expressions.cpp b/aidl_const_expressions.cpp
index 10731ed..8e9ebea 100644
--- a/aidl_const_expressions.cpp
+++ b/aidl_const_expressions.cpp
@@ -440,7 +440,11 @@
AidlConstantValue* AidlConstantValue::Array(
const AidlLocation& location, std::unique_ptr<vector<unique_ptr<AidlConstantValue>>> values) {
AIDL_FATAL_IF(values == nullptr, location);
- return new AidlConstantValue(location, Type::ARRAY, std::move(values));
+ std::vector<std::string> str_values;
+ for (const auto& v : *values) {
+ str_values.push_back(v->value_);
+ }
+ return new AidlConstantValue(location, Type::ARRAY, std::move(values), Join(str_values, ", "));
}
AidlConstantValue* AidlConstantValue::String(const AidlLocation& location, const string& value) {
@@ -455,26 +459,6 @@
return new AidlConstantValue(location, Type::STRING, value);
}
-AidlConstantValue* AidlConstantValue::ShallowIntegralCopy(const AidlConstantValue& other) {
- // TODO(b/141313220) Perform a full copy instead of parsing+unparsing
- AidlTypeSpecifier type = AidlTypeSpecifier(AIDL_LOCATION_HERE, "long", false, nullptr, "");
- // TODO(b/142722772) CheckValid() should be called before ValueString()
- if (!other.CheckValid() || !other.evaluate(type)) {
- AIDL_ERROR(other) << "Failed to parse expression as integer: " << other.value_;
- return nullptr;
- }
- const std::string& value = other.ValueString(type, AidlConstantValueDecorator);
- if (value.empty()) {
- return nullptr; // error already logged
- }
-
- AidlConstantValue* result = Integral(AIDL_LOCATION_HERE, value);
- if (result == nullptr) {
- AIDL_FATAL(other) << "Unable to perform ShallowIntegralCopy.";
- }
- return result;
-}
-
string AidlConstantValue::ValueString(const AidlTypeSpecifier& type,
const ConstantValueDecorator& decorator) const {
if (type.IsGeneric()) {
@@ -500,7 +484,7 @@
const AidlEnumDeclaration* enum_type = defined_type->AsEnumDeclaration();
if (!enum_type) {
AIDL_ERROR(this) << "Invalid type (" << defined_type->GetCanonicalName()
- << ") for a const value(" << value_ << ")";
+ << ") for a const value (" << value_ << ")";
return "";
}
if (type_ != Type::REF) {
@@ -621,7 +605,6 @@
case Type::INT8: // fall-through
case Type::INT32: // fall-through
case Type::INT64: // fall-through
- case Type::ARRAY: // fall-through
case Type::CHARACTER: // fall-through
case Type::STRING: // fall-through
case Type::REF: // fall-through
@@ -630,6 +613,10 @@
case Type::BINARY:
is_valid_ = true;
break;
+ case Type::ARRAY:
+ is_valid_ = true;
+ for (const auto& v : values_) is_valid_ &= v->CheckValid();
+ break;
case Type::ERROR:
return false;
default:
@@ -763,111 +750,66 @@
}
}
-bool AidlConstantReference::CheckValid() const {
- if (is_evaluated_) return is_valid_;
- if (is_validating_) {
- AIDL_ERROR(*this) << "Can't evaluate the circular reference (" << value_ << ")";
- return false;
- }
- is_validating_ = true;
-
+const AidlConstantValue* AidlConstantReference::Resolve() {
+ if (resolved_) return resolved_;
if (!GetRefType() || !GetRefType()->GetDefinedType()) {
// This can happen when "const reference" is used in an unsupported way,
// but missed in checks there. It works as a safety net.
AIDL_ERROR(*this) << "Can't resolve the reference (" << value_ << ")";
- is_validating_ = false;
- is_valid_ = false;
- return false;
+ return nullptr;
}
auto defined_type = GetRefType()->GetDefinedType();
if (auto enum_decl = defined_type->AsEnumDeclaration(); enum_decl) {
for (const auto& e : enum_decl->GetEnumerators()) {
if (e->GetName() == field_name_) {
- is_valid_ = !e->GetValue() || e->GetValue()->CheckValid();
- is_validating_ = false;
- return is_valid_;
+ resolved_ = e->GetValue();
+ return resolved_;
}
}
} else {
for (const auto& c : defined_type->GetConstantDeclarations()) {
if (c->GetName() == field_name_) {
- is_valid_ = c->GetValue().CheckValid();
- is_validating_ = false;
- return is_valid_;
+ resolved_ = &c->GetValue();
+ return resolved_;
}
}
}
AIDL_ERROR(*this) << "Can't find " << field_name_ << " in " << ref_type_->GetName();
- is_valid_ = false;
- is_validating_ = false;
- return false;
+ return nullptr;
+}
+
+bool AidlConstantReference::CheckValid() const {
+ if (is_evaluated_) return is_valid_;
+ AIDL_FATAL_IF(!resolved_, this) << "Should be resolved first: " << value_;
+ is_valid_ = resolved_->CheckValid();
+ return is_valid_;
}
bool AidlConstantReference::evaluate(const AidlTypeSpecifier& type) const {
if (is_evaluated_) return is_valid_;
- if (is_evaluating_) {
- AIDL_ERROR(*this) << "Can't evaluate the circular reference (" << value_ << ")";
- return false;
- }
- is_evaluating_ = true;
-
+ AIDL_FATAL_IF(!resolved_, this) << "Should be resolved first: " << value_;
+ is_evaluated_ = true;
const AidlDefinedType* view_type = type.GetDefinedType();
if (view_type) {
auto enum_decl = view_type->AsEnumDeclaration();
if (!enum_decl) {
AIDL_ERROR(type) << "Can't refer to a constant expression: " << value_;
- is_evaluating_ = false;
- is_evaluated_ = true;
return false;
}
}
- auto defined_type = GetRefType()->GetDefinedType();
- if (auto enum_decl = defined_type->AsEnumDeclaration(); enum_decl) {
- for (const auto& e : enum_decl->GetEnumerators()) {
- if (e->GetName() == field_name_) {
- if (e->GetValue() && e->GetValue()->evaluate(type)) {
- is_valid_ = e->GetValue()->is_valid_;
- if (is_valid_) {
- final_type_ = e->GetValue()->final_type_;
- if (final_type_ == Type::STRING) {
- final_string_value_ = e->GetValue()->final_string_value_;
- } else {
- final_value_ = e->GetValue()->final_value_;
- }
- is_evaluating_ = false;
- is_evaluated_ = true;
- return true;
- }
- }
- break;
- }
- }
- } else {
- for (const auto& c : defined_type->GetConstantDeclarations()) {
- if (c->GetName() == field_name_) {
- if (c->GetValue().evaluate(type)) {
- is_valid_ = c->GetValue().is_valid_;
- if (is_valid_) {
- final_type_ = c->GetValue().final_type_;
- if (final_type_ == Type::STRING) {
- final_string_value_ = c->GetValue().final_string_value_;
- } else {
- final_value_ = c->GetValue().final_value_;
- }
- is_evaluating_ = false;
- is_evaluated_ = true;
- return true;
- }
- }
- }
+ resolved_->evaluate(type);
+ is_valid_ = resolved_->is_valid_;
+ final_type_ = resolved_->final_type_;
+ if (is_valid_) {
+ if (final_type_ == Type::STRING) {
+ final_string_value_ = resolved_->final_string_value_;
+ } else {
+ final_value_ = resolved_->final_value_;
}
}
- is_evaluating_ = false;
- is_evaluated_ = true;
- is_valid_ = false;
- return false;
+ return is_valid_;
}
bool AidlUnaryConstExpression::CheckValid() const {
@@ -1102,10 +1044,12 @@
}
AidlConstantValue::AidlConstantValue(const AidlLocation& location, Type type,
- std::unique_ptr<vector<unique_ptr<AidlConstantValue>>> values)
+ std::unique_ptr<vector<unique_ptr<AidlConstantValue>>> values,
+ const std::string& value)
: AidlNode(location),
type_(type),
values_(std::move(*values)),
+ value_(value),
is_valid_(false),
is_evaluated_(false),
final_type_(type) {
diff --git a/aidl_language.cpp b/aidl_language.cpp
index 38af0bc..4ab9fad 100644
--- a/aidl_language.cpp
+++ b/aidl_language.cpp
@@ -1145,7 +1145,11 @@
AidlEnumerator::AidlEnumerator(const AidlLocation& location, const std::string& name,
AidlConstantValue* value, const std::string& comments)
- : AidlNode(location), name_(name), value_(value), comments_(comments) {}
+ : AidlNode(location),
+ name_(name),
+ value_(value),
+ comments_(comments),
+ value_user_specified_(value != nullptr) {}
bool AidlEnumerator::CheckValid(const AidlTypeSpecifier& enum_backing_type) const {
if (GetValue() == nullptr) {
@@ -1170,33 +1174,31 @@
std::vector<std::unique_ptr<AidlEnumerator>>* enumerators,
const std::string& package, const std::string& comments)
: AidlDefinedType(location, name, comments, package, nullptr),
- enumerators_(std::move(*enumerators)) {}
+ enumerators_(std::move(*enumerators)) {
+ Autofill();
+}
void AidlEnumDeclaration::SetBackingType(std::unique_ptr<const AidlTypeSpecifier> type) {
backing_type_ = std::move(type);
}
-bool AidlEnumDeclaration::Autofill() {
+void AidlEnumDeclaration::Autofill() {
const AidlEnumerator* previous = nullptr;
for (const auto& enumerator : enumerators_) {
if (enumerator->GetValue() == nullptr) {
+ auto loc = enumerator->GetLocation();
if (previous == nullptr) {
enumerator->SetValue(
- std::unique_ptr<AidlConstantValue>(AidlConstantValue::Integral(GetLocation(), "0")));
+ std::unique_ptr<AidlConstantValue>(AidlConstantValue::Integral(loc, "0")));
} else {
- auto prev_value = std::unique_ptr<AidlConstantValue>(
- AidlConstantValue::ShallowIntegralCopy(*previous->GetValue()));
- if (prev_value == nullptr) {
- return false;
- }
+ auto prev_value = std::make_unique<AidlConstantReference>(loc, previous->GetName(), "");
enumerator->SetValue(std::make_unique<AidlBinaryConstExpression>(
- GetLocation(), std::move(prev_value), "+",
- std::unique_ptr<AidlConstantValue>(AidlConstantValue::Integral(GetLocation(), "1"))));
+ loc, std::move(prev_value), "+",
+ std::unique_ptr<AidlConstantValue>(AidlConstantValue::Integral(loc, "1"))));
}
}
previous = enumerator.get();
}
- return true;
}
std::set<AidlAnnotation::Type> AidlEnumDeclaration::GetSupportedAnnotations() const {
@@ -1477,3 +1479,32 @@
AidlImport::AidlImport(const AidlLocation& location, const std::string& needed_class)
: AidlNode(location), needed_class_(needed_class) {}
+
+// Resolves unresolved type name to fully qualified typename to import
+// case #1: SimpleName --> import p.SimpleName
+// case #2: Outer.Inner --> import p.Outer
+// case #3: p.SimpleName --> (as is)
+std::optional<std::string> AidlDocument::ResolveName(const std::string& unresolved_name) const {
+ std::string canonical_name;
+ const auto first_dot = unresolved_name.find_first_of('.');
+ const std::string class_name =
+ (first_dot == std::string::npos) ? unresolved_name : unresolved_name.substr(0, first_dot);
+ for (const auto& import : Imports()) {
+ const auto& fq_name = import->GetNeededClass();
+ const auto last_dot = fq_name.find_last_of('.');
+ const std::string imported_type_name =
+ (last_dot == std::string::npos) ? fq_name : fq_name.substr(last_dot + 1);
+ if (imported_type_name == class_name) {
+ if (canonical_name != "" && canonical_name != fq_name) {
+ AIDL_ERROR(import) << "Ambiguous type: " << canonical_name << " vs. " << fq_name;
+ return {};
+ }
+ canonical_name = fq_name;
+ }
+ }
+ // if not found, use unresolved_name as it is
+ if (canonical_name == "") {
+ return unresolved_name;
+ }
+ return canonical_name;
+}
\ No newline at end of file
diff --git a/aidl_language.h b/aidl_language.h
index 1df620e..1f03d4c 100644
--- a/aidl_language.h
+++ b/aidl_language.h
@@ -537,25 +537,29 @@
// example: "\"asdf\""
static AidlConstantValue* String(const AidlLocation& location, const string& value);
- // Construct an AidlConstantValue by evaluating the other integral constant's
- // value string. This does not preserve the structure of the copied constant.
- // Returns nullptr and logs if value cannot be copied.
- static AidlConstantValue* ShallowIntegralCopy(const AidlConstantValue& other);
-
Type GetType() const { return final_type_; }
+ const std::string& Literal() const { return value_; }
virtual bool CheckValid() const;
// Raw value of type (currently valid in C++ and Java). Empty string on error.
string ValueString(const AidlTypeSpecifier& type, const ConstantValueDecorator& decorator) const;
- virtual void Accept(Visitor& visitor) { visitor.Visit(*this); }
+ virtual void Accept(Visitor& visitor) {
+ visitor.Visit(*this);
+ if (type_ == Type::ARRAY) {
+ for (const auto& v : values_) {
+ v.get()->Accept(visitor);
+ }
+ }
+ }
private:
AidlConstantValue(const AidlLocation& location, Type parsed_type, int64_t parsed_value,
const string& checked_value);
AidlConstantValue(const AidlLocation& location, Type type, const string& checked_value);
AidlConstantValue(const AidlLocation& location, Type type,
- std::unique_ptr<vector<unique_ptr<AidlConstantValue>>> values);
+ std::unique_ptr<vector<unique_ptr<AidlConstantValue>>> values,
+ const std::string& value);
static string ToString(Type type);
static bool ParseIntegral(const string& value, int64_t* parsed_value, Type* parsed_type);
static bool IsHex(const string& value);
@@ -594,6 +598,7 @@
bool CheckValid() const override;
void Accept(Visitor& visitor) override { visitor.Visit(*this); }
+ const AidlConstantValue* Resolve();
private:
bool evaluate(const AidlTypeSpecifier& type) const override;
@@ -601,8 +606,7 @@
std::unique_ptr<AidlTypeSpecifier> ref_type_;
std::string field_name_;
const std::string comments_;
- mutable bool is_evaluating_ = false; // to prevent re-entrant CheckValid with circular references
- mutable bool is_validating_ = false; // to prevent re-entrant CheckValid with circular references
+ const AidlConstantValue* resolved_ = nullptr;
};
class AidlUnaryConstExpression : public AidlConstantValue {
@@ -950,11 +954,13 @@
const ConstantValueDecorator& decorator) const;
void SetValue(std::unique_ptr<AidlConstantValue> value) { value_ = std::move(value); }
+ bool IsValueUserSpecified() const { return value_user_specified_; }
private:
const std::string name_;
unique_ptr<AidlConstantValue> value_;
const std::string comments_;
+ const bool value_user_specified_;
};
class AidlEnumDeclaration : public AidlDefinedType {
@@ -975,7 +981,6 @@
const std::vector<std::unique_ptr<AidlEnumerator>>& GetEnumerators() const {
return enumerators_;
}
- bool Autofill();
std::set<AidlAnnotation::Type> GetSupportedAnnotations() const override;
bool CheckValid(const AidlTypenames& typenames) const override;
bool LanguageSpecificCheckValid(const AidlTypenames& /*typenames*/,
@@ -988,6 +993,8 @@
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_;
@@ -1078,6 +1085,7 @@
AidlDocument& operator=(const AidlDocument&) = delete;
AidlDocument& operator=(AidlDocument&&) = delete;
+ std::optional<std::string> ResolveName(const std::string& unresolved_type) const;
const std::vector<std::unique_ptr<AidlImport>>& Imports() const { return imports_; }
const std::vector<std::unique_ptr<AidlDefinedType>>& DefinedTypes() const {
return defined_types_;
diff --git a/aidl_language_y.yy b/aidl_language_y.yy
index b7fd889..8273815 100644
--- a/aidl_language_y.yy
+++ b/aidl_language_y.yy
@@ -410,11 +410,7 @@
delete $1;
}
| qualified_name {
- auto ref = new AidlConstantReference(loc(@1), $1->GetText(), $1->GetComments());
- if (ref->GetRefType()) {
- ps->DeferResolution(ref->GetRefType().get());
- }
- $$ = ref;
+ $$ = new AidlConstantReference(loc(@1), $1->GetText(), $1->GetComments());
delete $1;
}
| '{' constant_value_list '}' {
diff --git a/aidl_typenames.cpp b/aidl_typenames.cpp
index cd0a36d..2e77976 100644
--- a/aidl_typenames.cpp
+++ b/aidl_typenames.cpp
@@ -131,6 +131,17 @@
return true;
}
+const AidlDocument* AidlTypenames::GetDocumentFor(const AidlDefinedType* type) const {
+ for (const auto& doc : AllDocuments()) {
+ for (const auto& defined_type : doc->DefinedTypes()) {
+ if (defined_type.get() == type) {
+ return doc.get();
+ }
+ }
+ }
+ return nullptr;
+}
+
const AidlDocument& AidlTypenames::MainDocument() const {
AIDL_FATAL_IF(documents_.size() == 0, AIDL_LOCATION_HERE) << "Main document doesn't exist";
return *(documents_[0]);
diff --git a/aidl_typenames.h b/aidl_typenames.h
index bd85530..2a6973f 100644
--- a/aidl_typenames.h
+++ b/aidl_typenames.h
@@ -57,6 +57,7 @@
public:
AidlTypenames() = default;
bool AddDocument(std::unique_ptr<AidlDocument> doc);
+ const AidlDocument* GetDocumentFor(const AidlDefinedType* type) const;
const std::vector<std::unique_ptr<AidlDocument>>& AllDocuments() const { return documents_; }
const AidlDocument& MainDocument() const;
bool AddPreprocessedType(unique_ptr<AidlDefinedType> type);
diff --git a/aidl_unittest.cpp b/aidl_unittest.cpp
index 35d5edd..0956fa6 100644
--- a/aidl_unittest.cpp
+++ b/aidl_unittest.cpp
@@ -2991,8 +2991,7 @@
TEST_P(AidlTest, FailOnOutOfBoundsAutofilledEnum) {
AidlError error;
const string expected_stderr =
- "ERROR: p/TestEnum.aidl:3.35-44: Invalid type specifier for an int32 "
- "literal: byte\n"
+ "ERROR: p/TestEnum.aidl:5.1-36: Invalid type specifier for an int32 literal: byte\n"
"ERROR: p/TestEnum.aidl:5.1-36: Enumerator type differs from enum backing type.\n";
CaptureStderr();
EXPECT_EQ(nullptr, Parse("p/TestEnum.aidl",
@@ -4218,7 +4217,10 @@
auto options = Options::From("aidl -I a --lang ndk -o out -h out a/p/Foo.aidl");
EXPECT_EQ(1, aidl::compile_aidl(options, io_delegate_));
auto err = GetCapturedStderr();
- EXPECT_EQ("ERROR: a/p/Foo.aidl:1.26-28: Failed to parse expression as integer: B\n", err);
+ EXPECT_EQ(
+ "ERROR: a/p/Foo.aidl:1.26-28: Found a circular reference: B -> A -> B\n"
+ "ERROR: a/p/Foo.aidl:1.29-31: Found a circular reference: A -> B -> A\n",
+ err);
}
TEST_F(AidlTest, RejectsCircularReferencingConsts) {
@@ -4228,10 +4230,28 @@
auto options = Options::From("aidl -I a --lang ndk -o out -h out a/p/Foo.aidl");
EXPECT_EQ(1, aidl::compile_aidl(options, io_delegate_));
auto err = GetCapturedStderr();
- EXPECT_EQ(
- "ERROR: a/p/Foo.aidl:1.42-44: Can't evaluate the circular reference (A)\n"
- "ERROR: a/p/Foo.aidl:1.42-44: Invalid left operand in binary expression: A+1\n",
- err);
+ EXPECT_EQ("ERROR: a/p/Foo.aidl:1.42-44: Found a circular reference: A -> A\n", err);
+}
+
+TEST_F(AidlTest, RecursiveReferences) {
+ io_delegate_.SetFileContents("a/p/Foo.aidl",
+ "package p; parcelable Foo { const int A = p.Bar.A + 1; }");
+ io_delegate_.SetFileContents("a/p/Bar.aidl",
+ "package p; parcelable Bar { const int A = p.Baz.A + 1; }");
+ io_delegate_.SetFileContents("a/p/Baz.aidl", "package p; parcelable Baz { const int A = 1; }");
+ CaptureStderr();
+ auto options = Options::From("aidl -I a --lang ndk -o out -h out a/p/Foo.aidl");
+ EXPECT_EQ(0, aidl::compile_aidl(options, io_delegate_));
+ EXPECT_EQ("", GetCapturedStderr());
+}
+
+TEST_P(AidlTest, JavaCompatibleBuiltinTypes) {
+ string contents = R"(
+import android.os.IBinder;
+import android.os.IInterface;
+interface IFoo {}
+ )";
+ EXPECT_NE(nullptr, Parse("IFoo.aidl", contents, typenames_, GetLanguage()));
}
} // namespace aidl
diff --git a/parser.cpp b/parser.cpp
index b5053b6..15faed5 100644
--- a/parser.cpp
+++ b/parser.cpp
@@ -61,55 +61,110 @@
}
}
-bool Parser::Resolve() {
+class ConstantReferenceResolver : public AidlConstantValue::Visitor {
+ public:
+ ConstantReferenceResolver(const AidlDefinedType* scope, const AidlTypenames& typenames,
+ TypeResolver& resolver, bool* success)
+ : scope_(scope), typenames_(typenames), resolver_(resolver), success_(success) {}
+ void Visit(AidlConstantValue&) override {}
+ void Visit(AidlUnaryConstExpression&) override {}
+ void Visit(AidlBinaryConstExpression&) override {}
+ void Visit(AidlConstantReference& v) override {
+ if (IsCircularReference(&v)) {
+ *success_ = false;
+ return;
+ }
+
+ // when <type> is missing, we use a scope type
+ if (!v.GetRefType()) {
+ v.SetRefType(std::make_unique<AidlTypeSpecifier>(v.GetLocation(), scope_->GetCanonicalName(),
+ false, nullptr, ""));
+ }
+ if (!v.GetRefType()->IsResolved()) {
+ if (!resolver_(typenames_.GetDocumentFor(scope_), v.GetRefType().get())) {
+ AIDL_ERROR(v.GetRefType()) << "Failed to resolve '" << v.GetRefType()->GetName() << "'";
+ *success_ = false;
+ return;
+ }
+ }
+ const AidlConstantValue* resolved = v.Resolve();
+ if (!resolved) {
+ AIDL_ERROR(v.GetRefType()) << "Failed to resolve '" << v.GetRefType()->GetName() << "'";
+ *success_ = false;
+ return;
+ }
+
+ // resolve recursive references
+ Push(&v);
+ const_cast<AidlConstantValue*>(resolved)->Accept(*this);
+ Pop();
+ }
+
+ private:
+ struct StackElem {
+ const AidlDefinedType* scope;
+ const AidlConstantReference* ref;
+ };
+
+ void Push(const AidlConstantReference* ref) {
+ stack_.push_back({scope_, ref});
+ scope_ = ref->GetRefType()->GetDefinedType();
+ }
+
+ void Pop() {
+ scope_ = stack_.back().scope;
+ stack_.pop_back();
+ }
+
+ bool IsCircularReference(const AidlConstantReference* ref) {
+ auto it = std::find_if(stack_.begin(), stack_.end(),
+ [&](const auto& elem) { return elem.ref == ref; });
+ if (it == stack_.end()) {
+ return false;
+ }
+ std::vector<std::string> path;
+ while (it != stack_.end()) {
+ path.push_back(it->ref->Literal());
+ ++it;
+ }
+ path.push_back(ref->Literal());
+ AIDL_ERROR(ref) << "Found a circular reference: " << android::base::Join(path, " -> ");
+ return true;
+ }
+
+ const AidlDefinedType* scope_;
+ const AidlTypenames& typenames_;
+ TypeResolver& resolver_;
+ bool* success_;
+ std::vector<StackElem> stack_ = {};
+};
+
+bool Parser::Resolve(TypeResolver& type_resolver) {
bool success = true;
for (AidlTypeSpecifier* typespec : unresolved_typespecs_) {
- if (!typespec->Resolve(typenames_)) {
+ if (!type_resolver(document_, typespec)) {
AIDL_ERROR(typespec) << "Failed to resolve '" << typespec->GetUnresolvedName() << "'";
success = false;
// don't stop to show more errors if any
}
}
- struct ConstantReferenceResolver : public AidlConstantValue::Visitor {
- const std::string name_;
- const AidlTypenames& typenames_;
- bool* success_;
- ConstantReferenceResolver(const std::string& name, const AidlTypenames& typenames,
- bool* success)
- : name_(name), typenames_(typenames), success_(success) {}
- void Visit(AidlConstantValue&) override {}
- void Visit(AidlUnaryConstExpression&) override {}
- void Visit(AidlBinaryConstExpression&) override {}
- void Visit(AidlConstantReference& v) override {
- // when <type> is missing, we use
- if (!v.GetRefType()) {
- auto type = std::make_unique<AidlTypeSpecifier>(v.GetLocation(), name_, false, nullptr, "");
- type->Resolve(typenames_);
- v.SetRefType(std::move(type));
- }
- // check if the reference points to a valid field
- if (!v.CheckValid()) {
- *success_ = false;
- }
- }
- };
// resolve "field references" as well.
for (const auto& type : document_->DefinedTypes()) {
- ConstantReferenceResolver resolver{type->GetCanonicalName(), typenames_, &success};
+ ConstantReferenceResolver ref_resolver{type.get(), typenames_, type_resolver, &success};
if (auto enum_type = type->AsEnumDeclaration(); enum_type) {
for (const auto& enumerator : enum_type->GetEnumerators()) {
if (auto value = enumerator->GetValue(); value) {
- value->Accept(resolver);
+ value->Accept(ref_resolver);
}
}
} else {
for (const auto& constant : type->GetConstantDeclarations()) {
- const_cast<AidlConstantValue&>(constant->GetValue()).Accept(resolver);
+ const_cast<AidlConstantValue&>(constant->GetValue()).Accept(ref_resolver);
}
for (const auto& field : type->GetFields()) {
if (field->IsDefaultUserSpecified()) {
- const_cast<AidlConstantValue*>(field->GetDefaultValue())->Accept(resolver);
+ const_cast<AidlConstantValue*>(field->GetDefaultValue())->Accept(ref_resolver);
}
}
}
diff --git a/parser.h b/parser.h
index 726922d..f1b6dd8 100644
--- a/parser.h
+++ b/parser.h
@@ -53,6 +53,8 @@
std::string comments_;
};
+using TypeResolver = std::function<bool(const AidlDocument*, AidlTypeSpecifier*)>;
+
class Parser {
public:
// non-copyable, non-assignable
@@ -90,7 +92,7 @@
const vector<AidlTypeSpecifier*>& GetUnresolvedTypespecs() const { return unresolved_typespecs_; }
- bool Resolve();
+ bool Resolve(TypeResolver& type_resolver);
void SetDocument(std::unique_ptr<AidlDocument>&& document) {
// The parsed document is owned by typenames_. This parser object only has
// a reference to it.