From 1161725d6e199f44b38f47e97ca9b75da8a1eee3 Mon Sep 17 00:00:00 2001 From: mbudiu-vmw Date: Mon, 6 Mar 2017 11:43:03 -0800 Subject: [PATCH] Fixed bug in name resolution; better shadowing tests --- .../resolveReferences/resolveReferences.cpp | 55 ++++++++----------- frontends/p4/validateParsedProgram.cpp | 24 ++++++++ frontends/p4/validateParsedProgram.h | 19 +++++-- testdata/p4_16_errors/dup-param.p4 | 19 +++++++ testdata/p4_16_errors/dup-param1.p4 | 19 +++++++ testdata/p4_16_errors/dup-param2.p4 | 19 +++++++ testdata/p4_16_errors/dup-param3.p4 | 19 +++++++ testdata/p4_16_errors_outputs/dup-param.p4 | 5 ++ .../p4_16_errors_outputs/dup-param.p4-stderr | 6 ++ testdata/p4_16_errors_outputs/dup-param1.p4 | 5 ++ .../p4_16_errors_outputs/dup-param1.p4-stderr | 6 ++ testdata/p4_16_errors_outputs/dup-param2.p4 | 5 ++ .../p4_16_errors_outputs/dup-param2.p4-stderr | 6 ++ testdata/p4_16_errors_outputs/dup-param3.p4 | 5 ++ .../p4_16_errors_outputs/dup-param3.p4-stderr | 12 ++++ testdata/p4_16_samples/shadow3.p4 | 23 ++++++++ testdata/p4_16_samples/simplify.p4 | 1 - .../p4_16_samples_outputs/shadow3-frontend.p4 | 3 + testdata/p4_16_samples_outputs/shadow3.p4 | 9 +++ .../p4_16_samples_outputs/shadow3.p4-stderr | 7 +++ testdata/p4_16_samples_outputs/simplify.p4 | 1 - 21 files changed, 231 insertions(+), 37 deletions(-) create mode 100644 testdata/p4_16_errors/dup-param.p4 create mode 100644 testdata/p4_16_errors/dup-param1.p4 create mode 100644 testdata/p4_16_errors/dup-param2.p4 create mode 100644 testdata/p4_16_errors/dup-param3.p4 create mode 100644 testdata/p4_16_errors_outputs/dup-param.p4 create mode 100644 testdata/p4_16_errors_outputs/dup-param.p4-stderr create mode 100644 testdata/p4_16_errors_outputs/dup-param1.p4 create mode 100644 testdata/p4_16_errors_outputs/dup-param1.p4-stderr create mode 100644 testdata/p4_16_errors_outputs/dup-param2.p4 create mode 100644 testdata/p4_16_errors_outputs/dup-param2.p4-stderr create mode 100644 testdata/p4_16_errors_outputs/dup-param3.p4 create mode 100644 testdata/p4_16_errors_outputs/dup-param3.p4-stderr create mode 100644 testdata/p4_16_samples/shadow3.p4 create mode 100644 testdata/p4_16_samples_outputs/shadow3-frontend.p4 create mode 100644 testdata/p4_16_samples_outputs/shadow3.p4 create mode 100644 testdata/p4_16_samples_outputs/shadow3.p4-stderr diff --git a/frontends/common/resolveReferences/resolveReferences.cpp b/frontends/common/resolveReferences/resolveReferences.cpp index f22a81bb67..3fd81e656a 100644 --- a/frontends/common/resolveReferences/resolveReferences.cpp +++ b/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() && node->is()) { - auto md = node->to(); - auto mp = pnode->to(); - if (md->parameters->size() != mp->parameters->size()) - continue; - } + if (pnode->is() && node->is()) + // 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); diff --git a/frontends/p4/validateParsedProgram.cpp b/frontends/p4/validateParsedProgram.cpp index 2adccd6e2c..c2fd34e65b 100644 --- a/frontends/p4/validateParsedProgram.cpp +++ b/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 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(); if (inAction != nullptr) diff --git a/frontends/p4/validateParsedProgram.h b/frontends/p4/validateParsedProgram.h index c2f44aebf9..24dcb6fe50 100644 --- a/frontends/p4/validateParsedProgram.h +++ b/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 diff --git a/testdata/p4_16_errors/dup-param.p4 b/testdata/p4_16_errors/dup-param.p4 new file mode 100644 index 0000000000..f6804f4c3a --- /dev/null +++ b/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 {} +} diff --git a/testdata/p4_16_errors/dup-param1.p4 b/testdata/p4_16_errors/dup-param1.p4 new file mode 100644 index 0000000000..973e7a093e --- /dev/null +++ b/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

(inout bit<32> p) { + apply {} +} diff --git a/testdata/p4_16_errors/dup-param2.p4 b/testdata/p4_16_errors/dup-param2.p4 new file mode 100644 index 0000000000..acbc48f03e --- /dev/null +++ b/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

(inout bit<32> x)(bit<32> p) { + apply {} +} diff --git a/testdata/p4_16_errors/dup-param3.p4 b/testdata/p4_16_errors/dup-param3.p4 new file mode 100644 index 0000000000..43aa439414 --- /dev/null +++ b/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

(inout bit<32> p)(bit<32> p) { + apply {} +} diff --git a/testdata/p4_16_errors_outputs/dup-param.p4 b/testdata/p4_16_errors_outputs/dup-param.p4 new file mode 100644 index 0000000000..710b562932 --- /dev/null +++ b/testdata/p4_16_errors_outputs/dup-param.p4 @@ -0,0 +1,5 @@ +control c(bit<32> p)(bool p) { + apply { + } +} + diff --git a/testdata/p4_16_errors_outputs/dup-param.p4-stderr b/testdata/p4_16_errors_outputs/dup-param.p4-stderr new file mode 100644 index 0000000000..bc00651075 --- /dev/null +++ b/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) { + ^ diff --git a/testdata/p4_16_errors_outputs/dup-param1.p4 b/testdata/p4_16_errors_outputs/dup-param1.p4 new file mode 100644 index 0000000000..0d7467c5e2 --- /dev/null +++ b/testdata/p4_16_errors_outputs/dup-param1.p4 @@ -0,0 +1,5 @@ +control MyIngress

(inout bit<32> p) { + apply { + } +} + diff --git a/testdata/p4_16_errors_outputs/dup-param1.p4-stderr b/testdata/p4_16_errors_outputs/dup-param1.p4-stderr new file mode 100644 index 0000000000..73c896f797 --- /dev/null +++ b/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

(inout bit<32> p) { + ^ +../testdata/p4_16_errors/dup-param1.p4(17) +control MyIngress

(inout bit<32> p) { + ^ diff --git a/testdata/p4_16_errors_outputs/dup-param2.p4 b/testdata/p4_16_errors_outputs/dup-param2.p4 new file mode 100644 index 0000000000..bd90c021fd --- /dev/null +++ b/testdata/p4_16_errors_outputs/dup-param2.p4 @@ -0,0 +1,5 @@ +control MyIngress

(inout bit<32> x)(bit<32> p) { + apply { + } +} + diff --git a/testdata/p4_16_errors_outputs/dup-param2.p4-stderr b/testdata/p4_16_errors_outputs/dup-param2.p4-stderr new file mode 100644 index 0000000000..4be97f634e --- /dev/null +++ b/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

(inout bit<32> x)(bit<32> p) { + ^ +../testdata/p4_16_errors/dup-param2.p4(17) +control MyIngress

(inout bit<32> x)(bit<32> p) { + ^ diff --git a/testdata/p4_16_errors_outputs/dup-param3.p4 b/testdata/p4_16_errors_outputs/dup-param3.p4 new file mode 100644 index 0000000000..f3766b585e --- /dev/null +++ b/testdata/p4_16_errors_outputs/dup-param3.p4 @@ -0,0 +1,5 @@ +control MyIngress

(inout bit<32> p)(bit<32> p) { + apply { + } +} + diff --git a/testdata/p4_16_errors_outputs/dup-param3.p4-stderr b/testdata/p4_16_errors_outputs/dup-param3.p4-stderr new file mode 100644 index 0000000000..76353916bb --- /dev/null +++ b/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

(inout bit<32> p)(bit<32> p) { + ^ +../testdata/p4_16_errors/dup-param3.p4(17) +control MyIngress

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

(inout bit<32> p)(bit<32> p) { + ^ +../testdata/p4_16_errors/dup-param3.p4(17) +control MyIngress

(inout bit<32> p)(bit<32> p) { + ^ diff --git a/testdata/p4_16_samples/shadow3.p4 b/testdata/p4_16_samples/shadow3.p4 new file mode 100644 index 0000000000..f49799dc78 --- /dev/null +++ b/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 { + } +} diff --git a/testdata/p4_16_samples/simplify.p4 b/testdata/p4_16_samples/simplify.p4 index 8fd0908325..8641f8e1d3 100644 --- a/testdata/p4_16_samples/simplify.p4 +++ b/testdata/p4_16_samples/simplify.p4 @@ -17,7 +17,6 @@ limitations under the License. #include control c(out bool x) { - bit<32> x; table t1 { key = { x : exact; } actions = { NoAction; } diff --git a/testdata/p4_16_samples_outputs/shadow3-frontend.p4 b/testdata/p4_16_samples_outputs/shadow3-frontend.p4 new file mode 100644 index 0000000000..5b771d3849 --- /dev/null +++ b/testdata/p4_16_samples_outputs/shadow3-frontend.p4 @@ -0,0 +1,3 @@ +header H { +} + diff --git a/testdata/p4_16_samples_outputs/shadow3.p4 b/testdata/p4_16_samples_outputs/shadow3.p4 new file mode 100644 index 0000000000..8fcc7a8286 --- /dev/null +++ b/testdata/p4_16_samples_outputs/shadow3.p4 @@ -0,0 +1,9 @@ +header H { +} + +control MyIngress(inout H p) { + bit<8> p = 0; + apply { + } +} + diff --git a/testdata/p4_16_samples_outputs/shadow3.p4-stderr b/testdata/p4_16_samples_outputs/shadow3.p4-stderr new file mode 100644 index 0000000000..bc24fea4f5 --- /dev/null +++ b/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 diff --git a/testdata/p4_16_samples_outputs/simplify.p4 b/testdata/p4_16_samples_outputs/simplify.p4 index 5cb7ff3049..a6ab3c5de1 100644 --- a/testdata/p4_16_samples_outputs/simplify.p4 +++ b/testdata/p4_16_samples_outputs/simplify.p4 @@ -1,7 +1,6 @@ #include control c(out bool x) { - bit<32> x; table t1() { key = { x: exact;