Changeset 8f557161

Timestamp:

Jun 13, 2023, 4:33:16 PM (18 months ago)

Author:

Michael Brooks <mlbrooks@…>

Branches:

master

Children:

5668740, fec8bd1

Parents:

576aadb

Message:

Clarify and fix accuracy in eval public API, on reporting "unable to evaluate."

While the eval internals always used the flag pair valid and cfavalid almost correctly, the public interface exposed the outcome as a single flag, and the various interpretations of this flag were a mess.

Old cfacc treated sizeof(whatever) in some contexts as "known to be zero," which is wrong.
The generally correct answer, which new cfacc now uses is, "unknown now, but GCC will see it as a fine constant."
New tests/eval.cfa captures this fact: is runnable and would fail on old cfacc; it passes with new cfacc.

Files:

• src/AST/Decl.cpp (modified) (1 diff)
• src/Common/Eval.cc (modified) (4 diffs)
• src/Common/Eval.h (modified) (1 diff)
• src/ResolvExpr/CurrentObject.cc (modified) (2 diffs)
• src/ResolvExpr/Resolver.cc (modified) (1 diff)
• tests/.expect/eval.txt (added)
• tests/eval.cfa (added)

src/AST/Decl.cpp

@@ -142 +142 @@
 bool EnumDecl::valueOf( const Decl * enumerator, long long& value ) const {
         if ( enumValues.empty() ) {
-                long long crntVal = 0;
+                Evaluation crntVal = {0, true, true};  // until expression is given, we know to start counting from 0
                 for ( const Decl * member : members ) {
                         const ObjectDecl* field = strict_dynamic_cast< const ObjectDecl* >( member );
                         if ( field->init ) {
                                 const SingleInit * init = strict_dynamic_cast< const SingleInit* >( field->init.get() );
-                                auto result = eval( init->value );
-                                if ( ! result.second ) {
+                                crntVal = eval( init->value );
+                                if ( ! crntVal.isEvaluableInGCC ) {
                                         SemanticError( init->location, ::toString( "Non-constexpr in initialization of "
                                                 "enumerator: ", field ) );
                                 }
-                                crntVal = result.first;
                         }
                         if ( enumValues.count( field->name ) != 0 ) {
                                 SemanticError( location, ::toString( "Enum ", name, " has multiple members with the "   "name ", field->name ) );
                         }
-                        enumValues[ field->name ] = crntVal;
-                        ++crntVal;
+                        if (crntVal.hasKnownValue) {
+                                enumValues[ field->name ] = crntVal.knownValue;
+                        }
+                        ++crntVal.knownValue;
                 }
         }

src/Common/Eval.cc

@@ -96 +96 @@
 // New AST
 struct EvalNew : public ast::WithShortCircuiting {
-        long long int value = 0;                                                        // compose the result of the constant expression
-        bool valid = true;                                                                      // true => constant expression and value is the result
-                                                                                                                // false => not constant expression, e.g., ++i
-        bool cfavalid = true;                                                           // true => constant expression and value computable
-                                                                                                                // false => constant expression but value not computable, e.g., sizeof(int)
+        Evaluation result = { 0, true, true };
 
         void previsit( const ast::Node * ) { visit_children = false; }
-        void postvisit( const ast::Node * ) { cfavalid = valid = false; }
+        void postvisit( const ast::Node * ) { result.isEvaluableInGCC = result.hasKnownValue = false; }
 
         void postvisit( const ast::UntypedExpr * ) {
@@ -110 +106 @@
 
         void postvisit( const ast::ConstantExpr * expr ) {      // only handle int constants
-                value = expr->intValue();
+                result.knownValue = expr->intValue();
+                result.hasKnownValue = true;
+                result.isEvaluableInGCC = true;
         }
 
         void postvisit( const ast::SizeofExpr * ) {
-                // do not change valid or value => let C figure it out
-                cfavalid = false;
+                result.hasKnownValue = false;
+                result.isEvaluableInGCC = true;
         }
 
         void postvisit( const ast::AlignofExpr * ) {
-                // do not change valid or value => let C figure it out
-                cfavalid = false;
+                result.hasKnownValue = false;
+                result.isEvaluableInGCC = true;
         }
 
         void postvisit( const ast::OffsetofExpr * ) {
-                // do not change valid or value => let C figure it out
-                cfavalid = false;
+                result.hasKnownValue = false;
+                result.isEvaluableInGCC = true;
         }
 
         void postvisit( const ast::LogicalExpr * expr ) {
-                std::pair<long long int, bool> arg1, arg2;
+                Evaluation arg1, arg2;
                 arg1 = eval( expr->arg1 );
-                valid &= arg1.second;
-                if ( ! valid ) return;
+                result.isEvaluableInGCC &= arg1.isEvaluableInGCC;
+                if ( ! result.isEvaluableInGCC ) return;
                 arg2 = eval( expr->arg2 );
-                valid &= arg2.second;
-                if ( ! valid ) return;
+                result.isEvaluableInGCC &= arg2.isEvaluableInGCC;
+                if ( ! result.isEvaluableInGCC ) return;
+
+                result.hasKnownValue &= arg1.hasKnownValue;
+                result.hasKnownValue &= arg2.hasKnownValue;
+                if ( ! result.hasKnownValue ) return;
 
                 if ( expr->isAnd ) {
-                        value = arg1.first && arg2.first;
+                        result.knownValue = arg1.knownValue && arg2.knownValue;
                 } else {
-                        value = arg1.first || arg2.first;
+                        result.knownValue = arg1.knownValue || arg2.knownValue;
                 } // if
         }
 
         void postvisit( const ast::ConditionalExpr * expr ) {
-                std::pair<long long int, bool> arg1, arg2, arg3;
+                Evaluation arg1, arg2, arg3;
                 arg1 = eval( expr->arg1 );
-                valid &= arg1.second;
-                if ( ! valid ) return;
+                result.isEvaluableInGCC &= arg1.isEvaluableInGCC;
+                if ( ! result.isEvaluableInGCC ) return;
                 arg2 = eval( expr->arg2 );
-                valid &= arg2.second;
-                if ( ! valid ) return;
+                result.isEvaluableInGCC &= arg2.isEvaluableInGCC;
+                if ( ! result.isEvaluableInGCC ) return;
                 arg3 = eval( expr->arg3 );
-                valid &= arg3.second;
-                if ( ! valid ) return;
-
-                value = arg1.first ? arg2.first : arg3.first;
+                result.isEvaluableInGCC &= arg3.isEvaluableInGCC;
+                if ( ! result.isEvaluableInGCC ) return;
+
+                result.hasKnownValue &= arg1.hasKnownValue;
+                result.hasKnownValue &= arg2.hasKnownValue;
+                result.hasKnownValue &= arg3.hasKnownValue;
+                if ( ! result.hasKnownValue ) return;
+
+                result.knownValue = arg1.knownValue ? arg2.knownValue : arg3.knownValue;
         }
 
         void postvisit( const ast::CastExpr * expr ) {          
-                // cfa-cc generates a cast before every constant and many other places, e.g., (int)3, so the cast argument must
-                // be evaluated to get the constant value.
-                auto arg = eval(expr->arg);
-                valid = arg.second;
-                value = arg.first;
-                cfavalid = false;
+                // cfa-cc generates a cast before every constant and many other places, e.g., (int)3, 
+                // so we must use the value from the cast argument, even though we lack any basis for evaluating wraparound effects, etc
+                result = eval(expr->arg);
         }
 
         void postvisit( const ast::VariableExpr * expr ) {
+                result.hasKnownValue = false;
+                result.isEvaluableInGCC = false;
                 if ( const ast::EnumInstType * inst = dynamic_cast<const ast::EnumInstType *>(expr->result.get()) ) {
                         if ( const ast::EnumDecl * decl = inst->base ) {
-                                if ( decl->valueOf( expr->var, value ) ) { // value filled by valueOf
-                                        return;
-                                }
+                                result.isEvaluableInGCC = true;
+                                result.hasKnownValue = decl->valueOf( expr->var, result.knownValue ); // result.knownValue filled by valueOf
                         }
                 }
-                valid = false;
         }
 
         void postvisit( const ast::ApplicationExpr * expr ) {
                 const ast::DeclWithType * function = ast::getFunction(expr);
-                if ( ! function || function->linkage != ast::Linkage::Intrinsic ) { valid = false; return; }
+                if ( ! function || function->linkage != ast::Linkage::Intrinsic ) { 
+                        result.isEvaluableInGCC = false;
+                        result.hasKnownValue = false;
+                        return;
+                }
                 const std::string & fname = function->name;
                 assertf( expr->args.size() == 1 || expr->args.size() == 2, "Intrinsic function with %zd arguments: %s", expr->args.size(), fname.c_str() );
@@ -187 +195 @@
                 if ( expr->args.size() == 1 ) {
                         // pre/postfix operators ++ and -- => assignment, which is not constant
-                        std::pair<long long int, bool> arg1;
+                        Evaluation arg1;
                         arg1 = eval(expr->args.front());
-                        valid &= arg1.second;
-                        if ( ! valid ) return;
+                        result.isEvaluableInGCC &= arg1.isEvaluableInGCC;
+                        if ( ! result.isEvaluableInGCC ) return;
+
+                        result.hasKnownValue &= arg1.hasKnownValue;
+                        if ( ! result.hasKnownValue ) return;
 
                         if (fname == "+?") {
-                                value = arg1.first;
+                                result.knownValue = arg1.knownValue;
                         } else if (fname == "-?") {
-                                value = -arg1.first;
+                                result.knownValue = -arg1.knownValue;
                         } else if (fname == "~?") {
-                                value = ~arg1.first;
+                                result.knownValue = ~arg1.knownValue;
                         } else if (fname == "!?") {
-                                value = ! arg1.first;
+                                result.knownValue = ! arg1.knownValue;
                         } else {
-                                valid = false;
+                                result.isEvaluableInGCC = false;
+                                result.hasKnownValue = false;
                         } // if
                 } else { // => expr->args.size() == 2
                         // infix assignment operators => assignment, which is not constant
-                        std::pair<long long int, bool> arg1, arg2;
+                        Evaluation arg1, arg2;
                         arg1 = eval(expr->args.front());
-                        valid &= arg1.second;
-                        if ( ! valid ) return;
+                        result.isEvaluableInGCC &= arg1.isEvaluableInGCC;
+                        if ( ! result.isEvaluableInGCC ) return;
                         arg2 = eval(expr->args.back());
-                        valid &= arg2.second;
-                        if ( ! valid ) return;
+                        result.isEvaluableInGCC &= arg2.isEvaluableInGCC;
+                        if ( ! result.isEvaluableInGCC ) return;
+
+                        result.hasKnownValue &= arg1.hasKnownValue;
+                        result.hasKnownValue &= arg2.hasKnownValue;
+                        if ( ! result.hasKnownValue ) return;
 
                         if (fname == "?+?") {
-                                value = arg1.first + arg2.first;
+                                result.knownValue = arg1.knownValue + arg2.knownValue;
                         } else if (fname == "?-?") {
-                                value = arg1.first - arg2.first;
+                                result.knownValue = arg1.knownValue - arg2.knownValue;
                         } else if (fname == "?*?") {
-                                value = arg1.first * arg2.first;
+                                result.knownValue = arg1.knownValue * arg2.knownValue;
                         } else if (fname == "?/?") {
-                                if ( arg2.first ) value = arg1.first / arg2.first;
+                                if ( arg2.knownValue ) result.knownValue = arg1.knownValue / arg2.knownValue;
                         } else if (fname == "?%?") {
-                                if ( arg2.first ) value = arg1.first % arg2.first;
+                                if ( arg2.knownValue ) result.knownValue = arg1.knownValue % arg2.knownValue;
                         } else if (fname == "?<<?") {
-                                value = arg1.first << arg2.first;
+                                result.knownValue = arg1.knownValue << arg2.knownValue;
                         } else if (fname == "?>>?") {
-                                value = arg1.first >> arg2.first;
+                                result.knownValue = arg1.knownValue >> arg2.knownValue;
                         } else if (fname == "?<?") {
-                                value = arg1.first < arg2.first;
+                                result.knownValue = arg1.knownValue < arg2.knownValue;
                         } else if (fname == "?>?") {
-                                value = arg1.first > arg2.first;
+                                result.knownValue = arg1.knownValue > arg2.knownValue;
                         } else if (fname == "?<=?") {
-                                value = arg1.first <= arg2.first;
+                                result.knownValue = arg1.knownValue <= arg2.knownValue;
                         } else if (fname == "?>=?") {
-                                value = arg1.first >= arg2.first;
+                                result.knownValue = arg1.knownValue >= arg2.knownValue;
                         } else if (fname == "?==?") {
-                                value = arg1.first == arg2.first;
+                                result.knownValue = arg1.knownValue == arg2.knownValue;
                         } else if (fname == "?!=?") {
-                                value = arg1.first != arg2.first;
+                                result.knownValue = arg1.knownValue != arg2.knownValue;
                         } else if (fname == "?&?") {
-                                value = arg1.first & arg2.first;
+                                result.knownValue = arg1.knownValue & arg2.knownValue;
                         } else if (fname == "?^?") {
-                                value = arg1.first ^ arg2.first;
+                                result.knownValue = arg1.knownValue ^ arg2.knownValue;
                         } else if (fname == "?|?") {
-                                value = arg1.first | arg2.first;
+                                result.knownValue = arg1.knownValue | arg2.knownValue;
                         } else {
-                                valid = false;
+                                result.isEvaluableInGCC = false;
+                                result.hasKnownValue = false;
                         }
                 } // if
@@ -263 +280 @@
 }
 
-std::pair<long long int, bool> eval( const ast::Expr * expr ) {
-        ast::Pass<EvalNew> ev;
+Evaluation eval( const ast::Expr * expr ) {
         if ( expr ) {
-                expr->accept( ev );
-                return std::make_pair( ev.core.value, ev.core.valid );
+
+                return ast::Pass<EvalNew>::read(expr);
+                // Evaluation ret = ast::Pass<EvalNew>::read(expr);
+                // ret.knownValue = 777;
+                // return ret;
+
         } else {
-                return std::make_pair( 0, false );
+                return { 0, false, false };
         }
 }

src/Common/Eval.h

@@ -23 +23 @@
 }
 
+struct Evaluation {
+        long long int knownValue;
+        bool hasKnownValue;
+        bool isEvaluableInGCC;
+};
+
 /// Evaluates expr as a long long int.
 /// If second is false, expr could not be evaluated.
 std::pair<long long int, bool> eval(const Expression * expr);
-std::pair<long long int, bool> eval(const ast::Expr * expr);
+Evaluation eval(const ast::Expr * expr);
 
 // Local Variables: //

src/ResolvExpr/CurrentObject.cc

@@ -689 +689 @@
 
                         auto arg = eval( expr );
-                        index = arg.first;
+                        assertf( arg.hasKnownValue, "Non-evaluable expression made it to IndexIterator" );
+                        index = arg.knownValue;
 
                         // if ( auto constExpr = dynamic_cast< const ConstantExpr * >( expr ) ) {
@@ -728 +729 @@
                 size_t getSize( const Expr * expr ) {
                         auto res = eval( expr );
-                        if ( !res.second ) {
+                        if ( !res.hasKnownValue ) {
                                 SemanticError( location, toString( "Array designator must be a constant expression: ", expr ) );
                         }
-                        return res.first;
+                        return res.knownValue;
                 }
 

src/ResolvExpr/Resolver.cc

@@ -1341 +1341 @@
                                         auto mutAttr = mutate(attr);
                                         mutAttr->params.front() = resolved;
-                                        if (! result.second) {
+                                        if (! result.hasKnownValue) {
                                                 SemanticWarning(loc, Warning::GccAttributes,
                                                         toCString( name, " priorities must be integers from 0 to 65535 inclusive: ", arg ) );
                                         }
                                         else {
-                                                auto priority = result.first;
+                                                auto priority = result.knownValue;
                                                 if (priority < 101) {
                                                         SemanticWarning(loc, Warning::GccAttributes,