Fixed bug in name resolution; better shadowing tests (#349)

frontends/common/resolveReferences/resolveReferences.cpp

@@ -180,6 +180,7 @@ ResolveReferences::ResolveReferences(ReferenceMap* refMap,
 void ResolveReferences::addToContext(const IR::INamespace* ns) {
     LOG1("Adding to context " << dbp(ns));
     BUG_CHECK(context != nullptr, "No resolution context; did not start at P4Program?");
+    checkShadowing(ns);
     context->push(ns);
 }
 
@@ -213,24 +214,20 @@ void ResolveReferences::resolvePath(const IR::Path* path, bool isType) const {
     refMap->setDeclaration(path, decl);
 }
 
-void ResolveReferences::checkShadowing(const IR::INamespace*ns) const {
+void ResolveReferences::checkShadowing(const IR::INamespace* ns) const {
     if (!checkShadow) return;
-
-    auto e = ns->getDeclarations();
-    while (e->moveNext()) {
-        const IR::IDeclaration* decl = e->getCurrent();
+    for (auto decl : *ns->getDeclarations()) {
         const IR::Node* node = decl->getNode();
         auto prev = context->resolve(decl->getName(), ResolutionType::Any, !anyOrder);
         if (prev->empty()) continue;
+
         for (auto p : *prev) {
             const IR::Node* pnode = p->getNode();
             if (pnode == node) continue;
-            if (pnode->is<IR::Type_Method>() && node->is<IR::Type_Method>()) {
-                auto md = node->to<IR::Type_Method>();
-                auto mp = pnode->to<IR::Type_Method>();
-                if (md->parameters->size() != mp->parameters->size())
-                    continue;
-            }
+            if (pnode->is<IR::Type_Method>() && node->is<IR::Type_Method>())
+                // Methods can overload each other if they have a different number of arguments
+                continue;
+
             ::warning("%1% shadows %2%", decl->getName(), p->getName());
         }
     }
@@ -287,36 +284,34 @@ bool ResolveReferences::preorder(const IR::Type_Name* type) {
 
 bool ResolveReferences::preorder(const IR::P4Control *c) {
     refMap->usedName(c->name.name);
-    addToContext(c);
     addToContext(c->type->typeParameters);
     addToContext(c->type->applyParams);
     addToContext(c->constructorParams);
+    addToContext(c);  // add the locals
     return true;
 }
 
 void ResolveReferences::postorder(const IR::P4Control *c) {
+    removeFromContext(c);
     removeFromContext(c->constructorParams);
     removeFromContext(c->type->applyParams);
     removeFromContext(c->type->typeParameters);
-    removeFromContext(c);
-    checkShadowing(c);
 }
 
-bool ResolveReferences::preorder(const IR::P4Parser *c) {
-    refMap->usedName(c->name.name);
-    addToContext(c);
-    addToContext(c->type->typeParameters);
-    addToContext(c->type->applyParams);
-    addToContext(c->constructorParams);
+bool ResolveReferences::preorder(const IR::P4Parser *p) {
+    refMap->usedName(p->name.name);
+    addToContext(p->type->typeParameters);
+    addToContext(p->type->applyParams);
+    addToContext(p->constructorParams);
+    addToContext(p);
     return true;
 }
 
-void ResolveReferences::postorder(const IR::P4Parser *c) {
-    removeFromContext(c->constructorParams);
-    removeFromContext(c->type->applyParams);
-    removeFromContext(c->type->typeParameters);
-    removeFromContext(c);
-    checkShadowing(c);
+void ResolveReferences::postorder(const IR::P4Parser *p) {
+    removeFromContext(p);
+    removeFromContext(p->constructorParams);
+    removeFromContext(p->type->applyParams);
+    removeFromContext(p->type->typeParameters);
 }
 
 bool ResolveReferences::preorder(const IR::Function* function) {
@@ -350,15 +345,14 @@ void ResolveReferences::postorder(const IR::TableProperties *p) {
 
 bool ResolveReferences::preorder(const IR::P4Action *c) {
     refMap->usedName(c->name.name);
-    addToContext(c);
     addToContext(c->parameters);
+    addToContext(c);
     return true;
 }
 
 void ResolveReferences::postorder(const IR::P4Action *c) {
-    removeFromContext(c->parameters);
     removeFromContext(c);
-    checkShadowing(c);
+    removeFromContext(c->parameters);
 }
 
 bool ResolveReferences::preorder(const IR::Type_Method *t) {
@@ -397,7 +391,6 @@ bool ResolveReferences::preorder(const IR::ParserState *s) {
 void ResolveReferences::postorder(const IR::ParserState *s) {
     removeFromContext(s);
     resolveForward.pop_back();
-    checkShadowing(s);
 }
 
 bool ResolveReferences::preorder(const IR::Declaration_MatchKind *d)
@@ -425,7 +418,7 @@ bool ResolveReferences::preorder(const IR::BlockStatement *b)
 { addToContext(b); return true; }
 
 void ResolveReferences::postorder(const IR::BlockStatement *b)
-{ removeFromContext(b); checkShadowing(b); }
+{ removeFromContext(b); }
 
 bool ResolveReferences::preorder(const IR::Declaration_Instance *decl) {
     refMap->usedName(decl->name.name);

frontends/p4/validateParsedProgram.cpp

@@ -79,6 +79,30 @@ void ValidateParsedProgram::postorder(const IR::P4Table* t) {
     }
 }
 
+void ValidateParsedProgram::distinctParameters(
+    const IR::TypeParameters* typeParams,
+    const IR::ParameterList* apply,
+    const IR::ParameterList* constr) {
+    std::map<cstring, const IR::Node*> found;
+
+    for (auto p : *typeParams->parameters)
+        found.emplace(p->getName(), p);
+    for (auto p : *apply->parameters) {
+        auto it = found.find(p->getName());
+        if (it != found.end())
+            ::error("Duplicated parameter name: %1% and %2%",
+                    it->second, p);
+        else
+            found.emplace(p->getName(), p);
+    }
+    for (auto p : *constr->parameters) {
+        auto it = found.find(p->getName());
+        if (it != found.end())
+            ::error("Duplicated parameter name: %1% and %2%",
+                    it->second, p);
+    }
+}
+
 void ValidateParsedProgram::postorder(const IR::ConstructorCallExpression* expression) {
     auto inAction = findContext<IR::P4Action>();
     if (inAction != nullptr)

frontends/p4/validateParsedProgram.h

@@ -41,6 +41,11 @@ namespace P4 {
 class ValidateParsedProgram final : public Inspector {
     bool isv1;
     void container(const IR::IContainer* type);
+    // Make sure that type, apply and constructor parameters are distinct
+    void distinctParameters(
+        const IR::TypeParameters* typeParams,
+        const IR::ParameterList* apply,
+        const IR::ParameterList* constr);
 
  public:
     explicit ValidateParsedProgram(bool isv1) : isv1(isv1)
@@ -61,10 +66,16 @@ class ValidateParsedProgram final : public Inspector {
     void postorder(const IR::ExitStatement* statement) override;
     void postorder(const IR::Type_Package* package) override
     { container(package); }
-    void postorder(const IR::P4Control* control) override
-    { container(control); }
-    void postorder(const IR::P4Parser* parser) override
-    { container(parser); }
+    void postorder(const IR::P4Control* control) override {
+        container(control);
+        distinctParameters(control->type->typeParameters,
+                           control->type->applyParams,
+                           control->constructorParams); }
+    void postorder(const IR::P4Parser* parser) override {
+        container(parser);
+        distinctParameters(parser->type->typeParameters,
+                           parser->type->applyParams,
+                           parser->constructorParams); }
 };
 
 }  // namespace P4

testdata/p4_16_errors/dup-param.p4

@@ -0,0 +1,19 @@
+/*
+Copyright 2017 VMware, Inc.
+
+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.
+*/
+
+control c(bit<32> p)(bool p) {
+  apply {}
+}

testdata/p4_16_errors/dup-param1.p4

@@ -0,0 +1,19 @@
+/*
+Copyright 2017 VMware, Inc.
+
+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.
+*/
+
+control MyIngress<p>(inout bit<32> p) {
+  apply {}
+}

testdata/p4_16_errors/dup-param2.p4

@@ -0,0 +1,19 @@
+/*
+Copyright 2017 VMware, Inc.
+
+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.
+*/
+
+control MyIngress<p>(inout bit<32> x)(bit<32> p) {
+  apply {}
+}

testdata/p4_16_errors/dup-param3.p4

@@ -0,0 +1,19 @@
+/*
+Copyright 2017 VMware, Inc.
+
+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.
+*/
+
+control MyIngress<p>(inout bit<32> p)(bit<32> p) {
+  apply {}
+}

testdata/p4_16_errors_outputs/dup-param.p4

@@ -0,0 +1,5 @@
+control c(bit<32> p)(bool p) {
+    apply {
+    }
+}
+

testdata/p4_16_errors_outputs/dup-param.p4-stderr

@@ -0,0 +1,6 @@
+../testdata/p4_16_errors/dup-param.p4(17): error: Duplicated parameter name: p and p
+control c(bit<32> p)(bool p) {
+                  ^
+../testdata/p4_16_errors/dup-param.p4(17)
+control c(bit<32> p)(bool p) {
+                          ^

testdata/p4_16_errors_outputs/dup-param1.p4

@@ -0,0 +1,5 @@
+control MyIngress<p>(inout bit<32> p) {
+    apply {
+    }
+}
+

testdata/p4_16_errors_outputs/dup-param1.p4-stderr

@@ -0,0 +1,6 @@
+../testdata/p4_16_errors/dup-param1.p4(17): error: Duplicated parameter name: p and p
+control MyIngress<p>(inout bit<32> p) {
+                  ^
+../testdata/p4_16_errors/dup-param1.p4(17)
+control MyIngress<p>(inout bit<32> p) {
+                                   ^

testdata/p4_16_errors_outputs/dup-param2.p4

@@ -0,0 +1,5 @@
+control MyIngress<p>(inout bit<32> x)(bit<32> p) {
+    apply {
+    }
+}
+

testdata/p4_16_errors_outputs/dup-param2.p4-stderr

@@ -0,0 +1,6 @@
+../testdata/p4_16_errors/dup-param2.p4(17): error: Duplicated parameter name: p and p
+control MyIngress<p>(inout bit<32> x)(bit<32> p) {
+                  ^
+../testdata/p4_16_errors/dup-param2.p4(17)
+control MyIngress<p>(inout bit<32> x)(bit<32> p) {
+                                              ^

testdata/p4_16_errors_outputs/dup-param3.p4

@@ -0,0 +1,5 @@
+control MyIngress<p>(inout bit<32> p)(bit<32> p) {
+    apply {
+    }
+}
+

testdata/p4_16_errors_outputs/dup-param3.p4-stderr

@@ -0,0 +1,12 @@
+../testdata/p4_16_errors/dup-param3.p4(17): error: Duplicated parameter name: p and p
+control MyIngress<p>(inout bit<32> p)(bit<32> p) {
+                  ^
+../testdata/p4_16_errors/dup-param3.p4(17)
+control MyIngress<p>(inout bit<32> p)(bit<32> p) {
+                                   ^
+../testdata/p4_16_errors/dup-param3.p4(17): error: Duplicated parameter name: p and p
+control MyIngress<p>(inout bit<32> p)(bit<32> p) {
+                  ^
+../testdata/p4_16_errors/dup-param3.p4(17)
+control MyIngress<p>(inout bit<32> p)(bit<32> p) {
+                                              ^

testdata/p4_16_samples/shadow3.p4

@@ -0,0 +1,23 @@
+/*
+Copyright 2017 VMware, Inc.
+
+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.
+*/
+
+header H {}
+
+control MyIngress(inout H p) {
+  bit<8> p = 0;
+  apply {
+  }
+}

testdata/p4_16_samples/simplify.p4

@@ -17,7 +17,6 @@ limitations under the License.
 #include <core.p4>
 
 control c(out bool x) {
-    bit<32> x;
     table t1 {
         key = { x : exact; }
         actions = { NoAction; }

testdata/p4_16_samples_outputs/shadow3-frontend.p4

@@ -0,0 +1,3 @@
+header H {
+}
+

testdata/p4_16_samples_outputs/shadow3.p4

@@ -0,0 +1,9 @@
+header H {
+}
+
+control MyIngress(inout H p) {
+    bit<8> p = 0;
+    apply {
+    }
+}
+

testdata/p4_16_samples_outputs/shadow3.p4-stderr

@@ -0,0 +1,7 @@
+../testdata/p4_16_samples/shadow3.p4(20): warning: p shadows p
+  bit<8> p = 0;
+         ^
+../testdata/p4_16_samples/shadow3.p4(19)
+control MyIngress(inout H p) {
+                          ^
+warning: Program does not contain a `main' module

testdata/p4_16_samples_outputs/simplify.p4

@@ -1,7 +1,6 @@
 #include <core.p4>
 
 control c(out bool x) {
-    bit<32> x;
     table t1() {
         key = {
             x: exact;