aconfig: update c/c++ codegen
1, Current codegen would still include an empty flag value cache even if
there are no read write flags. This is important as the flag value cache
is a std::vector. Thus adding this cache would automatically pull in
libc++ dependency which we should not.
2, There lacks of c/c++ codegen unit tests to lock down the generated
code when a flag package contains only read only flags. It should not
include server_configurable_flags header. And it should not include the
flag cache.
Bug: b/316614694
Test: atest aconfig.test
Change-Id: Iee4366672932ee507694f5fc73b526c66fcf0e9b
diff --git a/tools/aconfig/src/codegen/cpp.rs b/tools/aconfig/src/codegen/cpp.rs
index ada022a..568302d 100644
--- a/tools/aconfig/src/codegen/cpp.rs
+++ b/tools/aconfig/src/codegen/cpp.rs
@@ -135,6 +135,7 @@
#[cfg(test)]
mod tests {
use super::*;
+ use crate::protos::ProtoParsedFlags;
use std::collections::HashMap;
const EXPORTED_PROD_HEADER_EXPECTED: &str = r#"
@@ -732,8 +733,127 @@
"#;
- fn test_generate_cpp_code(mode: CodegenMode) {
- let parsed_flags = crate::test::parse_test_flags();
+ const READ_ONLY_EXPORTED_PROD_HEADER_EXPECTED: &str = r#"
+#pragma once
+
+#ifndef COM_ANDROID_ACONFIG_TEST
+#define COM_ANDROID_ACONFIG_TEST(FLAG) COM_ANDROID_ACONFIG_TEST_##FLAG
+#endif
+
+#ifndef COM_ANDROID_ACONFIG_TEST_DISABLED_FIXED_RO
+#define COM_ANDROID_ACONFIG_TEST_DISABLED_FIXED_RO false
+#endif
+
+#ifndef COM_ANDROID_ACONFIG_TEST_ENABLED_FIXED_RO
+#define COM_ANDROID_ACONFIG_TEST_ENABLED_FIXED_RO true
+#endif
+
+#ifdef __cplusplus
+
+#include <memory>
+
+namespace com::android::aconfig::test {
+
+class flag_provider_interface {
+public:
+ virtual ~flag_provider_interface() = default;
+
+ virtual bool disabled_fixed_ro() = 0;
+
+ virtual bool disabled_ro() = 0;
+
+ virtual bool enabled_fixed_ro() = 0;
+
+ virtual bool enabled_ro() = 0;
+};
+
+extern std::unique_ptr<flag_provider_interface> provider_;
+
+inline bool disabled_fixed_ro() {
+ return COM_ANDROID_ACONFIG_TEST_DISABLED_FIXED_RO;
+}
+
+inline bool disabled_ro() {
+ return false;
+}
+
+inline bool enabled_fixed_ro() {
+ return COM_ANDROID_ACONFIG_TEST_ENABLED_FIXED_RO;
+}
+
+inline bool enabled_ro() {
+ return true;
+}
+}
+
+extern "C" {
+#endif // __cplusplus
+
+bool com_android_aconfig_test_disabled_fixed_ro();
+
+bool com_android_aconfig_test_disabled_ro();
+
+bool com_android_aconfig_test_enabled_fixed_ro();
+
+bool com_android_aconfig_test_enabled_ro();
+
+#ifdef __cplusplus
+} // extern "C"
+#endif
+"#;
+
+ const READ_ONLY_PROD_SOURCE_FILE_EXPECTED: &str = r#"
+#include "com_android_aconfig_test.h"
+
+namespace com::android::aconfig::test {
+
+ class flag_provider : public flag_provider_interface {
+ public:
+
+ virtual bool disabled_fixed_ro() override {
+ return COM_ANDROID_ACONFIG_TEST_DISABLED_FIXED_RO;
+ }
+
+ virtual bool disabled_ro() override {
+ return false;
+ }
+
+ virtual bool enabled_fixed_ro() override {
+ return COM_ANDROID_ACONFIG_TEST_ENABLED_FIXED_RO;
+ }
+
+ virtual bool enabled_ro() override {
+ return true;
+ }
+ };
+
+ std::unique_ptr<flag_provider_interface> provider_ =
+ std::make_unique<flag_provider>();
+}
+
+bool com_android_aconfig_test_disabled_fixed_ro() {
+ return COM_ANDROID_ACONFIG_TEST_DISABLED_FIXED_RO;
+}
+
+bool com_android_aconfig_test_disabled_ro() {
+ return false;
+}
+
+bool com_android_aconfig_test_enabled_fixed_ro() {
+ return COM_ANDROID_ACONFIG_TEST_ENABLED_FIXED_RO;
+}
+
+bool com_android_aconfig_test_enabled_ro() {
+ return true;
+}
+"#;
+
+ fn test_generate_cpp_code(
+ parsed_flags: ProtoParsedFlags,
+ mode: CodegenMode,
+ expected_header: &str,
+ expected_src: &str,
+ ) {
let generated = generate_cpp_code(
crate::test::TEST_PACKAGE,
parsed_flags.parsed_flag.into_iter(),
@@ -753,12 +873,7 @@
assert_eq!(
None,
crate::test::first_significant_code_diff(
- match mode {
- CodegenMode::Production => EXPORTED_PROD_HEADER_EXPECTED,
- CodegenMode::Test => EXPORTED_TEST_HEADER_EXPECTED,
- CodegenMode::Exported =>
- todo!("exported mode not yet supported for cpp, see b/313894653."),
- },
+ expected_header,
generated_files_map.get(&target_file_path).unwrap()
)
);
@@ -768,12 +883,7 @@
assert_eq!(
None,
crate::test::first_significant_code_diff(
- match mode {
- CodegenMode::Production => PROD_SOURCE_FILE_EXPECTED,
- CodegenMode::Test => TEST_SOURCE_FILE_EXPECTED,
- CodegenMode::Exported =>
- todo!("exported mode not yet supported for cpp, see b/313894653."),
- },
+ expected_src,
generated_files_map.get(&target_file_path).unwrap()
)
);
@@ -781,11 +891,34 @@
#[test]
fn test_generate_cpp_code_for_prod() {
- test_generate_cpp_code(CodegenMode::Production);
+ let parsed_flags = crate::test::parse_test_flags();
+ test_generate_cpp_code(
+ parsed_flags,
+ CodegenMode::Production,
+ EXPORTED_PROD_HEADER_EXPECTED,
+ PROD_SOURCE_FILE_EXPECTED,
+ );
}
#[test]
fn test_generate_cpp_code_for_test() {
- test_generate_cpp_code(CodegenMode::Test);
+ let parsed_flags = crate::test::parse_test_flags();
+ test_generate_cpp_code(
+ parsed_flags,
+ CodegenMode::Test,
+ EXPORTED_TEST_HEADER_EXPECTED,
+ TEST_SOURCE_FILE_EXPECTED,
+ );
+ }
+
+ #[test]
+ fn test_generate_cpp_code_for_read_only_prod() {
+ let parsed_flags = crate::test::parse_read_only_test_flags();
+ test_generate_cpp_code(
+ parsed_flags,
+ CodegenMode::Production,
+ READ_ONLY_EXPORTED_PROD_HEADER_EXPECTED,
+ READ_ONLY_PROD_SOURCE_FILE_EXPECTED,
+ );
}
}
diff --git a/tools/aconfig/src/test.rs b/tools/aconfig/src/test.rs
index 71de57e..309cb28 100644
--- a/tools/aconfig/src/test.rs
+++ b/tools/aconfig/src/test.rs
@@ -225,6 +225,24 @@
}
"#;
+ pub fn parse_read_only_test_flags() -> ProtoParsedFlags {
+ let bytes = crate::commands::parse_flags(
+ "com.android.aconfig.test",
+ Some("system"),
+ vec![Input {
+ source: "tests/read_only_test.aconfig".to_string(),
+ reader: Box::new(include_bytes!("../tests/read_only_test.aconfig").as_slice()),
+ }],
+ vec![Input {
+ source: "tests/read_only_test.values".to_string(),
+ reader: Box::new(include_bytes!("../tests/read_only_test.values").as_slice()),
+ }],
+ crate::commands::DEFAULT_FLAG_PERMISSION,
+ )
+ .unwrap();
+ crate::protos::parsed_flags::try_from_binary_proto(&bytes).unwrap()
+ }
+
pub fn parse_test_flags() -> ProtoParsedFlags {
let bytes = crate::commands::parse_flags(
"com.android.aconfig.test",
diff --git a/tools/aconfig/templates/cpp_exported_header.template b/tools/aconfig/templates/cpp_exported_header.template
index cc1b18d..377295d 100644
--- a/tools/aconfig/templates/cpp_exported_header.template
+++ b/tools/aconfig/templates/cpp_exported_header.template
@@ -5,12 +5,14 @@
#ifndef {package_macro}
#define {package_macro}(FLAG) {package_macro}_##FLAG
#endif
-{{ for item in class_elements- }}
+{{ for item in class_elements }}
+
{{ if item.is_fixed_read_only- }}
#ifndef {package_macro}_{item.flag_macro}
#define {package_macro}_{item.flag_macro} {item.default_value}
#endif
-{{ endif }}
+{{ -endif }}
+
{{ -endfor }}
{{ -endif }}
{{ -endif }}
@@ -27,7 +29,7 @@
{{ for item in class_elements}}
virtual bool {item.flag_name}() = 0;
- {{ if for_test }}
+ {{ if for_test- }}
virtual void {item.flag_name}(bool val) = 0;
{{ -endif }}
{{ -endfor }}
@@ -41,13 +43,13 @@
{{ for item in class_elements}}
inline bool {item.flag_name}() \{
- {{ if for_test }}
+ {{ if for_test- }}
return provider_->{item.flag_name}();
{{ -else- }}
{{ if item.readwrite- }}
return provider_->{item.flag_name}();
{{ -else- }}
- {{ if item.is_fixed_read_only }}
+ {{ if item.is_fixed_read_only- }}
return {package_macro}_{item.flag_macro};
{{ -else- }}
return {item.default_value};
@@ -56,14 +58,14 @@
{{ -endif }}
}
-{{ if for_test }}
+{{ if for_test- }}
inline void {item.flag_name}(bool val) \{
provider_->{item.flag_name}(val);
}
{{ -endif }}
{{ -endfor }}
-{{ if for_test }}
+{{ if for_test- }}
inline void reset_flags() \{
return provider_->reset_flags();
}
@@ -77,12 +79,12 @@
{{ for item in class_elements }}
bool {header}_{item.flag_name}();
-{{ if for_test }}
+{{ if for_test- }}
void set_{header}_{item.flag_name}(bool val);
{{ -endif }}
{{ -endfor }}
-{{ if for_test }}
+{{ if for_test- }}
void {header}_reset_flags();
{{ -endif }}
diff --git a/tools/aconfig/templates/cpp_source_file.template b/tools/aconfig/templates/cpp_source_file.template
index 1bfa4b6..fbbfedc 100644
--- a/tools/aconfig/templates/cpp_source_file.template
+++ b/tools/aconfig/templates/cpp_source_file.template
@@ -1,17 +1,21 @@
#include "{header}.h"
-{{ if readwrite }}
+
+{{ if readwrite- }}
#include <server_configurable_flags/get_flags.h>
-{{ endif }}
-{{ if for_test }}
+{{ -endif }}
+
+{{ if for_test- }}
#include <unordered_map>
#include <string>
{{ -else- }}
+{{ if readwrite- }}
#include <vector>
-{{ endif }}
+{{ -endif }}
+{{ -endif }}
namespace {cpp_namespace} \{
-{{ if for_test }}
+{{ if for_test- }}
class flag_provider : public flag_provider_interface \{
private:
std::unordered_map<std::string, bool> overrides_;
@@ -21,7 +25,7 @@
: overrides_()
\{}
- {{ for item in class_elements}}
+ {{ for item in class_elements }}
virtual bool {item.flag_name}() override \{
auto it = overrides_.find("{item.flag_name}");
if (it != overrides_.end()) \{
@@ -41,7 +45,7 @@
virtual void {item.flag_name}(bool val) override \{
overrides_["{item.flag_name}"] = val;
}
- {{ endfor }}
+ {{ -endfor }}
virtual void reset_flags() override \{
overrides_.clear();
@@ -52,7 +56,8 @@
class flag_provider : public flag_provider_interface \{
public:
- {{ for item in class_elements}}
+
+ {{ for item in class_elements }}
virtual bool {item.flag_name}() override \{
{{ if item.readwrite- }}
if (cache_[{item.readwrite_idx}] == -1) \{
@@ -71,8 +76,10 @@
{{ -endif }}
}
{{ endfor }}
+ {{ if readwrite- }}
private:
std::vector<int8_t> cache_ = std::vector<int8_t>({readwrite_count}, -1);
+ {{ -endif }}
};
@@ -82,10 +89,9 @@
std::make_unique<flag_provider>();
}
-
-{{ for item in class_elements}}
+{{ for item in class_elements }}
bool {header}_{item.flag_name}() \{
- {{ if for_test }}
+ {{ if for_test- }}
return {cpp_namespace}::{item.flag_name}();
{{ -else- }}
{{ if item.readwrite- }}
@@ -100,12 +106,12 @@
{{ -endif }}
}
-{{ if for_test }}
+{{ if for_test- }}
void set_{header}_{item.flag_name}(bool val) \{
{cpp_namespace}::{item.flag_name}(val);
}
{{ -endif }}
-{{ endfor -}}
+{{ endfor-}}
{{ if for_test }}
void {header}_reset_flags() \{
diff --git a/tools/aconfig/tests/read_only_test.aconfig b/tools/aconfig/tests/read_only_test.aconfig
new file mode 100644
index 0000000..5eb5056
--- /dev/null
+++ b/tools/aconfig/tests/read_only_test.aconfig
@@ -0,0 +1,32 @@
+package: "com.android.aconfig.test"
+container: "system"
+
+flag {
+ name: "enabled_ro"
+ namespace: "aconfig_test"
+ description: "This flag is ENABLED + READ_ONLY"
+ bug: "abc"
+}
+
+flag {
+ name: "disabled_ro"
+ namespace: "aconfig_test"
+ description: "This flag is DISABLED + READ_ONLY"
+ bug: "123"
+}
+
+flag {
+ name: "enabled_fixed_ro"
+ namespace: "aconfig_test"
+ description: "This flag is fixed READ_ONLY + ENABLED"
+ bug: ""
+ is_fixed_read_only: true
+}
+
+flag {
+ name: "disabled_fixed_ro"
+ namespace: "aconfig_test"
+ description: "This flag is fixed READ_ONLY + DISABLED"
+ bug: ""
+ is_fixed_read_only: true
+}
diff --git a/tools/aconfig/tests/read_only_test.values b/tools/aconfig/tests/read_only_test.values
new file mode 100644
index 0000000..349c7aa
--- /dev/null
+++ b/tools/aconfig/tests/read_only_test.values
@@ -0,0 +1,18 @@
+flag_value {
+ package: "com.android.aconfig.test"
+ name: "disabled_ro"
+ state: DISABLED
+ permission: READ_ONLY
+}
+flag_value {
+ package: "com.android.aconfig.test"
+ name: "enabled_ro"
+ state: ENABLED
+ permission: READ_ONLY
+}
+flag_value {
+ package: "com.android.aconfig.test"
+ name: "enabled_fixed_ro"
+ state: ENABLED
+ permission: READ_ONLY
+}