Minor tweaks (#4106)

* Minor tweaks

- Expression::toString() generate actual expression strings
- LocalCopyProp typechecking arg optional
- json factor common code
* Better name tracking for error messages
* Additional slice-related strength reductions
  - slice on slice
* Type_Specialized ctor sugar
* Fix brief dbprint of ParserState
* update expected error messages

---------

Co-authored-by: Chris Dodd <[email protected]>

frontends/p4/fromv1.0/converters.cpp

@@ -221,8 +221,7 @@ const IR::Node *ExpressionConverter::postorder(IR::GlobalRef *ref) {
     // FIXME -- has put into a different control.  In that case, ResolveReferences on this
     // FIXME -- path will later fail as the declaration is not in scope.  We should at
     // FIXME -- least detect that here and give a warning or other indication of the problem.
-    return new IR::PathExpression(
-        ref->srcInfo, new IR::Path(ref->srcInfo, IR::ID(ref->srcInfo, ref->toString())));
+    return new IR::PathExpression(ref->srcInfo, new IR::Path(ref->srcInfo, ref->Name()));
 }
 
 /// P4_16 is stricter on comparing booleans with ints

frontends/p4/strengthReduction.cpp

@@ -413,10 +413,12 @@ const IR::Node *DoStrengthReduction::postorder(IR::Slice *expr) {
 
     // out-of-bound error has been caught in type checking
     if (auto sl = expr->e0->to<IR::Slice>()) {
-        auto e = sl->e0;
-        auto hi = expr->getH() + sl->getL();
-        auto lo = expr->getL() + sl->getL();
-        return new IR::Slice(e, hi, lo);
+        int delta = sl->getL();
+        expr->e0 = sl->e0;
+        if (delta != 0) {
+            expr->e1 = new IR::Constant(expr->getH() + delta);
+            expr->e2 = new IR::Constant(expr->getL() + delta);
+        }
     }
 
     auto slice_width = expr->getH() - expr->getL() + 1;

ir/base.def

@@ -379,7 +379,7 @@ interface IAnnotated {
 }
 
 interface IInstance {
-    virtual cstring Name() const = 0;
+    virtual ID Name() const = 0;
     virtual Type getType() const = 0;
 }
 

ir/dbprint-p4.cpp

@@ -108,7 +108,9 @@ void IR::V1Parser::dbprint(std::ostream &out) const {
 }
 void IR::ParserException::dbprint(std::ostream &out) const { out << "IR::ParserException"; }
 void IR::ParserState::dbprint(std::ostream &out) const {
-    out << "state " << name << " " << annotations << "{" << indent;
+    out << "state " << name;
+    if (dbgetflags(out) & Brief) return;
+    out << " " << annotations << "{" << indent;
     for (auto s : components) out << Log::endl << s;
     if (selectExpression) out << Log::endl << selectExpression;
     out << " }" << unindent;

ir/expression.def

@@ -12,6 +12,7 @@ abstract Operation_Unary : Operation {
         if (!srcInfo && expr) srcInfo = expr->srcInfo;
         if (type->is<Type::Unknown>() && expr) type = expr->type; }
     precedence = DBPrint::Prec_Prefix;
+    toString{ return getStringOp() + expr->toString(); }
 }
 
 class Neg : Operation_Unary {
@@ -37,6 +38,11 @@ abstract Operation_Binary : Operation {
         if (!srcInfo && left && right) srcInfo = left->srcInfo + right->srcInfo;
         if (type->is<Type::Unknown>() && left && right && left->type == right->type)
             type = left->type; }
+    toString {
+        std::stringstream tmp;
+        tmp << DBPrint::Prec_Low << *this;
+        return tmp.str();
+    }
 }
 
 abstract Operation_Ternary : Operation {
@@ -374,7 +380,7 @@ class Cast : Operation_Unary {
     /// type, and 'type' will only be updated later when type inferencing occurs
     precedence = DBPrint::Prec_Prefix;
     stringOp = "(cast)";
-    toString{ return "cast"; }
+    toString{ return "(" + destType->toString() + ")" + expr->toString(); }
     validate{ BUG_CHECK(!destType->is<Type_Unknown>(), "%1%: Cannot cast to unknown type", this); }
 }
 

ir/ir.def

@@ -404,7 +404,7 @@ class Declaration_Instance : Declaration, IAnnotated, IInstance {
 
     Annotations getAnnotations() const override { return annotations; }
     Type getType() const override { return type; }
-    cstring Name() const override { return name; }
+    ID Name() const override { return name; }
     validate{ BUG_CHECK(type->is<Type_Name>() ||
                         type->is<Type_Specialized>() ||
                         type->is<Type_Extern>(),        // P4_14 only?

ir/json_generator.h

@@ -195,29 +195,7 @@ class JSONGenerator {
 
     void generate(cstring v) {
         if (v) {
-            out << "\"";
-            for (auto ch : v) {
-                switch (ch) {
-                    case '\n':
-                        out << "\\n";
-                        break;
-                    case '\r':
-                        out << "\\r";
-                        break;
-                    case '\t':
-                        out << "\\t";
-                        break;
-                    case '\"':
-                        out << "\\\"";
-                        break;
-                    case '\\':
-                        out << "\\\\";
-                        break;
-                    default:
-                        out << ch;
-                }
-            }
-            out << "\"";
+            out << "\"" << v.escapeJson() << "\"";
         } else {
             out << "null";
         }

ir/type.def

@@ -533,6 +533,8 @@ class Type_Specialized : Type {
         result += ">";
         return result;
     }
+    Type_Specialized(cstring bt, std::initializer_list<Type> args)
+    : baseType(new Type_Name(bt)), arguments(new Vector<Type>(args)) {}
 }
 
 /** Canonical representation of a Type_Specialized;

ir/v1.def

@@ -272,7 +272,7 @@ class ParserException {}
 abstract Attached : IInstance, IAnnotated {
     optional ID                 name;
     optional Annotations        annotations = Annotations::empty;
-    cstring Name() const override { return name; }
+    ID Name() const override { return name; }
     virtual const char *kind() const = 0;
     Type getType() const override { return Type_Unknown::get(); }
     Annotations getAnnotations() const override { return annotations; }
@@ -407,7 +407,7 @@ class V1Table : IInstance, IAnnotated {
     void addProperty(Property prop) { properties.push_back(prop); }
     Annotations getAnnotations() const override { return annotations; }
     toString { return node_type_name() + " " + name; }
-    cstring Name() const override { return name; }
+    ID Name() const override { return name; }
     Type getType() const override { return Type_AnyTable::get(); }
 }
 
@@ -449,7 +449,8 @@ class GlobalRef : Expression {
                                 // FIXME -- interface references directly in the IR
     GlobalRef { type = obj->to<IInstance>()->getType(); }
     validate { obj->is<IInstance>(); }
-    toString { return obj->to<IInstance>()->Name(); }
+    toString { return obj->to<IInstance>()->toString(); }
+    ID Name() const { return obj->to<IInstance>()->Name(); }
     dbprint { out << obj->to<IInstance>()->Name(); }
 }
 

midend/local_copyprop.h

@@ -150,6 +150,9 @@ class LocalCopyPropagation : public PassManager {
         passes.push_back(typeChecking);
         passes.push_back(new DoLocalCopyPropagation(refMap, typeMap, policy, elimUnusedTables));
     }
+    LocalCopyPropagation(ReferenceMap *refMap, TypeMap *typeMap,
+                         std::function<bool(const Context *, const IR::Expression *)> policy)
+        : LocalCopyPropagation(refMap, typeMap, nullptr, policy) {}
 };
 
 }  // namespace P4

testdata/p4_14_errors_outputs/issue905.p4-stderr

@@ -1,4 +1,4 @@
-issue905.p4(34): [--Werror=type-error] error: *: not defined on operands of type varbit<0>
+issue905.p4(34): [--Werror=type-error] error: can_data.value * 2: not defined on operands of type varbit<0>
  modify_field(can_data.value, can_data.value * 2); // error
                                              ^
 [--Werror=overlimit] error: 1 errors encountered, aborting compilation

testdata/p4_16_errors_outputs/binary_e.p4-stderr

@@ -4,12 +4,12 @@ binary_e.p4(32): [--Werror=type-error] error: Indexing a[2] applied to non-array
 binary_e.p4(33): [--Werror=type-error] error: Array index d must be an integer, but it has type bool
         c = stack[d]; // indexing with bool
                   ^
-binary_e.p4(35): [--Werror=type-error] error: &: Cannot operate on values with different widths 2 and 4
+binary_e.p4(35): [--Werror=type-error] error: e & f: Cannot operate on values with different widths 2 and 4
         f = e & f; // different width
             ^^^^^
-binary_e.p4(38): [--Werror=type-error] error: <: not defined on bool and bool
+binary_e.p4(38): [--Werror=type-error] error: d < d: not defined on bool and bool
         d = d < d; // not defined on bool
             ^^^^^
-binary_e.p4(39): [--Werror=type-error] error: >: not defined on int<2> and int<4>
+binary_e.p4(39): [--Werror=type-error] error: a > c: not defined on int<2> and int<4>
         d = a > c; // different width
             ^^^^^

testdata/p4_16_errors_outputs/const1_e.p4-stderr

@@ -1,6 +1,6 @@
-const1_e.p4(19): [--Werror=invalid] error: +: operands have different types: bit<32> and bit<16>
+const1_e.p4(19): [--Werror=invalid] error: 5 + 3: operands have different types: bit<32> and bit<16>
     x = 32w5 + 16w3;
         ^^^^^^^^^^^
-const1_e.p4(21): [--Werror=invalid] error: /: Division by zero
+const1_e.p4(21): [--Werror=invalid] error: 5 / 0: Division by zero
     x = 5 / 0;
         ^^^^^

testdata/p4_16_errors_outputs/div.p4-stderr

@@ -1,9 +1,9 @@
-div.p4(18): [--Werror=invalid] error: /: Division is not defined for negative numbers
+div.p4(18): [--Werror=invalid] error: -8 / -2: Division is not defined for negative numbers
     a = - 8 / -2;
         ^^^^^^^^
-div.p4(19): [--Werror=invalid] error: %: Modulo is not defined for negative numbers
+div.p4(19): [--Werror=invalid] error: -10 % 2: Modulo is not defined for negative numbers
     a = -10 % 2;
         ^^^^^^^
-div.p4(20): [--Werror=invalid] error: %: Modulo is not defined for negative numbers
+div.p4(20): [--Werror=invalid] error: 10 % -2: Modulo is not defined for negative numbers
     a = 10 % -2;
         ^^^^^^^

testdata/p4_16_errors_outputs/div0.p4-stderr

@@ -1,12 +1,12 @@
-div0.p4(18): [--Werror=expr] error: /: Division by zero
+div0.p4(18): [--Werror=expr] error: a / 0: Division by zero
     a = a / 0;
         ^^^^^
-div0.p4(19): [--Werror=expr] error: %: Modulo by zero
+div0.p4(19): [--Werror=expr] error: a % 0: Modulo by zero
     a = a % 0;
         ^^^^^
-div0.p4(20): [--Werror=expr] error: /: Division by zero
+div0.p4(20): [--Werror=expr] error: a / 0: Division by zero
     a = a / 8w0;
         ^^^^^^^
-div0.p4(21): [--Werror=expr] error: %: Modulo by zero
+div0.p4(21): [--Werror=expr] error: a % 0: Modulo by zero
     a = a % 8w0;
         ^^^^^^^

testdata/p4_16_errors_outputs/div1.p4-stderr

@@ -1,12 +1,12 @@
-div1.p4(18): [--Werror=type-error] error: /: not defined on negative numbers
+div1.p4(18): [--Werror=type-error] error: a / -1: not defined on negative numbers
     a = a / -1; // not defined for negative numbers
         ^^^^^^
-div1.p4(19): [--Werror=type-error] error: /: not defined on negative numbers
+div1.p4(19): [--Werror=type-error] error: -5 / a: not defined on negative numbers
     a = -5 / a;
         ^^^^^^
-div1.p4(20): [--Werror=type-error] error: %: not defined on negative numbers
+div1.p4(20): [--Werror=type-error] error: a % -1: not defined on negative numbers
     a = a % -1;
         ^^^^^^
-div1.p4(21): [--Werror=type-error] error: %: not defined on negative numbers
+div1.p4(21): [--Werror=type-error] error: -5 % a: not defined on negative numbers
     a = -5 % a;
         ^^^^^^

testdata/p4_16_errors_outputs/div3.p4-stderr

@@ -1,3 +1,3 @@
-div3.p4(19): [--Werror=invalid] error: /: could not evaluate expression at compilation time
+div3.p4(19): [--Werror=invalid] error: a / 3: could not evaluate expression at compilation time
         a = a / b; // not a compile-time constant
             ^^^^^

testdata/p4_16_errors_outputs/issue1542.p4-stderr

@@ -1,4 +1,4 @@
-issue1542.p4(27): [--Werror=type-error] error: cast: argument does not match declaration in actions list: cast
+issue1542.p4(27): [--Werror=type-error] error: (bit<32>)32w2: argument does not match declaration in actions list: (bit<32>)32w1
             ( true, 1, true ) : multicast(2);
                                           ^
 issue1542.p4(22)

testdata/p4_16_errors_outputs/issue2206.p4-stderr

@@ -1,3 +1,3 @@
-issue2206.p4(27): [--Werror=type-error] error: <<: width of left operand of shift needs to be specified
+issue2206.p4(27): [--Werror=type-error] error: 1 << h.h.c: width of left operand of shift needs to be specified
         h.h.a = (1 << h.h.c) + 8w2;
                  ^^^^^^^^^^

testdata/p4_16_errors_outputs/issue2444-1.p4-stderr

@@ -1,3 +1,3 @@
-issue2444-1.p4(2): [--Werror=invalid] error: cast: Only 0 and 1 can be cast to booleans
+issue2444-1.p4(2): [--Werror=invalid] error: (bool)2: Only 0 and 1 can be cast to booleans
 const bool b1 = (bool)a1;
                 ^^^^^^^^

testdata/p4_16_errors_outputs/issue2496.p4-stderr

@@ -1,3 +1,3 @@
-issue2496.p4(25): [--Werror=unsupported] error: <<: Compiler only supports widths up to 2048
+issue2496.p4(25): [--Werror=unsupported] error: 1985245330 << 903012108: Compiler only supports widths up to 2048
         h.eth_hdr.eth_type = 1985245330 << 903012108;
                              ^^^^^^^^^^^^^^^^^^^^^^^

testdata/p4_16_errors_outputs/issue3057-1.p4-stderr

@@ -1,6 +1,14 @@
-issue3057-1.p4(11): [--Werror=type-error] error: ==: cannot compare structure-valued expressions with unknown types
+issue3057-1.p4(11): [--Werror=type-error] error: {
+  a:1;
+  b:2; } == {
+  a:1;
+  b:2; }: cannot compare structure-valued expressions with unknown types
         bool b2 = {a = 32w1,b = 32w2} == {a = 32w1,b = 32w2};
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-issue3057-1.p4(12): [--Werror=type-error] error: ==: cannot compare structure-valued expressions with unknown types
+issue3057-1.p4(12): [--Werror=type-error] error: {
+  a:1;
+  b:2; } == {
+  a:1;
+  b:2; }: cannot compare structure-valued expressions with unknown types
         bool b2_ = {a = 1,b = 2} == {a = 1,b = 2};
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

testdata/p4_16_errors_outputs/issue3221.p4-stderr

@@ -1,4 +1,4 @@
-issue3221.p4(8): [--Werror=type-error] error: cast
+issue3221.p4(8): [--Werror=type-error] error: (t1)a
   return (t1)a;
          ^^^^^
   ---- Actual error:

testdata/p4_16_errors_outputs/issue401.p4-stderr

@@ -1,4 +1,4 @@
-issue401.p4(42): [--Werror=type-error] error: cast
+issue401.p4(42): [--Werror=type-error] error: (bit<1>)ListExpression
     bit<1> b = (bit<1>) { 0 };
                ^^^^^^^^^^^^^^
   ---- Actual error:

testdata/p4_16_errors_outputs/issue473.p4-stderr

@@ -1,3 +1,3 @@
-issue473.p4(70): [--Werror=type-error] error: cast: action argument must be a compile-time constant
+issue473.p4(70): [--Werror=type-error] error: (bit<8>)meta.d: action argument must be a compile-time constant
         default_action = b(meta.c, (bit<8>) meta.d);
                                    ^^^^^^^^^^^^^^^

testdata/p4_16_errors_outputs/key-name.p4-stderr

@@ -1,3 +1,3 @@
-key-name.p4(28): [--Werror=expected] error: +: Complex key expression requires a @name annotation
+key-name.p4(28): [--Werror=expected] error: h.a + h.a: Complex key expression requires a @name annotation
         key = { h.a + h.a : exact; }
                 ^^^^^^^^^

testdata/p4_16_errors_outputs/neg.p4-stderr

@@ -1,6 +1,6 @@
-neg.p4(21): [--Werror=type-error] error: /: Cannot operate on signed values
+neg.p4(21): [--Werror=type-error] error: a / b: Cannot operate on signed values
     c = a / b; // not defined for signed values
         ^^^^^
-neg.p4(22): [--Werror=type-error] error: %: Cannot operate on signed values
+neg.p4(22): [--Werror=type-error] error: a % b: Cannot operate on signed values
     c = a % b;
         ^^^^^

testdata/p4_16_errors_outputs/newtype-err.p4-stderr

@@ -6,7 +6,7 @@ newtype-err.p4(2): Cannot cast implicitly type 'bit<32>' to type 'N32'
   type bit<32> N32;
                ^^^
   ---- Originating from:
-newtype-err.p4(10): Source expression '+' produces a result of type 'bit<32>' which cannot be assigned to a left-value with type 'N32'
+newtype-err.p4(10): Source expression 'b + b' produces a result of type 'bit<32>' which cannot be assigned to a left-value with type 'N32'
           n = b + b;
               ^^^^^
 newtype-err.p4(2)

testdata/p4_16_errors_outputs/psa-meter2.p4-stderr

@@ -1,4 +1,4 @@
-psa-meter2.p4(57): [--Werror=type-error] error: cast
+psa-meter2.p4(57): [--Werror=type-error] error: (bit<16>)meter0.execute
         b.data = (bit<16>)meter0.execute(index, PSA_MeterColor_t.GREEN);
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   ---- Actual error:

testdata/p4_16_errors_outputs/range_e.p4-stderr

@@ -1,3 +1,3 @@
-range_e.p4(20): [--Werror=type-error] error: ..: Cannot operate on values with different types bit<2> and bit<3>
+range_e.p4(20): [--Werror=type-error] error: 2 .. 3: Cannot operate on values with different types bit<2> and bit<3>
             2w2 .. 3w3 : reject; // different widths
             ^^^^^^^^^^

testdata/p4_16_errors_outputs/serenum-type.p4-stderr

@@ -1,7 +1,7 @@
 serenum-type.p4(8): [--Werror=type-error] error: EthT: Illegal type for enum; only bit<> and int<> are allowed
 enum EthT EthTypes {
      ^^^^
-serenum-type.p4(49): [--Werror=type-error] error: cast
+serenum-type.p4(49): [--Werror=type-error] error: (EthTypes)16w0
             h.eth.type = (EthTypes)(bit<16>)0;
                          ^^^^^^^^^^^^^^^^^^^^
   ---- Actual error:

testdata/p4_16_errors_outputs/serenum-typedef.p4-stderr

@@ -1,7 +1,7 @@
 serenum-typedef.p4(8): [--Werror=type-error] error: EthT: Illegal type for enum; only bit<> and int<> are allowed
 enum EthT EthTypes {
      ^^^^
-serenum-typedef.p4(49): [--Werror=type-error] error: cast
+serenum-typedef.p4(49): [--Werror=type-error] error: (EthTypes)16w0
             h.eth.type = (EthTypes)(bit<16>)0;
                          ^^^^^^^^^^^^^^^^^^^^
   ---- Actual error:

testdata/p4_16_errors_outputs/signs.p4-stderr

@@ -7,6 +7,6 @@ signs.p4(18): [--Werror=type-error] error: b
 signs.p4(18): Source expression '8w0' produces a result of type 'bit<8>' which cannot be assigned to a left-value with type 'int<8>'
       int<8> b = 8w0;
                  ^^^
-signs.p4(19): [--Werror=type-error] error: -: Cannot operate on values with different signs
+signs.p4(19): [--Werror=type-error] error: a - b: Cannot operate on values with different signs
     a = a - b;
         ^^^^^

testdata/p4_16_errors_outputs/table-entries-lpm-2.p4-stderr

@@ -13,9 +13,9 @@ table-entries-lpm-2.p4(82): [--Wwarn=mismatch] warning: 8w0x100: value does not
 table-entries-lpm-2.p4(71): [--Wwarn=mismatch] warning: P4Runtime requires that LPM matches have masked-off bits set to 0, updating value 8w0x11 to conform to the P4Runtime specification
             0x11 &&& 0x1F0 : a_with_control_params(12);
             ^^^^
-table-entries-lpm-2.p4(74): [--Werror=invalid] error: &&& invalid mask for LPM key
+table-entries-lpm-2.p4(74): [--Werror=invalid] error: 17 &&& 225 invalid mask for LPM key
             0x11 &&& 0xE1 : a_with_control_params(13);
             ^^^^^^^^^^^^^
-table-entries-lpm-2.p4(78): [--Werror=invalid] error: &&& invalid mask for LPM key
+table-entries-lpm-2.p4(78): [--Werror=invalid] error: 17 &&& 129 invalid mask for LPM key
             0x11 &&& 0x181 : a_with_control_params(14);
             ^^^^^^^^^^^^^^

testdata/p4_16_errors_outputs/table-entries-optional-2-bmv2.p4-stderr

@@ -4,6 +4,6 @@ table-entries-optional-2-bmv2.p4(71): [--Wwarn=mismatch] warning: 8w0x100: value
 table-entries-optional-2-bmv2.p4(73): [--Wwarn=mismatch] warning: 16w0x10000: value does not fit in 16 bits
             (default, 0x10000): a_with_control_params(3);
                       ^^^^^^^
-table-entries-optional-2-bmv2.p4(69): [--Werror=invalid] error: &&& invalid key expression
+table-entries-optional-2-bmv2.p4(69): [--Werror=invalid] error: 170 &&& 240 invalid key expression
             (0xaa &&& 0xf0, 0x1111 &&& 0xffff) : a_with_control_params(1);
              ^^^^^^^^^^^^^

testdata/p4_16_errors_outputs/tuple-newtype.p4-stderr

@@ -1,6 +1,6 @@
 tuple-newtype.p4(5): [--Werror=type-error] error: T: `type' can only be applied to base types
 type tuple<bit> T;
                 ^
-tuple-newtype.p4(12): [--Werror=type-error] error: cast: Illegal cast from tuple<bit<1>> to T
+tuple-newtype.p4(12): [--Werror=type-error] error: (T)ListExpression: Illegal cast from tuple<bit<1>> to T
         T tt2 = (T){1w0};
                 ^^^^^^^^