Fix for issue #1331 (#1384)

* Fix for issue #1331

Author: Mihai Budiu

frontends/p4/externInstance.cpp

@@ -83,15 +83,13 @@ ExternInstance::resolve(const IR::ConstructorCallExpression* constructorCallExpr
     CHECK_NULL(typeMap);
 
     auto constructorCall =
-      P4::ConstructorCall::resolve(constructorCallExpr, refMap, typeMap);
+            P4::ConstructorCall::resolve(constructorCallExpr, refMap, typeMap);
     if (!constructorCall->is<P4::ExternConstructorCall>()) return boost::none;
 
-    ConstructorCallDescription ccDesc(constructorCallExpr, refMap, typeMap);
-
     auto type = constructorCall->to<P4::ExternConstructorCall>()->type;
     return ExternInstance{name, constructorCallExpr, type,
                           constructorCallExpr->arguments,
-                          ccDesc.substitution,
+                          constructorCall->substitution,
                           constructorCallExpr->to<IR::IAnnotated>()};
 }
 

frontends/p4/inlining.cpp

@@ -392,14 +392,17 @@ void DiscoverInlining::visit_all(const IR::Block* block) {
         if (it.second->is<IR::Block>()) {
             visit(it.second->getNode());
         }
+        if (::errorCount() > 0)
+            return;
     }
 }
 
 bool DiscoverInlining::preorder(const IR::ControlBlock* block) {
     LOG4("Visiting " << block);
     if (getContext()->node->is<IR::ParserBlock>()) {
-        ::error("%1%: invocation of a control from a parser",
+        ::error("%1%: instantiation of control in parser",
                 block->node);
+        return false;
     } else if (getContext()->node->is<IR::ControlBlock>() && allowControls) {
         auto parent = getContext()->node->to<IR::ControlBlock>();
         LOG3("Will inline " << dbp(block) << "@" << dbp(block->node) << " into " << dbp(parent));
@@ -409,15 +412,18 @@ bool DiscoverInlining::preorder(const IR::ControlBlock* block) {
     }
 
     visit_all(block);
+    if (::errorCount() > 0)
+        return false;
     visit(block->container->body);
     return false;
 }
 
 bool DiscoverInlining::preorder(const IR::ParserBlock* block) {
     LOG4("Visiting " << block);
     if (getContext()->node->is<IR::ControlBlock>()) {
-        ::error("%1%: invocation of a parser from a control",
+        ::error("%1%: instantiation of parser in control",
                 block->node);
+        return false;
     } else if (getContext()->node->is<IR::ParserBlock>()) {
         auto parent = getContext()->node->to<IR::ParserBlock>();
         LOG3("Will inline " << block << "@" << block->node << " into " << parent);
@@ -426,6 +432,8 @@ bool DiscoverInlining::preorder(const IR::ParserBlock* block) {
         inlineList->addInstantiation(parent->container, callee, instance);
     }
     visit_all(block);
+    if (::errorCount() > 0)
+        return false;
     visit(block->container->states, "states");
     return false;
 }

frontends/p4/methodInstance.cpp

@@ -147,21 +147,21 @@ ConstructorCall::resolve(const IR::ConstructorCallExpression* cce,
         auto ext = decl->to<IR::Type_Extern>();
         BUG_CHECK(ext, "%1%: expected an extern type", dbp(decl));
         auto constr = ext->lookupConstructor(cce->arguments->size());
-        result = new ExternConstructorCall(ext->to<IR::Type_Extern>(), constr);
+        result = new ExternConstructorCall(cce, ext->to<IR::Type_Extern>(), constr);
         BUG_CHECK(constr, "%1%: constructor not found", ext);
         constructorParameters = constr->type->parameters;
     } else if (ct->is<IR::IContainer>()) {
         auto decl = refMap->getDeclaration(type->path, true);
         auto cont = decl->to<IR::IContainer>();
         BUG_CHECK(cont, "%1%: expected a container", dbp(decl));
-        result = new ContainerConstructorCall(cont);
+        result = new ContainerConstructorCall(cce, cont);
         constructorParameters = cont->getConstructorParameters();
     } else {
         BUG("Unexpected constructor call %1%; type is %2%", dbp(cce), dbp(ct));
     }
-    result->cce = cce;
     result->typeArguments = typeArguments;
     result->constructorParameters = constructorParameters;
+    result->substitution.populate(result->constructorParameters, cce->arguments);
     return result;
 }
 

frontends/p4/methodInstance.h

@@ -213,7 +213,11 @@ class MethodCallDescription {
 class ConstructorCall : public InstanceBase {
  protected:
     virtual ~ConstructorCall() {}
+    explicit ConstructorCall(const IR::ConstructorCallExpression* cce): cce(cce)
+    { CHECK_NULL(cce); }
  public:
+    /// For each callee parameter the corresponding argument
+    ParameterSubstitution substitution;
     const IR::ConstructorCallExpression* cce;
     const IR::Vector<IR::Type>*          typeArguments;
     const IR::ParameterList*             constructorParameters;
@@ -224,9 +228,10 @@ class ConstructorCall : public InstanceBase {
 
 /** Represents a constructor call that allocates an Extern object */
 class ExternConstructorCall : public ConstructorCall {
-    explicit ExternConstructorCall(const IR::Type_Extern* type,
+    explicit ExternConstructorCall(const IR::ConstructorCallExpression* cce,
+                                   const IR::Type_Extern* type,
                                    const IR::Method* constructor) :
-            type(type), constructor(constructor)
+            ConstructorCall(cce), type(type), constructor(constructor)
     { CHECK_NULL(type); CHECK_NULL(constructor); }
     friend class ConstructorCall;
  public:
@@ -237,25 +242,14 @@ class ExternConstructorCall : public ConstructorCall {
 /** Represents a constructor call that allocates an object that implements IContainer.
     These can be package, control or parser */
 class ContainerConstructorCall : public ConstructorCall {
-    explicit ContainerConstructorCall(const IR::IContainer* cont) :
-            container(cont) { CHECK_NULL(cont); }
+    explicit ContainerConstructorCall(const IR::ConstructorCallExpression* cce,
+                                      const IR::IContainer* cont) :
+            ConstructorCall(cce), container(cont) { CHECK_NULL(cont); }
     friend class ConstructorCall;
  public:
     const IR::IContainer* container;  // actual container in program IR
 };
 
-class ConstructorCallDescription {
- public:
-    ConstructorCall       *call;
-    /// For each callee parameter the corresponding argument
-    ParameterSubstitution substitution;
-    ConstructorCallDescription(const IR::ConstructorCallExpression* cce,
-                               ReferenceMap* refMap, TypeMap* typeMap) {
-        auto cc = ConstructorCall::resolve(cce, refMap, typeMap);
-        substitution.populate(cc->constructorParameters, cce->arguments);
-    }
-};
-
 /////////////////////////////////////////////
 
 /// Used to resolve a Declaration_Instance

frontends/p4/parameterSubstitution.h

@@ -60,6 +60,13 @@ class ParameterSubstitution : public IHasDbPrint {
         return true;
     }
 
+    const IR::Parameter* findParameter(const IR::Argument* argument) const {
+        for (auto p : *getParametersInOrder())
+            if (lookup(p) == argument)
+                return p;
+        return nullptr;
+    }
+
     bool empty() const
     { return parameterValues.empty(); }
 

frontends/p4/specialize.cpp

@@ -61,15 +61,15 @@ const IR::Type_Declaration* SpecializationInfo::synthesize(ReferenceMap* refMap)
 }
 
 const IR::Argument* SpecializationMap::convertArgument(
-    const IR::Argument* arg, SpecializationInfo* spec) {
+    const IR::Argument* arg, SpecializationInfo* spec, const IR::Parameter* param) {
     if (arg->expression->is<IR::ConstructorCallExpression>()) {
         auto cce = arg->expression->to<IR::ConstructorCallExpression>();
-        cstring nName = refMap->newName("inst");
+        cstring nName = refMap->newName(param->name);
+        IR::ID id(param->srcInfo, nName, param->name);
         auto decl = new IR::Declaration_Instance(
-            IR::ID(nName, nullptr),
-            cce->constructedType, cce->arguments);
+            param->srcInfo, id, cce->constructedType, cce->arguments);
         spec->declarations->push_back(decl);
-        auto path = new IR::PathExpression(IR::ID(nName, nullptr));
+        auto path = new IR::PathExpression(param->srcInfo, new IR::Path(param->srcInfo, id));
         return new IR::Argument(arg->srcInfo, arg->name, path);
     } else {
         return arg;
@@ -91,7 +91,9 @@ void SpecializationMap::addSpecialization(
     CHECK_NULL(ccc);
     spec->constructorArguments = new IR::Vector<IR::Argument>();
     for (auto ca : *invocation->arguments) {
-        auto arg = convertArgument(ca, spec);
+        auto param = cc->substitution.findParameter(ca);
+        CHECK_NULL(param);
+        auto arg = convertArgument(ca, spec, param);
         spec->constructorArguments->push_back(arg);
     }
     spec->typeArguments = ccc->typeArguments;
@@ -118,10 +120,14 @@ void SpecializationMap::addSpecialization(
         type = invocation->type->to<IR::Type_Name>();
         typeArgs = new IR::Vector<IR::Type>();
     }
+    Instantiation* inst = Instantiation::resolve(invocation, refMap, typeMap);
+
     spec->typeArguments = typeArgs;
     CHECK_NULL(type);
     for (auto ca : *invocation->arguments) {
-        auto arg = convertArgument(ca, spec);
+        auto param = inst->substitution.findParameter(ca);
+        CHECK_NULL(param);
+        auto arg = convertArgument(ca, spec, param);
         spec->constructorArguments->push_back(arg);
     }
     specializations.emplace(invocation, spec);
@@ -287,7 +293,7 @@ const IR::Node* Specialize::postorder(IR::Declaration_Instance* decl) {
     if (!name.isNullOrEmpty()) {
         auto typeRef = new IR::Type_Name(IR::ID(name, nullptr));
         replacement = new IR::Declaration_Instance(
-            decl->name, decl->annotations, typeRef,
+            decl->srcInfo, decl->name, decl->annotations, typeRef,
             new IR::Vector<IR::Argument>(), decl->initializer);
         LOG2("Replaced " << decl << " with " << replacement);
     }

frontends/p4/specialize.h

@@ -55,7 +55,8 @@ struct SpecializationInfo {
 class SpecializationMap {
     /// Maps invocation to specialization info.
     ordered_map<const IR::Node*, SpecializationInfo*> specializations;
-    const IR::Argument* convertArgument(const IR::Argument* arg, SpecializationInfo* info);
+    const IR::Argument* convertArgument(
+        const IR::Argument* arg, SpecializationInfo* info, const IR::Parameter* param);
 
  public:
     TypeMap*      typeMap;

ir/base.cpp

@@ -42,7 +42,7 @@ cstring IDeclaration::externalName(cstring replace /* = cstring() */) const {
         return Annotation::getName(anno);
     if (replace)
         return replace;
-    return getName().name;
+    return getName().toString();
 }
 
 cstring IDeclaration::controlPlaneName(cstring replace /* = cstring() */) const {

midend/orderArguments.cpp

@@ -45,8 +45,8 @@ const IR::Node* DoOrderArguments::postorder(IR::MethodCallExpression* expression
 }
 
 const IR::Node* DoOrderArguments::postorder(IR::ConstructorCallExpression* expression) {
-    ConstructorCallDescription ccd(expression, refMap, typeMap);
-    expression->arguments = reorder(ccd.substitution);
+    ConstructorCall* ccd = ConstructorCall::resolve(expression, refMap, typeMap);
+    expression->arguments = reorder(ccd->substitution);
     return expression;
 }
 

testdata/p4_16_errors/issue1331.p4

@@ -0,0 +1,31 @@
+#include <core.p4>
+
+header ethernet {
+  bit<112> data;
+}
+
+parser P(packet_in pkt);
+control C(packet_in pkt, inout ethernet ether);
+package S(C c);
+
+parser MyP(packet_in pkt) {
+  state start {
+    transition accept;
+  }
+}
+
+control MyC(packet_in pkt, inout ethernet ether)(P p) {
+    action a() {
+    }
+    table t {
+        key = {}
+        actions = { a; }
+    }
+    apply {
+        pkt.extract(ether);
+        p.apply(pkt);
+        t.apply();
+    }
+}
+
+S(MyC(MyP())) main;