commit 74675c265d5a3dcfab97566442d338867696ff60
author Jooyung Han <jooyung@google.com> Tue Dec 15 08:39:57 2020 +0900
committer Treehugger Robot <treehugger-gerrit@google.com> Tue Dec 15 03:41:34 2020 +0000
tree 4513e177c92754a2be65f98053d7f749e1cda285
parent 30f64adb50e9833bc29ee4aea919a754c93b5ad6

AidlConstantValue::evaluate() with no type arg

Because the result of AidlConstantValue::evaluate() is cached we should
not rely on "type" parameter. For example, if ValueString(type) is later
called with a different type, the cached result may not be valid.

Actual type-related checks/conversions are done in ValueString(type).

Bug: none
Test: aidl_unittests
Change-Id: Ic6bc36566d82a917a8cf08c1740f576be5578706

aidl_const_expressions.cpp

diff --git a/aidl_const_expressions.cpp b/aidl_const_expressions.cpp
index 8e9ebea..5509999 100644
--- a/aidl_const_expressions.cpp
+++ b/aidl_const_expressions.cpp
@@ -468,7 +468,7 @@
   if (!is_evaluated_) {
     // TODO(b/142722772) CheckValid() should be called before ValueString()
     bool success = CheckValid();
-    success &= evaluate(type);
+    success &= evaluate();
     if (!success) {
       // the detailed error message shall be printed in evaluate
       return "";
@@ -627,7 +627,7 @@
   return true;
 }
 
-bool AidlConstantValue::evaluate(const AidlTypeSpecifier& type) const {
+bool AidlConstantValue::evaluate() const {
   if (is_evaluated_) {
     return is_valid_;
   }
@@ -636,17 +636,12 @@
 
   switch (type_) {
     case Type::ARRAY: {
-      if (!type.IsArray()) {
-        AIDL_ERROR(this) << "Invalid constant array type: " << type.GetName();
-        err = -1;
-        break;
-      }
       Type array_type = Type::ERROR;
       bool success = true;
       for (const auto& value : values_) {
         success = value->CheckValid();
         if (success) {
-          success = value->evaluate(type.ArrayBase());
+          success = value->evaluate();
           if (!success) {
             AIDL_ERROR(this) << "Invalid array element: " << value->value_;
             break;
@@ -786,20 +781,12 @@
   return is_valid_;
 }
 
-bool AidlConstantReference::evaluate(const AidlTypeSpecifier& type) const {
+bool AidlConstantReference::evaluate() const {
   if (is_evaluated_) return is_valid_;
   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_;
-      return false;
-    }
-  }
 
-  resolved_->evaluate(type);
+  resolved_->evaluate();
   is_valid_ = resolved_->is_valid_;
   final_type_ = resolved_->final_type_;
   if (is_valid_) {
@@ -825,7 +812,7 @@
   return AidlConstantValue::CheckValid();
 }
 
-bool AidlUnaryConstExpression::evaluate(const AidlTypeSpecifier& type) const {
+bool AidlUnaryConstExpression::evaluate() const {
   if (is_evaluated_) {
     return is_valid_;
   }
@@ -835,7 +822,7 @@
   if (!unary_->is_evaluated_) {
     // TODO(b/142722772) CheckValid() should be called before ValueString()
     bool success = CheckValid();
-    success &= unary_->evaluate(type);
+    success &= unary_->evaluate();
     if (!success) {
       is_valid_ = false;
       return false;
@@ -895,20 +882,20 @@
   return AidlConstantValue::CheckValid();
 }
 
-bool AidlBinaryConstExpression::evaluate(const AidlTypeSpecifier& type) const {
+bool AidlBinaryConstExpression::evaluate() const {
   if (is_evaluated_) {
     return is_valid_;
   }
   is_evaluated_ = true;
-  AIDL_FATAL_IF(left_val_ == nullptr, type);
-  AIDL_FATAL_IF(right_val_ == nullptr, type);
+  AIDL_FATAL_IF(left_val_ == nullptr, this);
+  AIDL_FATAL_IF(right_val_ == nullptr, this);
 
   // Recursively evaluate the binary expression tree
   if (!left_val_->is_evaluated_ || !right_val_->is_evaluated_) {
     // TODO(b/142722772) CheckValid() should be called before ValueString()
     bool success = CheckValid();
-    success &= left_val_->evaluate(type);
-    success &= right_val_->evaluate(type);
+    success &= left_val_->evaluate();
+    success &= right_val_->evaluate();
     if (!success) {
       is_valid_ = false;
       return false;