Fixed bug in name resolution; better shadowing tests #349
Conversation
| addToContext(c->type->typeParameters); | ||
| addToContext(c->type->applyParams); | ||
| addToContext(c->constructorParams); | ||
| addToContext(c); // add the locals |
There was a problem hiding this comment.
@jnfoster: This change ensures that parameters enter the scope before the locals.
| @@ -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); | |||
There was a problem hiding this comment.
It seems like this will now check every case of shadowing. Are there cases where shadowing should not be a warning? Actions in a control with the same name as some non-action global thing? Cases where a programmer might reasonably expect to reuse a name?
There was a problem hiding this comment.
All I can tell you is that there are no new warnings on the public tests we have.
If this is too strict we can refine it later.
| 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 |
There was a problem hiding this comment.
@mbudiu-vmw You deleted the check that they have different numbers of parameters. Does this code now allow any pair of methods with the same name to shadow each other?
There was a problem hiding this comment.
It will just avoid giving a shadowing warnings for all methods.
There is another check somewhere that two methods may not have the same signature.
There was a problem hiding this comment.
I see.
Do you happen to know where that is?
There was a problem hiding this comment.
One case is in Type_Extern::lookupMethod.
I don't remember if there is another one.
This will signal an error only if the method is invoked.
Also, make sure that type parameters, apply parameters and constructor parameters have distinct names.