Allow concatenation of string literals at compile time (#4856)

* Allow type-checking string cocnatenation

Signed-off-by: Vladimir Still <[email protected]>

* Constant-fold string concatenation

Signed-off-by: Vladimir Still <[email protected]>

* Allow @name/@deprecated/@nowarn to contain concats

Signed-off-by: Vladimir Still <[email protected]>

* Prevent BindTypeVariables to descend into unstructured annotation and
fail on missing type in typeMap

Signed-off-by: Vladimir Still <[email protected]>

* Add a test for constant-folding strings

Signed-off-by: Vladimir Still <[email protected]>

* Fix formatting

Signed-off-by: Vladimir Still <[email protected]>

* Add a hint when + on strings is used, add negative ests

Signed-off-by: Vladimir Still <[email protected]>

* Fix formatting

Signed-off-by: Vladimir Still <[email protected]>

* Fix a typo

Signed-off-by: Vladimir Still <[email protected]>

* Update frontends/p4/typeChecking/bindVariables.h

Co-authored-by: Anton Korobeynikov <[email protected]>
Signed-off-by: Vladimír Štill <[email protected]>
Signed-off-by: Vladimir Still <[email protected]>

* Add explicit validation of string annotations

Signed-off-by: Vladimir Still <[email protected]>

* Let invalid concats be resolved by type checker, not contant folder

Signed-off-by: Vladimir Still <[email protected]>

* Fix compilation

Signed-off-by: Vladimir Still <[email protected]>

* Avoid complaining about unsized integer in the string ++ int cas

Signed-off-by: Vladimir Still <[email protected]>

* Update test reference outputs

Signed-off-by: Vladimir Still <[email protected]>

---------

Signed-off-by: Vladimir Still <[email protected]>
Signed-off-by: Vladimír Štill <[email protected]>
Co-authored-by: Anton Korobeynikov <[email protected]>

frontends/common/constantFolding.cpp

@@ -730,13 +730,18 @@ const IR::Node *DoConstantFolding::postorder(IR::Concat *e) {
     auto eright = getConstant(e->right);
     if (eleft == nullptr || eright == nullptr) return e;
 
-    auto left = eleft->to<IR::Constant>();
-    if (left == nullptr) {
-        // Could be a SerEnum
-        return e;
+    const auto *lstr = eleft->to<IR::StringLiteral>();
+    const auto *rstr = eright->to<IR::StringLiteral>();
+
+    const auto *left = eleft->to<IR::Constant>();
+    const auto *right = eright->to<IR::Constant>();
+
+    // handle string concatenations
+    if (lstr && rstr) {
+        return new IR::StringLiteral(e->srcInfo, IR::Type_String::get(), lstr->value + rstr->value);
     }
-    auto right = eright->to<IR::Constant>();
-    if (right == nullptr) {
+
+    if (left == nullptr || right == nullptr) {
         // Could be a SerEnum
         return e;
     }

frontends/p4/frontend.cpp

@@ -74,6 +74,7 @@ limitations under the License.
 #include "uselessCasts.h"
 #include "validateMatchAnnotations.h"
 #include "validateParsedProgram.h"
+#include "validateStringAnnotations.h"
 #include "validateValueSets.h"
 
 namespace P4 {
@@ -174,6 +175,8 @@ const IR::P4Program *FrontEnd::run(const CompilerOptions &options, const IR::P4P
         // First pass of constant folding, before types are known --
         // may be needed to compute types.
         new ConstantFolding(constantFoldingPolicy),
+        // Validate @name/@deprecated/@noWarn. Should run after constant folding.
+        new ValidateStringAnnotations(),
         // Desugars direct parser and control applications
         // into instantiations followed by application
         new InstantiateDirectCalls(),

frontends/p4/parseAnnotations.cpp

@@ -32,10 +32,11 @@ ParseAnnotations::HandlerMap ParseAnnotations::standardHandlers() {
         PARSE_EMPTY("unroll"_cs),
         PARSE_EMPTY("nounroll"_cs),
 
-        // string literal argument.
-        PARSE(IR::Annotation::nameAnnotation, StringLiteral),
-        PARSE(IR::Annotation::deprecatedAnnotation, StringLiteral),
-        PARSE(IR::Annotation::noWarnAnnotation, StringLiteral),
+        // String arguments. These are allowed to contain concatenation which will be
+        // constant-folded, so just parse them as expressions.
+        PARSE(IR::Annotation::nameAnnotation, Expression),
+        PARSE(IR::Annotation::deprecatedAnnotation, Expression),
+        PARSE(IR::Annotation::noWarnAnnotation, Expression),
 
         // @length has an expression argument.
         PARSE(IR::Annotation::lengthAnnotation, Expression),

frontends/p4/typeChecking/bindVariables.cpp

@@ -135,6 +135,13 @@ const IR::Node *DoBindTypeVariables::postorder(IR::ConstructorCallExpression *ex
     return expression;
 }
 
+const IR::Node *DoBindTypeVariables::preorder(IR::Annotation *annotation) {
+    if (!annotation->structured) {
+        prune();
+    }
+    return annotation;
+}
+
 const IR::Node *DoBindTypeVariables::insertTypes(const IR::Node *node) {
     CHECK_NULL(node);
     CHECK_NULL(newTypes);

frontends/p4/typeChecking/bindVariables.h

@@ -20,6 +20,11 @@ class DoBindTypeVariables : public Transform {
         setName("DoBindTypeVariables");
         newTypes = new IR::IndexedVector<IR::Node>();
     }
+
+    /// Don't descend to unstructured annotations, they are not type checked, so we can't process
+    /// them here either.
+    const IR::Node *preorder(IR::Annotation *annotation) override;
+
     const IR::Node *postorder(IR::Expression *expression) override;
     const IR::Node *postorder(IR::Declaration_Instance *decl) override;
     const IR::Node *postorder(IR::MethodCallExpression *expression) override;

frontends/p4/typeChecking/typeCheckExpr.cpp

@@ -288,53 +288,65 @@ const IR::Node *TypeInference::postorder(IR::Concat *expression) {
     auto rtype = getType(expression->right);
     if (ltype == nullptr || rtype == nullptr) return expression;
 
-    if (ltype->is<IR::Type_InfInt>()) {
-        typeError("Please specify a width for the operand %1% of a concatenation",
-                  expression->left);
-        return expression;
-    }
-    if (rtype->is<IR::Type_InfInt>()) {
-        typeError("Please specify a width for the operand %1% of a concatenation",
-                  expression->right);
-        return expression;
-    }
+    const IR::Type *resultType = nullptr;
+    if (ltype->is<IR::Type_String>() && rtype->is<IR::Type_String>()) {
+        // For strings the output type is just string.
+        resultType = ltype;
+    } else {
+        // All the integer concatenations, including with serializable enums.
+        bool castLeft = false;
+        bool castRight = false;
+        if (auto se = ltype->to<IR::Type_SerEnum>()) {
+            ltype = getTypeType(se->type);
+            castLeft = true;
+        }
+        if (auto se = rtype->to<IR::Type_SerEnum>()) {
+            rtype = getTypeType(se->type);
+            castRight = true;
+        }
+        if (ltype == nullptr || rtype == nullptr) {
+            // getTypeType should have already taken care of the error message
+            return expression;
+        }
+        if (!ltype->is<IR::Type_Bits>() || !rtype->is<IR::Type_Bits>()) {
+            typeError("%1%: Concatenation not defined on %2% and %3%", expression,
+                      ltype->toString(), rtype->toString());
+
+            // Provide further clarification in the common case of attempting to concat
+            // arbitrary-width int.
+            if (!ltype->is<IR::Type_String>() && !rtype->is<IR::Type_String>()) {
+                if (ltype->is<IR::Type_InfInt>()) {
+                    typeError("Please specify a width for the operand %1% of a concatenation",
+                              expression->left);
+                    return expression;
+                }
+                if (rtype->is<IR::Type_InfInt>()) {
+                    typeError("Please specify a width for the operand %1% of a concatenation",
+                              expression->right);
+                    return expression;
+                }
+            }
 
-    bool castLeft = false;
-    bool castRight = false;
-    if (auto se = ltype->to<IR::Type_SerEnum>()) {
-        ltype = getTypeType(se->type);
-        castLeft = true;
-    }
-    if (auto se = rtype->to<IR::Type_SerEnum>()) {
-        rtype = getTypeType(se->type);
-        castRight = true;
-    }
-    if (ltype == nullptr || rtype == nullptr) {
-        // getTypeType should have already taken care of the error message
-        return expression;
-    }
-    if (!ltype->is<IR::Type_Bits>() || !rtype->is<IR::Type_Bits>()) {
-        typeError("%1%: Concatenation not defined on %2% and %3%", expression, ltype->toString(),
-                  rtype->toString());
-        return expression;
-    }
-    auto bl = ltype->to<IR::Type_Bits>();
-    auto br = rtype->to<IR::Type_Bits>();
-    const IR::Type *resultType = IR::Type_Bits::get(bl->size + br->size, bl->isSigned);
+            return expression;
+        }
+        auto bl = ltype->to<IR::Type_Bits>();
+        auto br = rtype->to<IR::Type_Bits>();
+        resultType = IR::Type_Bits::get(bl->size + br->size, bl->isSigned);
 
-    if (castLeft) {
-        auto e = expression->clone();
-        e->left = new IR::Cast(e->left->srcInfo, bl, e->left);
-        if (isCompileTimeConstant(expression->left)) setCompileTimeConstant(e->left);
-        setType(e->left, ltype);
-        expression = e;
-    }
-    if (castRight) {
-        auto e = expression->clone();
-        e->right = new IR::Cast(e->right->srcInfo, br, e->right);
-        if (isCompileTimeConstant(expression->right)) setCompileTimeConstant(e->right);
-        setType(e->right, rtype);
-        expression = e;
+        if (castLeft) {
+            auto e = expression->clone();
+            e->left = new IR::Cast(e->left->srcInfo, bl, e->left);
+            if (isCompileTimeConstant(expression->left)) setCompileTimeConstant(e->left);
+            setType(e->left, ltype);
+            expression = e;
+        }
+        if (castRight) {
+            auto e = expression->clone();
+            e->right = new IR::Cast(e->right->srcInfo, br, e->right);
+            if (isCompileTimeConstant(expression->right)) setCompileTimeConstant(e->right);
+            setType(e->right, rtype);
+            expression = e;
+        }
     }
 
     resultType = canonicalize(resultType);
@@ -760,6 +772,14 @@ const IR::Node *TypeInference::binaryArith(const IR::Operation_Binary *expressio
     auto ltype = getType(expression->left);
     auto rtype = getType(expression->right);
     if (ltype == nullptr || rtype == nullptr) return expression;
+
+    if (expression->is<IR::Add>() && ltype->is<IR::Type_String>() && rtype->is<IR::Type_String>()) {
+        typeError(
+            "%1%: cannot be applied to expression '%2%' with type '%3%', did you mean to use '++'?",
+            expression->getStringOp(), expression->right, rtype->toString());
+        return expression;
+    }
+
     bool castLeft = false;
     bool castRight = false;
 

frontends/p4/validateStringAnnotations.h

@@ -0,0 +1,54 @@
+/*
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+#ifndef FRONTENDS_P4_VALIDATESTRINGANNOTATIONS_H_
+#define FRONTENDS_P4_VALIDATESTRINGANNOTATIONS_H_
+
+#include "frontends/p4/typeMap.h"
+#include "ir/ir.h"
+#include "lib/error.h"
+
+/// @file
+/// @brief Check validity of built-in string annotations.
+
+namespace P4 {
+
+/// Checks that the build-in string annotations (\@name, \@deprecated, \@noWarn) have string
+/// arguments.
+class ValidateStringAnnotations final : public Inspector {
+    TypeMap *typeMap;
+
+ public:
+    explicit ValidateStringAnnotations() {}
+
+    void postorder(const IR::Annotation *annotation) override {
+        const auto name = annotation->name;
+        if (name != IR::Annotation::nameAnnotation &&
+            name != IR::Annotation::deprecatedAnnotation &&
+            name != IR::Annotation::noWarnAnnotation) {
+            return;
+        }
+        if (annotation->expr.size() != 1)
+            error(ErrorType::ERR_INVALID, "%1%: annotation must have exactly 1 argument",
+                  annotation);
+        auto e0 = annotation->expr.at(0);
+        if (!e0->is<IR::StringLiteral>())
+            error(ErrorType::ERR_TYPE_ERROR, "%1%: @%2% annotation's value must be a string", e0,
+                  annotation->name.originalName);
+    }
+};
+
+}  // namespace P4
+
+#endif /* FRONTENDS_P4_VALIDATESTRINGANNOTATIONS_H_  */

testdata/p4_16_errors/spec-issue1297-string-cat-err-0.p4

@@ -0,0 +1,5 @@
+extern void log(string msg);
+
+void fn() {
+    log("a" + "b");
+}

testdata/p4_16_errors/spec-issue1297-string-cat-err-1.p4

@@ -0,0 +1,6 @@
+extern void log(string msg);
+
+void fn() {
+    log("a" ++ 4);
+    log(1w2 ++ "a");
+}

testdata/p4_16_errors/spec-issue1297-string-cat-err-2-anno.p4

@@ -0,0 +1,4 @@
+// should not compile -- not a string
+@name(4)
+void fn() {
+}

testdata/p4_16_errors/spec-issue1297-string-cat-err-3-anno.p4

@@ -0,0 +1,4 @@
+// should not compile -- not valid string op
+@deprecated("a" | "b")
+void fn() {
+}

testdata/p4_16_errors_outputs/spec-issue1297-string-cat-err-0.p4

@@ -0,0 +1,4 @@
+extern void log(string msg);
+void fn() {
+    log("a" + "b");
+}

testdata/p4_16_errors_outputs/spec-issue1297-string-cat-err-0.p4-stderr

@@ -0,0 +1,3 @@
+spec-issue1297-string-cat-err-0.p4(4): [--Werror=type-error] error: +: cannot be applied to expression '"b"' with type 'string', did you mean to use '++'?
+    log("a" + "b");
+                ^

testdata/p4_16_errors_outputs/spec-issue1297-string-cat-err-1.p4

@@ -0,0 +1,5 @@
+extern void log(string msg);
+void fn() {
+    log("a" ++ 4);
+    log(1w0 ++ "a");
+}

testdata/p4_16_errors_outputs/spec-issue1297-string-cat-err-1.p4-stderr

@@ -0,0 +1,9 @@
+spec-issue1297-string-cat-err-1.p4(5): [--Wwarn=mismatch] warning: 1w2: value does not fit in 1 bits
+    log(1w2
+        ^^^
+spec-issue1297-string-cat-err-1.p4(4): [--Werror=type-error] error: "a" ++ 4: Concatenation not defined on string and int
+    log("a" ++ 4);
+          ^^^^^^
+spec-issue1297-string-cat-err-1.p4(5): [--Werror=type-error] error: 0 ++ "a": Concatenation not defined on bit<1> and string
+    log(1w2 ++ "a");
+        ^^^^^^^^^^

testdata/p4_16_errors_outputs/spec-issue1297-string-cat-err-2-anno.p4

@@ -0,0 +1,2 @@
+@name(4) void fn() {
+}

testdata/p4_16_errors_outputs/spec-issue1297-string-cat-err-2-anno.p4-stderr

@@ -0,0 +1,3 @@
+spec-issue1297-string-cat-err-2-anno.p4(2): [--Werror=type-error] error: 4: @name annotation's value must be a string
+@name(4)
+      ^

testdata/p4_16_errors_outputs/spec-issue1297-string-cat-err-3-anno.p4

@@ -0,0 +1,2 @@
+@deprecated("a" | "b") void fn() {
+}

testdata/p4_16_errors_outputs/spec-issue1297-string-cat-err-3-anno.p4-stderr

@@ -0,0 +1,3 @@
+spec-issue1297-string-cat-err-3-anno.p4(2): [--Werror=type-error] error: "a" | "b": @deprecated annotation's value must be a string
+@deprecated("a" | "b")
+              ^^^^^^^

testdata/p4_16_samples/spec-issue1297-string-cat.p4

@@ -0,0 +1,51 @@
+#include <core.p4>
+
+#define DEPR_NAME depr
+#define _STR(x) #x
+#define STR(x) _STR(x)
+#define AAA "_x_"
+#define BBB "_y_"
+
+// test constant-folding concat in @deprecated
+@deprecated("function " ++ STR(DEPR_NAME) ++ " is deprecated")
+void DEPR_NAME (int<4> x) { }
+
+extern void log(string msg);
+
+control c() {
+
+    // constant-folded (unstructured, but with a special meaning defined in P4C)
+    @name("foo" ++ BBB ++ "bar" ++ AAA)
+    action foo() { }
+
+    // constant-folded
+    @DummyVendor_Structured[a="X_" ++ "Y", b="Y" ++ "_" ++ "z"]
+    action bar() { }
+
+    // constant-folded
+    @DummyVendor_Structured2["X_" ++ "Y", "Y" ++ "_" ++ "z"]
+    action baz() { }
+
+    // NOT constant-folded (unstructured with no special meaning)
+    @DummyVendor_Unstructured(a="X_" ++ "Y", b="Y" ++ "_" ++ "z")
+    action tmp() { }
+
+    @name("table" ++ "_" ++ "a")
+    table a {
+        actions = { foo; bar; baz; tmp; }
+    }
+
+    apply {
+        log("simple msg");
+        // constant-folded
+        log("concat" ++ " " ++ "msg "
+            ++ "multiple lines");
+        a.apply();
+        depr(4);
+     }
+}
+
+control proto();
+package top(proto p);
+
+top(c()) main;