From ac1cb3eb26525c868fd7dfeba90b6ee85161c9d8 Mon Sep 17 00:00:00 2001 From: Daniele Di Proietto Date: Fri, 3 Dec 2021 10:56:49 +0000 Subject: Revert "Add automatic default value for char-type field" This reverts commit 3cd81ac2ecc4998bfc9ace215c39d15b4d72d1b2. Reason for revert: Breaks some sc-dev builds. Bug: 208964910 Change-Id: I8649ad843224c2cec1c866b39a9ae580acfa2174 --- aidl_const_expressions.cpp | 32 +++++++++++++++++++++----------- aidl_language.h | 6 +++--- aidl_language_l.ll | 5 +++-- aidl_language_y.yy | 8 +++----- aidl_unittest.cpp | 11 +---------- 5 files changed, 31 insertions(+), 31 deletions(-) diff --git a/aidl_const_expressions.cpp b/aidl_const_expressions.cpp index b9b62ea6..b97889d6 100644 --- a/aidl_const_expressions.cpp +++ b/aidl_const_expressions.cpp @@ -242,6 +242,12 @@ bool handleLogical(const AidlConstantValue& context, bool lval, const string& op return false; } +static bool isValidLiteralChar(char c) { + return !(c <= 0x1f || // control characters are < 0x20 + c >= 0x7f || // DEL is 0x7f + c == '\\'); // Disallow backslashes for future proofing. +} + bool ParseFloating(std::string_view sv, double* parsed) { // float literal should be parsed successfully. android::base::ConsumeSuffix(&sv, "f"); @@ -332,9 +338,6 @@ AidlConstantValue* AidlConstantValue::Default(const AidlTypeSpecifier& specifier if (name == "boolean") { return Boolean(location, false); } - if (name == "char") { - return Character(location, "'\\0'"); // literal to be used in backends - } if (name == "byte" || name == "int" || name == "long") { return Integral(location, "0"); } @@ -351,9 +354,13 @@ AidlConstantValue* AidlConstantValue::Boolean(const AidlLocation& location, bool return new AidlConstantValue(location, Type::BOOLEAN, value ? "true" : "false"); } -AidlConstantValue* AidlConstantValue::Character(const AidlLocation& location, - const std::string& value) { - return new AidlConstantValue(location, Type::CHARACTER, value); +AidlConstantValue* AidlConstantValue::Character(const AidlLocation& location, char value) { + const std::string explicit_value = string("'") + value + "'"; + if (!isValidLiteralChar(value)) { + AIDL_ERROR(location) << "Invalid character literal " << value; + return new AidlConstantValue(location, Type::ERROR, explicit_value); + } + return new AidlConstantValue(location, Type::CHARACTER, explicit_value); } AidlConstantValue* AidlConstantValue::Floating(const AidlLocation& location, @@ -446,6 +453,14 @@ AidlConstantValue* AidlConstantValue::Array( } AidlConstantValue* AidlConstantValue::String(const AidlLocation& location, const string& value) { + for (size_t i = 0; i < value.length(); ++i) { + if (!isValidLiteralChar(value[i])) { + AIDL_ERROR(location) << "Found invalid character at index " << i << " in string constant '" + << value << "'"; + return new AidlConstantValue(location, Type::ERROR, value); + } + } + return new AidlConstantValue(location, Type::STRING, value); } @@ -992,8 +1007,6 @@ bool AidlBinaryConstExpression::evaluate() const { return false; } -// Constructor for integer(byte, int, long) -// Keep parsed integer & literal AidlConstantValue::AidlConstantValue(const AidlLocation& location, Type parsed_type, int64_t parsed_value, const string& checked_value) : AidlNode(location), @@ -1005,8 +1018,6 @@ AidlConstantValue::AidlConstantValue(const AidlLocation& location, Type parsed_t AIDL_FATAL_IF(type_ != Type::INT8 && type_ != Type::INT32 && type_ != Type::INT64, location); } -// Constructor for non-integer(String, char, boolean, float, double) -// Keep literal as it is. (e.g. String literal has double quotes at both ends) AidlConstantValue::AidlConstantValue(const AidlLocation& location, Type type, const string& checked_value) : AidlNode(location), @@ -1026,7 +1037,6 @@ AidlConstantValue::AidlConstantValue(const AidlLocation& location, Type type, } } -// Constructor for array AidlConstantValue::AidlConstantValue(const AidlLocation& location, Type type, std::unique_ptr>> values, const std::string& value) diff --git a/aidl_language.h b/aidl_language.h index 931c606c..e0c28a03 100644 --- a/aidl_language.h +++ b/aidl_language.h @@ -608,7 +608,7 @@ class AidlConstantValue : public AidlNode { } else if constexpr (is_one_of::value) { AIDL_FATAL_IF(final_type_ < Type::INT8 && final_type_ > Type::INT64, this); return static_cast(final_value_); - } else if constexpr (std::is_same::value) { + } else if constexpr (std::is_same::value) { AIDL_FATAL_IF(final_type_ != Type::CHARACTER, this); return final_string_value_.at(1); // unquote ' } else if constexpr (std::is_same::value) { @@ -633,9 +633,9 @@ class AidlConstantValue : public AidlNode { static AidlConstantValue* Default(const AidlTypeSpecifier& specifier); static AidlConstantValue* Boolean(const AidlLocation& location, bool value); - static AidlConstantValue* Character(const AidlLocation& location, const std::string& value); + static AidlConstantValue* Character(const AidlLocation& location, char value); // example: 123, -5498, maybe any size - static AidlConstantValue* Integral(const AidlLocation& location, const std::string& value); + static AidlConstantValue* Integral(const AidlLocation& location, const string& value); static AidlConstantValue* Floating(const AidlLocation& location, const std::string& value); static AidlConstantValue* Array(const AidlLocation& location, std::unique_ptr>> values); diff --git a/aidl_language_l.ll b/aidl_language_l.ll index 2b63b822..e43ef452 100644 --- a/aidl_language_l.ll +++ b/aidl_language_l.ll @@ -142,8 +142,9 @@ union { yylval->token = new AidlToken("union", comments); {identifier} { yylval->token = new AidlToken(yytext, comments); return yy::parser::token::IDENTIFIER; } -'.' { yylval->token = new AidlToken(yytext, comments); - return yy::parser::token::CHARVALUE; } +'.' { yylval->character = yytext[1]; + return yy::parser::token::CHARVALUE; + } {intvalue} { yylval->token = new AidlToken(yytext, comments); return yy::parser::token::INTVALUE; } {floatvalue} { yylval->token = new AidlToken(yytext, comments); diff --git a/aidl_language_y.yy b/aidl_language_y.yy index 079bda67..ab3e4799 100644 --- a/aidl_language_y.yy +++ b/aidl_language_y.yy @@ -96,6 +96,7 @@ AidlLocation loc(const yy::parser::location_type& l) { std::vector>* declarations; } +%destructor { } %destructor { } %destructor { delete ($$); } <*> @@ -111,7 +112,7 @@ AidlLocation loc(const yy::parser::location_type& l) { %token UNION "union" %token CONST "const" -%token CHARVALUE "char literal" +%token CHARVALUE "char literal" %token FLOATVALUE "float literal" %token HEXVALUE "hex literal" %token INTVALUE "int literal" @@ -394,10 +395,7 @@ interface_members const_expr : TRUE_LITERAL { $$ = AidlConstantValue::Boolean(loc(@1), true); } | FALSE_LITERAL { $$ = AidlConstantValue::Boolean(loc(@1), false); } - | CHARVALUE { - $$ = AidlConstantValue::Character(loc(@1), $1->GetText()); - delete $1; - } + | CHARVALUE { $$ = AidlConstantValue::Character(loc(@1), $1); } | INTVALUE { $$ = AidlConstantValue::Integral(loc(@1), $1->GetText()); if ($$ == nullptr) { diff --git a/aidl_unittest.cpp b/aidl_unittest.cpp index f825cfd2..7ea8c171 100644 --- a/aidl_unittest.cpp +++ b/aidl_unittest.cpp @@ -32,7 +32,6 @@ #include "aidl_language.h" #include "aidl_to_cpp.h" #include "aidl_to_java.h" -#include "aidl_to_ndk.h" #include "comments.h" #include "logging.h" #include "options.h" @@ -1026,7 +1025,7 @@ TEST_F(AidlTest, AidlConstantValue_EvaluatedValue) { using Ptr = unique_ptr; const AidlLocation& loc = AIDL_LOCATION_HERE; - EXPECT_EQ('c', Ptr(AidlConstantValue::Character(loc, "'c'"))->EvaluatedValue()); + EXPECT_EQ('c', Ptr(AidlConstantValue::Character(loc, 'c'))->EvaluatedValue()); EXPECT_EQ("abc", Ptr(AidlConstantValue::String(loc, "\"abc\""))->EvaluatedValue()); EXPECT_FLOAT_EQ(1.0f, Ptr(AidlConstantValue::Floating(loc, "1.0f"))->EvaluatedValue()); EXPECT_EQ(true, Ptr(AidlConstantValue::Boolean(loc, true))->EvaluatedValue()); @@ -1044,14 +1043,6 @@ TEST_F(AidlTest, AidlConstantValue_EvaluatedValue) { Ptr(AidlConstantValue::Array(loc, std::move(values)))->EvaluatedValue>()); } -TEST_F(AidlTest, AidlConstantCharacterDefault) { - AidlTypeSpecifier char_type(AIDL_LOCATION_HERE, "char", false, nullptr, {}); - auto default_value = unique_ptr(AidlConstantValue::Default(char_type)); - EXPECT_EQ("'\\0'", default_value->ValueString(char_type, cpp::ConstantValueDecorator)); - EXPECT_EQ("'\\0'", default_value->ValueString(char_type, ndk::ConstantValueDecorator)); - EXPECT_EQ("'\\0'", default_value->ValueString(char_type, java::ConstantValueDecorator)); -} - TEST_P(AidlTest, FailOnManyDefinedTypes) { AidlError error; const string expected_stderr = -- cgit v1.2.3