ConstantValueDecorator with strings
ConstantValueDecorator now handles array-type values better. Previously,
array values are passed to ConstantValueDecorator after joining into a
single string, which makes it difficult to handle array values because
it should rely on formatting ("{", Join(), "}) and "wrong" type.
Now, when the type is an array, raw_value is passed as vector<string>.
This is a preparation step for multi-dimensional fixed-size arrays.
Nested constant arrays like "{{1,2,3}, {4,5,6}}" will be used as a
default value for fixed-size arrays.
Bug: 204116012
Bug: 207087196
Test: aidl_unittests
Change-Id: I15353816dafcc80b55f1878f6bcc923e21cd12e8
diff --git a/aidl_const_expressions.cpp b/aidl_const_expressions.cpp
index 2d19efd..47d81e1 100644
--- a/aidl_const_expressions.cpp
+++ b/aidl_const_expressions.cpp
@@ -462,11 +462,13 @@
AidlConstantValue* AidlConstantValue::Array(
const AidlLocation& location, std::unique_ptr<vector<unique_ptr<AidlConstantValue>>> values) {
AIDL_FATAL_IF(values == nullptr, location);
+ // Reconstruct literal value
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, ", "));
+ return new AidlConstantValue(location, Type::ARRAY, std::move(values),
+ "{" + Join(str_values, ", ") + "}");
}
AidlConstantValue* AidlConstantValue::String(const AidlLocation& location, const string& value) {
@@ -517,7 +519,7 @@
return decorator(type, value_);
}
- const string& type_string = type.GetName();
+ const string& type_string = type.Signature();
int err = 0;
switch (final_type_) {
@@ -566,11 +568,10 @@
bool success = true;
for (const auto& value : values_) {
- // Pass array type(T[]) as it is instead of converting it to base type(T)
- // so that decorator can decorate the value in the context of array.
- // In C++/NDK, 'byte[]' and 'byte' are mapped to different types. If we pass 'byte'
- // decorator can't know the value should be treated as 'uint8_t'.
- string value_string = value->ValueString(type, decorator);
+ string value_string;
+ type.ViewAsArrayBase([&](const auto& base_type) {
+ value_string = value->ValueString(base_type, decorator);
+ });
if (value_string.empty()) {
success = false;
break;
@@ -581,8 +582,7 @@
err = -1;
break;
}
-
- return decorator(type, "{" + Join(value_strings, ", ") + "}");
+ return decorator(type, value_strings);
}
case Type::FLOATING: {
if (type_string == "double") {
diff --git a/aidl_language.cpp b/aidl_language.cpp
index 16ebea5..7db3b7f 100644
--- a/aidl_language.cpp
+++ b/aidl_language.cpp
@@ -725,18 +725,20 @@
return true;
}
-std::string AidlConstantValueDecorator(const AidlTypeSpecifier& type,
- const std::string& raw_value) {
+std::string AidlConstantValueDecorator(
+ const AidlTypeSpecifier& type,
+ const std::variant<std::string, std::vector<std::string>>& raw_value) {
if (type.IsArray()) {
- return raw_value;
+ const auto& values = std::get<std::vector<std::string>>(raw_value);
+ return "{" + Join(values, ", ") + "}";
}
-
+ const std::string& value = std::get<std::string>(raw_value);
if (auto defined_type = type.GetDefinedType(); defined_type) {
auto enum_type = defined_type->AsEnumDeclaration();
- AIDL_FATAL_IF(!enum_type, type) << "Invalid type for \"" << raw_value << "\"";
- return type.GetName() + "." + raw_value.substr(raw_value.find_last_of('.') + 1);
+ AIDL_FATAL_IF(!enum_type, type) << "Invalid type for \"" << value << "\"";
+ return type.GetName() + "." + value.substr(value.find_last_of('.') + 1);
}
- return raw_value;
+ return value;
}
AidlVariableDeclaration::AidlVariableDeclaration(const AidlLocation& location,
diff --git a/aidl_language.h b/aidl_language.h
index 392665d..019330c 100644
--- a/aidl_language.h
+++ b/aidl_language.h
@@ -20,6 +20,7 @@
#include <regex>
#include <string>
#include <unordered_set>
+#include <variant>
#include <vector>
#include <android-base/result.h>
@@ -215,8 +216,9 @@
// Transforms a value string into a language specific form. Raw value as produced by
// AidlConstantValue.
-using ConstantValueDecorator =
- std::function<std::string(const AidlTypeSpecifier& type, const std::string& raw_value)>;
+using ConstantValueDecorator = std::function<std::string(
+ const AidlTypeSpecifier& type,
+ const std::variant<std::string, std::vector<std::string>>& raw_value)>;
class AidlAnnotation : public AidlNode {
public:
@@ -458,7 +460,9 @@
};
// Returns the universal value unaltered.
-std::string AidlConstantValueDecorator(const AidlTypeSpecifier& type, const std::string& raw_value);
+std::string AidlConstantValueDecorator(
+ const AidlTypeSpecifier& type,
+ const std::variant<std::string, std::vector<std::string>>& raw_value);
class AidlMember : public AidlAnnotatable {
public:
diff --git a/aidl_to_cpp.cpp b/aidl_to_cpp.cpp
index c476c90..422a2e5 100644
--- a/aidl_to_cpp.cpp
+++ b/aidl_to_cpp.cpp
@@ -200,7 +200,9 @@
return WrapIfNullable(cpp_name, raw_type, typenames);
}
} // namespace
-std::string ConstantValueDecorator(const AidlTypeSpecifier& type, const std::string& raw_value) {
+std::string ConstantValueDecorator(
+ const AidlTypeSpecifier& type,
+ const std::variant<std::string, std::vector<std::string>>& raw_value) {
return CppConstantValueDecorator(type, raw_value, /*is_ndk=*/false);
};
diff --git a/aidl_to_cpp.h b/aidl_to_cpp.h
index eab471e..73b79bb 100644
--- a/aidl_to_cpp.h
+++ b/aidl_to_cpp.h
@@ -23,7 +23,9 @@
// This header provides functions that translate AIDL things to cpp things.
-std::string ConstantValueDecorator(const AidlTypeSpecifier& type, const std::string& raw_value);
+std::string ConstantValueDecorator(
+ const AidlTypeSpecifier& type,
+ const std::variant<std::string, std::vector<std::string>>& raw_value);
struct CodeGeneratorContext {
CodeWriter& writer;
diff --git a/aidl_to_cpp_common.cpp b/aidl_to_cpp_common.cpp
index 09c9e03..aa57037 100644
--- a/aidl_to_cpp_common.cpp
+++ b/aidl_to_cpp_common.cpp
@@ -693,56 +693,64 @@
out << "__assert2(__FILE__, __LINE__, __PRETTY_FUNCTION__, \"can't reach here\");\n";
}
-std::string CppConstantValueDecorator(const AidlTypeSpecifier& type, const std::string& raw_value,
- bool is_ndk) {
- AIDL_FATAL_IF(raw_value.empty(), type) << "Empty value for constants";
-
- // apply array type only if raw_value is actually an array value(`{...}`).
- if (type.IsArray() && raw_value[0] == '{') {
- return raw_value;
+std::string CppConstantValueDecorator(
+ const AidlTypeSpecifier& type,
+ const std::variant<std::string, std::vector<std::string>>& raw_value, bool is_ndk) {
+ if (type.IsArray()) {
+ const auto& values = std::get<std::vector<std::string>>(raw_value);
+ // Hexadecimal literals for byte arrays should be casted to uint8_t
+ if (type.GetName() == "byte" &&
+ std::any_of(values.begin(), values.end(),
+ [](const auto& value) { return !value.empty() && value[0] == '-'; })) {
+ std::vector<std::string> copy = values;
+ for (auto& value : copy) {
+ // cast only if necessary
+ if (value[0] == '-') {
+ value = "uint8_t(" + value + ")";
+ }
+ }
+ return "{" + Join(copy, ", ") + "}";
+ }
+ return "{" + Join(values, ", ") + "}";
}
+ const std::string& value = std::get<std::string>(raw_value);
if (AidlTypenames::IsBuiltinTypename(type.GetName())) {
if (type.GetName() == "boolean") {
- return raw_value;
+ return value;
} else if (type.GetName() == "byte") {
- // cast only if necessary
- if (type.IsArray() && raw_value[0] == '-') {
- return "uint8_t(" + raw_value + ")";
- } else {
- return raw_value;
- }
+ return value;
} else if (type.GetName() == "char") {
// TODO: consider 'L'-prefix for wide char literal
- return raw_value;
+ return value;
} else if (type.GetName() == "double") {
- return raw_value;
+ return value;
} else if (type.GetName() == "float") {
- return raw_value; // raw_value has 'f' suffix
+ return value; // value has 'f' suffix
} else if (type.GetName() == "int") {
- return raw_value;
+ return value;
} else if (type.GetName() == "long") {
- return raw_value + "L";
+ return value + "L";
} else if (type.GetName() == "String") {
if (is_ndk || type.IsUtf8InCpp()) {
- return raw_value;
+ return value;
} else {
- return "::android::String16(" + raw_value + ")";
+ return "::android::String16(" + value + ")";
}
}
AIDL_FATAL(type) << "Unknown built-in type: " << type.GetName();
}
auto defined_type = type.GetDefinedType();
- AIDL_FATAL_IF(!defined_type, type) << "Invalid type for \"" << raw_value << "\"";
+ AIDL_FATAL_IF(!defined_type, type) << "Invalid type for \"" << value << "\"";
auto enum_type = defined_type->AsEnumDeclaration();
- AIDL_FATAL_IF(!enum_type, type) << "Invalid type for \"" << raw_value << "\"";
+ AIDL_FATAL_IF(!enum_type, type) << "Invalid type for \"" << value << "\"";
auto cpp_type_name = "::" + Join(Split(enum_type->GetCanonicalName(), "."), "::");
if (is_ndk) {
cpp_type_name = "::aidl" + cpp_type_name;
}
- return cpp_type_name + "::" + raw_value.substr(raw_value.find_last_of('.') + 1);
+ return cpp_type_name + "::" + value.substr(value.find_last_of('.') + 1);
}
} // namespace cpp
} // namespace aidl
diff --git a/aidl_to_cpp_common.h b/aidl_to_cpp_common.h
index b1a613d..e6733b6 100644
--- a/aidl_to_cpp_common.h
+++ b/aidl_to_cpp_common.h
@@ -129,8 +129,9 @@
void WriteToParcel(CodeWriter& out, const ParcelWriterContext&) const;
};
-std::string CppConstantValueDecorator(const AidlTypeSpecifier& type, const std::string& raw_value,
- bool is_ndk);
+std::string CppConstantValueDecorator(
+ const AidlTypeSpecifier& type,
+ const std::variant<std::string, std::vector<std::string>>& raw_value, bool is_ndk);
} // namespace cpp
} // namespace aidl
} // namespace android
diff --git a/aidl_to_java.cpp b/aidl_to_java.cpp
index 376030e..d8a05d5 100644
--- a/aidl_to_java.cpp
+++ b/aidl_to_java.cpp
@@ -38,19 +38,23 @@
using std::string;
using std::vector;
-std::string ConstantValueDecorator(const AidlTypeSpecifier& type, const std::string& raw_value) {
- if (type.IsArray() && !raw_value.empty() && raw_value[0] == '{') {
- return raw_value;
+std::string ConstantValueDecorator(
+ const AidlTypeSpecifier& type,
+ const std::variant<std::string, std::vector<std::string>>& raw_value) {
+ if (type.IsArray()) {
+ const auto& values = std::get<std::vector<std::string>>(raw_value);
+ return "{" + Join(values, ", ") + "}";
}
+ const std::string& value = std::get<std::string>(raw_value);
if (type.GetName() == "long") {
- return raw_value + "L";
+ return value + "L";
}
if (auto defined_type = type.GetDefinedType(); defined_type) {
auto enum_type = defined_type->AsEnumDeclaration();
- AIDL_FATAL_IF(!enum_type, type) << "Invalid type for \"" << raw_value << "\"";
- return type.GetName() + "." + raw_value.substr(raw_value.find_last_of('.') + 1);
+ AIDL_FATAL_IF(!enum_type, type) << "Invalid type for \"" << value << "\"";
+ return type.GetName() + "." + value.substr(value.find_last_of('.') + 1);
}
- return raw_value;
+ return value;
};
const string& JavaNameOf(const AidlTypeSpecifier& aidl, bool instantiable = false,
diff --git a/aidl_to_java.h b/aidl_to_java.h
index aaee036..5a4632c 100644
--- a/aidl_to_java.h
+++ b/aidl_to_java.h
@@ -39,7 +39,9 @@
// This header provides functions that translate AIDL things to Java things.
-std::string ConstantValueDecorator(const AidlTypeSpecifier& type, const std::string& raw_value);
+std::string ConstantValueDecorator(
+ const AidlTypeSpecifier& type,
+ const std::variant<std::string, std::vector<std::string>>& raw_value);
// Returns the Java type signature of the AIDL type spec
// This includes generic type parameters with array modifiers.
diff --git a/aidl_to_ndk.cpp b/aidl_to_ndk.cpp
index c9ad112..b9b632c 100644
--- a/aidl_to_ndk.cpp
+++ b/aidl_to_ndk.cpp
@@ -70,7 +70,9 @@
std::shared_ptr<Aspect> nullable_array;
};
-std::string ConstantValueDecorator(const AidlTypeSpecifier& type, const std::string& raw_value) {
+std::string ConstantValueDecorator(
+ const AidlTypeSpecifier& type,
+ const std::variant<std::string, std::vector<std::string>>& raw_value) {
return cpp::CppConstantValueDecorator(type, raw_value, /*is_ndk=*/true);
};
diff --git a/aidl_to_ndk.h b/aidl_to_ndk.h
index 1f7f490..7d815c5 100644
--- a/aidl_to_ndk.h
+++ b/aidl_to_ndk.h
@@ -31,7 +31,9 @@
std::string NdkHeaderFile(const AidlDefinedType& defined_type, cpp::ClassNames name,
bool use_os_sep = true);
-std::string ConstantValueDecorator(const AidlTypeSpecifier& type, const std::string& raw_value);
+std::string ConstantValueDecorator(
+ const AidlTypeSpecifier& type,
+ const std::variant<std::string, std::vector<std::string>>& raw_value);
// Returns ::aidl::some_package::some_sub_package::foo::IFoo/BpFoo/BnFoo
std::string NdkFullClassName(const AidlDefinedType& type, cpp::ClassNames name);
diff --git a/aidl_to_rust.cpp b/aidl_to_rust.cpp
index 21e67cd..ad64b6e 100644
--- a/aidl_to_rust.cpp
+++ b/aidl_to_rust.cpp
@@ -39,40 +39,44 @@
namespace {
std::string GetRawRustName(const AidlTypeSpecifier& type);
-std::string ConstantValueDecoratorInternal(const AidlTypeSpecifier& type,
- const std::string& raw_value, bool by_ref) {
- if (type.IsArray() && !raw_value.empty() && raw_value[0] == '{') {
- // Convert `{ ... }` to `vec!{ ... }`
- return "vec!" + raw_value;
+std::string ConstantValueDecoratorInternal(
+ const AidlTypeSpecifier& type,
+ const std::variant<std::string, std::vector<std::string>>& raw_value, bool by_ref) {
+ if (type.IsArray()) {
+ // Convert `{ ... }` to `vec![ ... ]`
+ const auto& values = std::get<std::vector<std::string>>(raw_value);
+ return "vec![" + Join(values, ", ") + "]";
}
+ const std::string& value = std::get<std::string>(raw_value);
+
const auto& aidl_name = type.GetName();
if (aidl_name == "char") {
- return raw_value + " as u16";
+ return value + " as u16";
}
if (aidl_name == "float") {
- // raw_value already ends in `f`, so just add `32`
- return raw_value + "32";
+ // value already ends in `f`, so just add `32`
+ return value + "32";
}
if (aidl_name == "double") {
- return raw_value + "f64";
+ return value + "f64";
}
if (aidl_name == "String" && !by_ref) {
// The actual type might be String or &str,
// and .into() transparently converts into either one
- return raw_value + ".into()";
+ return value + ".into()";
}
if (auto defined_type = type.GetDefinedType(); defined_type) {
auto enum_type = defined_type->AsEnumDeclaration();
- AIDL_FATAL_IF(!enum_type, type) << "Invalid type for \"" << raw_value << "\"";
- return GetRawRustName(type) + "::" + raw_value.substr(raw_value.find_last_of('.') + 1);
+ AIDL_FATAL_IF(!enum_type, type) << "Invalid type for \"" << value << "\"";
+ return GetRawRustName(type) + "::" + value.substr(value.find_last_of('.') + 1);
}
- return raw_value;
+ return value;
}
std::string GetRawRustName(const AidlTypeSpecifier& type) {
@@ -140,7 +144,9 @@
}
} // namespace
-std::string ConstantValueDecorator(const AidlTypeSpecifier& type, const std::string& raw_value) {
+std::string ConstantValueDecorator(
+ const AidlTypeSpecifier& type,
+ const std::variant<std::string, std::vector<std::string>>& raw_value) {
auto rust_value = ConstantValueDecoratorInternal(type, raw_value, false);
if (type.IsNullable()) {
return "Some(" + rust_value + ")";
@@ -148,7 +154,9 @@
return rust_value;
}
-std::string ConstantValueDecoratorRef(const AidlTypeSpecifier& type, const std::string& raw_value) {
+std::string ConstantValueDecoratorRef(
+ const AidlTypeSpecifier& type,
+ const std::variant<std::string, std::vector<std::string>>& raw_value) {
auto rust_value = ConstantValueDecoratorInternal(type, raw_value, true);
if (type.IsNullable()) {
return "Some(" + rust_value + ")";
diff --git a/aidl_to_rust.h b/aidl_to_rust.h
index 6145122..2fe73ae 100644
--- a/aidl_to_rust.h
+++ b/aidl_to_rust.h
@@ -58,9 +58,13 @@
}
}
-std::string ConstantValueDecorator(const AidlTypeSpecifier& type, const std::string& raw_value);
+std::string ConstantValueDecorator(
+ const AidlTypeSpecifier& type,
+ const std::variant<std::string, std::vector<std::string>>& raw_value);
-std::string ConstantValueDecoratorRef(const AidlTypeSpecifier& type, const std::string& raw_value);
+std::string ConstantValueDecoratorRef(
+ const AidlTypeSpecifier& type,
+ const std::variant<std::string, std::vector<std::string>>& raw_value);
// Returns "'lifetime_name " including the initial apostrophe and the trailing space.
// Returns empty string for NONE.
diff --git a/aidl_unittest.cpp b/aidl_unittest.cpp
index d811cd8..e6f2a94 100644
--- a/aidl_unittest.cpp
+++ b/aidl_unittest.cpp
@@ -4896,7 +4896,7 @@
io_delegate_.SetFileContents(
"IFoo.aidl",
"interface IFoo {\n"
- " void foo(in @SuppressWarnings(value=\"inout-parameter\") int x);\n"
+ " void foo(in @SuppressWarnings(value={\"inout-parameter\"}) int x);\n"
"}");
auto options = Options::From("aidl --lang=java IFoo.aidl");
CaptureStderr();
diff --git a/tests/golden_output/aidl-test-interface-rust-source/gen/android/aidl/tests/StructuredParcelable.rs b/tests/golden_output/aidl-test-interface-rust-source/gen/android/aidl/tests/StructuredParcelable.rs
index d0710b9..10326ac 100644
--- a/tests/golden_output/aidl-test-interface-rust-source/gen/android/aidl/tests/StructuredParcelable.rs
+++ b/tests/golden_output/aidl-test-interface-rust-source/gen/android/aidl/tests/StructuredParcelable.rs
@@ -80,8 +80,8 @@
charDefaultsToC: 'C' as u16,
floatDefaultsToPi: 3.140000f32,
doubleWithDefault: -314000000000000000.000000f64,
- arrayDefaultsTo123: vec!{1, 2, 3},
- arrayDefaultsToEmpty: vec!{},
+ arrayDefaultsTo123: vec![1, 2, 3],
+ arrayDefaultsToEmpty: vec![],
boolDefault: false,
byteDefault: 0,
intDefault: 0,
@@ -89,16 +89,16 @@
floatDefault: 0.000000f32,
doubleDefault: 0.000000f64,
checkDoubleFromFloat: 3.140000f64,
- checkStringArray1: vec!{"a".into(), "b".into()},
- checkStringArray2: vec!{"a".into(), "b".into()},
+ checkStringArray1: vec!["a".into(), "b".into()],
+ checkStringArray2: vec!["a".into(), "b".into()],
int32_min: -2147483648,
int32_max: 2147483647,
int64_max: 9223372036854775807,
hexInt32_neg_1: -1,
ibinder: Default::default(),
- int8_1: vec!{1, 1, 1, 1, 1},
- int32_1: vec!{1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1},
- int64_1: vec!{1, 1, 1, 1, 1, 1, 1, 1, 1, 1},
+ int8_1: vec![1, 1, 1, 1, 1],
+ int32_1: vec![1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1],
+ int64_1: vec![1, 1, 1, 1, 1, 1, 1, 1, 1, 1],
hexInt32_pos_1: 1,
hexInt64_pos_1: 1,
const_exprs_1: Default::default(),
diff --git a/tests/golden_output/aidl-test-interface-rust-source/gen/android/aidl/tests/Union.rs b/tests/golden_output/aidl-test-interface-rust-source/gen/android/aidl/tests/Union.rs
index bf1c8a3..d9cddc4 100644
--- a/tests/golden_output/aidl-test-interface-rust-source/gen/android/aidl/tests/Union.rs
+++ b/tests/golden_output/aidl-test-interface-rust-source/gen/android/aidl/tests/Union.rs
@@ -13,7 +13,7 @@
pub const S1: &str = "a string constant in union";
impl Default for Union {
fn default() -> Self {
- Self::Ns(vec!{})
+ Self::Ns(vec![])
}
}
impl binder::parcel::Parcelable for Union {