From 6e1541704a457613d91d6667d2bb9adff77cae53 Mon Sep 17 00:00:00 2001 From: Anna Rift Date: Fri, 31 May 2024 12:05:28 -0700 Subject: [PATCH 01/19] Debug work in progress Signed-off-by: Anna Rift --- frontend/lib/resolution/Resolver.cpp | 8 ++++++++ frontend/lib/resolution/resolution-queries.cpp | 2 ++ 2 files changed, 10 insertions(+) diff --git a/frontend/lib/resolution/Resolver.cpp b/frontend/lib/resolution/Resolver.cpp index aefceac8c1df..aec2d35a886c 100644 --- a/frontend/lib/resolution/Resolver.cpp +++ b/frontend/lib/resolution/Resolver.cpp @@ -2652,6 +2652,7 @@ void Resolver::issueAmbiguityErrorIfNeeded(const Identifier* ident, ident->name(), prevConfig, traceResult); + gdbShouldBreakHere(); // emit an ambiguity error if this is not resolving a called ident CHPL_REPORT(context, AmbiguousIdentifier, ident, printFirstMention, vec, traceResult); @@ -2950,6 +2951,11 @@ void Resolver::resolveIdentifier(const Identifier* ident, // lookupIdentifier reports any errors that are needed auto parenlessInfo = ParenlessOverloadInfo(); + if (ident->name() == "size" || ident->name() == "this.size") { + gdbShouldBreakHere(); + auto& asdf = receiverScopes.front(); + (void)asdf; + } auto vec = lookupIdentifier(ident, receiverScopes, parenlessInfo); if (parenlessInfo.areCandidatesOnlyParenlessProcs() && !scopeResolveOnly) { @@ -3074,6 +3080,7 @@ bool Resolver::enter(const Identifier* ident) { std::ignore = initResolver->handleUseOfField(ident); return false; } else { + if (ident->name() == "size" || ident->name() == "this.size") gdbShouldBreakHere(); resolveIdentifier(ident, methodReceiverScopes()); return false; } @@ -3753,6 +3760,7 @@ void Resolver::exit(const Dot* dot) { if (initResolver && initResolver->handleUseOfField(dot)) return; ResolvedExpression& receiver = byPostorder.byAst(dot->receiver()); + if (dot->field() == "size") gdbShouldBreakHere(); bool deferToFunctionResolution = false; bool resolvingCalledDot = nearestCalledExpression() == dot; diff --git a/frontend/lib/resolution/resolution-queries.cpp b/frontend/lib/resolution/resolution-queries.cpp index 68dd34e7a8c7..5b63ddf80b01 100644 --- a/frontend/lib/resolution/resolution-queries.cpp +++ b/frontend/lib/resolution/resolution-queries.cpp @@ -4247,6 +4247,8 @@ CallResolutionResult resolveFnCall(Context* context, PoiInfo poiInfo; MostSpecificCandidates mostSpecific; + if (ci.name().str() == "size") gdbShouldBreakHere(); + // Note: currently type constructors are not implemented as methods if (ci.calledType().kind() == QualifiedType::TYPE && ci.isMethodCall() == false) { From cd0e44b146b7c56b67d51068fe12547fb8e74f40 Mon Sep 17 00:00:00 2001 From: Anna Rift Date: Wed, 12 Jun 2024 11:26:30 -0700 Subject: [PATCH 02/19] Draft solution that does function resolution Signed-off-by: Anna Rift --- frontend/lib/resolution/Resolver.cpp | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/frontend/lib/resolution/Resolver.cpp b/frontend/lib/resolution/Resolver.cpp index aec2d35a886c..98f020014017 100644 --- a/frontend/lib/resolution/Resolver.cpp +++ b/frontend/lib/resolution/Resolver.cpp @@ -2715,7 +2715,7 @@ Resolver::lookupIdentifier(const Identifier* ident, } else if (ambiguous && !outParenlessOverloadInfo.areCandidatesOnlyParenlessProcs() && !resolvingCalledIdent) { - issueAmbiguityErrorIfNeeded(ident, scope, receiverScopes, config); + /* issueAmbiguityErrorIfNeeded(ident, scope, receiverScopes, config); */ } } @@ -2953,8 +2953,8 @@ void Resolver::resolveIdentifier(const Identifier* ident, auto parenlessInfo = ParenlessOverloadInfo(); if (ident->name() == "size" || ident->name() == "this.size") { gdbShouldBreakHere(); - auto& asdf = receiverScopes.front(); - (void)asdf; + /* auto& asdf = receiverScopes.front(); */ + /* (void)asdf; */ } auto vec = lookupIdentifier(ident, receiverScopes, parenlessInfo); @@ -2970,6 +2970,27 @@ void Resolver::resolveIdentifier(const Identifier* ident, // can't establish the type. If this is in a function // call, we'll establish it later anyway. result.setType(QualifiedType()); + + QualifiedType receiverType; + ID receiverIdIgnored; + if (getMethodReceiver(&receiverType, &receiverIdIgnored) && receiverType.type()) { + // resolve a.x where a is a record/class and x is a field or parenless + // method + std::vector actuals; + actuals.push_back(CallInfoActual(receiverType, USTR("this"))); + auto ci = CallInfo(/* name */ ident->name(), + /* calledType */ QualifiedType(), + /* isMethodCall */ true, + /* hasQuestionArg */ false, + /* isParenless */ true, actuals); + auto inScope = scopeStack.back(); + auto inScopes = CallScopeInfo::forNormalCall(inScope, poiScope); + auto c = resolveGeneratedCall(context, ident, ci, inScopes); + // save the most specific candidates in the resolution result for the id + if (!handleResolvedCallWithoutError(result, ident, ci, c)) { + return; + } + } } else { // vec.size() == 1 and vec[0].numIds() <= 1 const IdAndFlags& idv = vec[0].firstIdAndFlags(); From 72684de29f61636fd7f257a2abe0014530f680da Mon Sep 17 00:00:00 2001 From: Anna Rift Date: Wed, 12 Jun 2024 12:00:00 -0700 Subject: [PATCH 03/19] Revert "Draft solution that does function resolution" This reverts commit ef708dbc3a44780ca5776ca74493409a3179fbba. Signed-off-by: Anna Rift --- frontend/lib/resolution/Resolver.cpp | 27 +++------------------------ 1 file changed, 3 insertions(+), 24 deletions(-) diff --git a/frontend/lib/resolution/Resolver.cpp b/frontend/lib/resolution/Resolver.cpp index 98f020014017..aec2d35a886c 100644 --- a/frontend/lib/resolution/Resolver.cpp +++ b/frontend/lib/resolution/Resolver.cpp @@ -2715,7 +2715,7 @@ Resolver::lookupIdentifier(const Identifier* ident, } else if (ambiguous && !outParenlessOverloadInfo.areCandidatesOnlyParenlessProcs() && !resolvingCalledIdent) { - /* issueAmbiguityErrorIfNeeded(ident, scope, receiverScopes, config); */ + issueAmbiguityErrorIfNeeded(ident, scope, receiverScopes, config); } } @@ -2953,8 +2953,8 @@ void Resolver::resolveIdentifier(const Identifier* ident, auto parenlessInfo = ParenlessOverloadInfo(); if (ident->name() == "size" || ident->name() == "this.size") { gdbShouldBreakHere(); - /* auto& asdf = receiverScopes.front(); */ - /* (void)asdf; */ + auto& asdf = receiverScopes.front(); + (void)asdf; } auto vec = lookupIdentifier(ident, receiverScopes, parenlessInfo); @@ -2970,27 +2970,6 @@ void Resolver::resolveIdentifier(const Identifier* ident, // can't establish the type. If this is in a function // call, we'll establish it later anyway. result.setType(QualifiedType()); - - QualifiedType receiverType; - ID receiverIdIgnored; - if (getMethodReceiver(&receiverType, &receiverIdIgnored) && receiverType.type()) { - // resolve a.x where a is a record/class and x is a field or parenless - // method - std::vector actuals; - actuals.push_back(CallInfoActual(receiverType, USTR("this"))); - auto ci = CallInfo(/* name */ ident->name(), - /* calledType */ QualifiedType(), - /* isMethodCall */ true, - /* hasQuestionArg */ false, - /* isParenless */ true, actuals); - auto inScope = scopeStack.back(); - auto inScopes = CallScopeInfo::forNormalCall(inScope, poiScope); - auto c = resolveGeneratedCall(context, ident, ci, inScopes); - // save the most specific candidates in the resolution result for the id - if (!handleResolvedCallWithoutError(result, ident, ci, c)) { - return; - } - } } else { // vec.size() == 1 and vec[0].numIds() <= 1 const IdAndFlags& idv = vec[0].firstIdAndFlags(); From 18b65577146f8364ba83a408a06135e192529f19 Mon Sep 17 00:00:00 2001 From: Anna Rift Date: Wed, 12 Jun 2024 12:43:43 -0700 Subject: [PATCH 04/19] Attempt based on filtering candidates by receiver type in resolveIdentifier Signed-off-by: Anna Rift --- frontend/lib/resolution/Resolver.cpp | 190 ++++++++++++++++----------- frontend/lib/resolution/Resolver.h | 2 + 2 files changed, 116 insertions(+), 76 deletions(-) diff --git a/frontend/lib/resolution/Resolver.cpp b/frontend/lib/resolution/Resolver.cpp index aec2d35a886c..9067ee1d2364 100644 --- a/frontend/lib/resolution/Resolver.cpp +++ b/frontend/lib/resolution/Resolver.cpp @@ -2715,7 +2715,7 @@ Resolver::lookupIdentifier(const Identifier* ident, } else if (ambiguous && !outParenlessOverloadInfo.areCandidatesOnlyParenlessProcs() && !resolvingCalledIdent) { - issueAmbiguityErrorIfNeeded(ident, scope, receiverScopes, config); + /* issueAmbiguityErrorIfNeeded(ident, scope, receiverScopes, config); */ } } @@ -2937,6 +2937,84 @@ void Resolver::tryResolveParenlessCall(const ParenlessOverloadInfo& info, } } +void Resolver::resolveIdentifierHandleResult( + const Identifier* ident, ID id, + ResolvedExpression& result) { + assert(!id.isEmpty()); + + // use the type established at declaration/initialization, + // but for things with generic type, use unknown. + QualifiedType type = typeForId(id, /*localGenericToUnknown*/ true); + + maybeEmitWarningsForId(this, type, ident, id); + + // Record uses of outer variables. + if (isOuterVariable(this, id) && outerVars) { + const ID& mention = ident->id(); + const ID& var = id; + outerVars->add(context, mention, var); + } + + if (type.kind() == QualifiedType::TYPE) { + // now, for a type that is generic with defaults, + // compute the default version when needed. e.g. + // record R { type t = int; } + // var x: R; // should refer to R(int) + bool computeDefaults = true; + bool resolvingCalledIdent = nearestCalledExpression() == ident; + + // For calls like + // + // type myType = anotherType(int) + // + // Use the generic version of anotherType to feed as receiver. + if (resolvingCalledIdent) { + computeDefaults = false; + } + + // Other special exceptions like 'r' in: + // + // proc r.init() { ... } + // + if (!genericReceiverOverrideStack.empty()) { + auto& topEntry = genericReceiverOverrideStack.back(); + if ((topEntry.first.isEmpty() || topEntry.first == ident->name()) && + topEntry.second == parsing::parentAst(context, ident)) { + computeDefaults = false; + } + } + + if (computeDefaults) { + type = computeTypeDefaults(*this, type); + } + // Do not resolve function calls under 'scopeResolveOnly' + } else if (type.kind() == QualifiedType::PARENLESS_FUNCTION) { + CHPL_ASSERT(scopeResolveOnly && + "resolution of parenless functions should've happened above"); + + // Possibly a "compatibility hack" with production: we haven't checked + // whether the call is valid, but the production scope resolver doesn't + // care and assumes `ident` points to this parenless function. Setting + // the toId also helps determine if this is a method call and should + // have `this` inserted, as well as whether or not to turn this + // into a parenless call. + + // Fall through to validateAndSetToId + } else if (scopeResolveOnly && type.kind() == QualifiedType::FUNCTION) { + // Possibly a "compatibility hack" with production: we haven't checked + // whether the call is valid, but the production scope resolver doesn't + // care and assumes `ident` points to this function. Setting + // the toId also helps determine if this is a method call + + // Fall through to validateAndSetToId + } + + validateAndSetToId(result, ident, id); + result.setType(type); + // if there are multiple ids we should have gotten + // a multiple definition error at the declarations. +} + void Resolver::resolveIdentifier(const Identifier* ident, llvm::ArrayRef receiverScopes) { ResolvedExpression& result = byPostorder.byAst(ident); @@ -2967,9 +3045,39 @@ void Resolver::resolveIdentifier(const Identifier* ident, } else if (vec.size() == 0) { result.setType(QualifiedType()); } else if (vec.size() > 1 || vec[0].numIds() > 1) { - // can't establish the type. If this is in a function - // call, we'll establish it later anyway. - result.setType(QualifiedType()); + // See if ambiguity can be resolved by considering method receiver types. + ID potentialResult; + for (auto& ids : vec) { + for (auto idIt = ids.begin(); idIt != ids.end(); ++idIt) { + auto cur = idIt.curIdAndFlags(); + // Skip considering methods where receiver type doesn't match. + if (cur.isMethod()) { + auto ast = parsing::idToAst(context, cur.id())->toFunction(); + ResolvedExpression& r = byPostorder.byAst(ast->thisFormal()); + QualifiedType curTy = r.type(); + if (curTy.type()) { + if (curTy != methodReceiverType()) { + continue; + } + } + } + + if (potentialResult.isEmpty()) { + potentialResult = cur.id(); + } else { + potentialResult = ID(); + break; + } + } + } + + if (potentialResult.isEmpty()) { + // can't establish the type. If this is in a function + // call, we'll establish it later anyway. + result.setType(QualifiedType()); + } else { + resolveIdentifierHandleResult(ident, potentialResult, result); + } } else { // vec.size() == 1 and vec[0].numIds() <= 1 const IdAndFlags& idv = vec[0].firstIdAndFlags(); @@ -2999,79 +3107,9 @@ void Resolver::resolveIdentifier(const Identifier* ident, result.setType(type); return; + } else { + resolveIdentifierHandleResult(ident, id, result); } - - // use the type established at declaration/initialization, - // but for things with generic type, use unknown. - type = typeForId(id, /*localGenericToUnknown*/ true); - - maybeEmitWarningsForId(this, type, ident, id); - - // Record uses of outer variables. - if (isOuterVariable(this, id) && outerVars) { - const ID& mention = ident->id(); - const ID& var = id; - outerVars->add(context, mention, var); - } - - if (type.kind() == QualifiedType::TYPE) { - // now, for a type that is generic with defaults, - // compute the default version when needed. e.g. - // record R { type t = int; } - // var x: R; // should refer to R(int) - bool computeDefaults = true; - bool resolvingCalledIdent = nearestCalledExpression() == ident; - - // For calls like - // - // type myType = anotherType(int) - // - // Use the generic version of anotherType to feed as receiver. - if (resolvingCalledIdent) { - computeDefaults = false; - } - - // Other special exceptions like 'r' in: - // - // proc r.init() { ... } - // - if (!genericReceiverOverrideStack.empty()) { - auto& topEntry = genericReceiverOverrideStack.back(); - if ((topEntry.first.isEmpty() || topEntry.first == ident->name()) && - topEntry.second == parsing::parentAst(context, ident)) { - computeDefaults = false; - } - } - - if (computeDefaults) { - type = computeTypeDefaults(*this, type); - } - // Do not resolve function calls under 'scopeResolveOnly' - } else if (type.kind() == QualifiedType::PARENLESS_FUNCTION) { - CHPL_ASSERT(scopeResolveOnly && "resolution of parenless functions should've happened above"); - - // Possibly a "compatibility hack" with production: we haven't checked - // whether the call is valid, but the production scope resolver doesn't - // care and assumes `ident` points to this parenless function. Setting - // the toId also helps determine if this is a method call and should - // have `this` inserted, as well as whether or not to turn this - // into a parenless call. - - // Fall through to validateAndSetToId - } else if (scopeResolveOnly && - type.kind() == QualifiedType::FUNCTION) { - // Possibly a "compatibility hack" with production: we haven't checked - // whether the call is valid, but the production scope resolver doesn't - // care and assumes `ident` points to this function. Setting - // the toId also helps determine if this is a method call - - // Fall through to validateAndSetToId - } - - validateAndSetToId(result, ident, id); - result.setType(type); - // if there are multiple ids we should have gotten - // a multiple definition error at the declarations. } } diff --git a/frontend/lib/resolution/Resolver.h b/frontend/lib/resolution/Resolver.h index ab6ccec358bc..c0e902002806 100644 --- a/frontend/lib/resolution/Resolver.h +++ b/frontend/lib/resolution/Resolver.h @@ -542,6 +542,8 @@ struct Resolver { const uast::Identifier* ident, llvm::ArrayRef receiverScopes); + void resolveIdentifierHandleResult(const uast::Identifier* ident, ID id, + ResolvedExpression& result); void resolveIdentifier(const uast::Identifier* ident, llvm::ArrayRef receiverScopes); From 645e1d4982c2cd2278a90f5a3ea4cd044331f4e6 Mon Sep 17 00:00:00 2001 From: Anna Rift Date: Wed, 12 Jun 2024 12:43:47 -0700 Subject: [PATCH 05/19] Revert "Attempt based on filtering candidates by receiver type in resolveIdentifier" This reverts commit b9e6f13d6c739568b3e9570f8add3dd34f32c801. Signed-off-by: Anna Rift --- frontend/lib/resolution/Resolver.cpp | 190 +++++++++++---------------- frontend/lib/resolution/Resolver.h | 2 - 2 files changed, 76 insertions(+), 116 deletions(-) diff --git a/frontend/lib/resolution/Resolver.cpp b/frontend/lib/resolution/Resolver.cpp index 9067ee1d2364..aec2d35a886c 100644 --- a/frontend/lib/resolution/Resolver.cpp +++ b/frontend/lib/resolution/Resolver.cpp @@ -2715,7 +2715,7 @@ Resolver::lookupIdentifier(const Identifier* ident, } else if (ambiguous && !outParenlessOverloadInfo.areCandidatesOnlyParenlessProcs() && !resolvingCalledIdent) { - /* issueAmbiguityErrorIfNeeded(ident, scope, receiverScopes, config); */ + issueAmbiguityErrorIfNeeded(ident, scope, receiverScopes, config); } } @@ -2937,84 +2937,6 @@ void Resolver::tryResolveParenlessCall(const ParenlessOverloadInfo& info, } } -void Resolver::resolveIdentifierHandleResult( - const Identifier* ident, ID id, - ResolvedExpression& result) { - assert(!id.isEmpty()); - - // use the type established at declaration/initialization, - // but for things with generic type, use unknown. - QualifiedType type = typeForId(id, /*localGenericToUnknown*/ true); - - maybeEmitWarningsForId(this, type, ident, id); - - // Record uses of outer variables. - if (isOuterVariable(this, id) && outerVars) { - const ID& mention = ident->id(); - const ID& var = id; - outerVars->add(context, mention, var); - } - - if (type.kind() == QualifiedType::TYPE) { - // now, for a type that is generic with defaults, - // compute the default version when needed. e.g. - // record R { type t = int; } - // var x: R; // should refer to R(int) - bool computeDefaults = true; - bool resolvingCalledIdent = nearestCalledExpression() == ident; - - // For calls like - // - // type myType = anotherType(int) - // - // Use the generic version of anotherType to feed as receiver. - if (resolvingCalledIdent) { - computeDefaults = false; - } - - // Other special exceptions like 'r' in: - // - // proc r.init() { ... } - // - if (!genericReceiverOverrideStack.empty()) { - auto& topEntry = genericReceiverOverrideStack.back(); - if ((topEntry.first.isEmpty() || topEntry.first == ident->name()) && - topEntry.second == parsing::parentAst(context, ident)) { - computeDefaults = false; - } - } - - if (computeDefaults) { - type = computeTypeDefaults(*this, type); - } - // Do not resolve function calls under 'scopeResolveOnly' - } else if (type.kind() == QualifiedType::PARENLESS_FUNCTION) { - CHPL_ASSERT(scopeResolveOnly && - "resolution of parenless functions should've happened above"); - - // Possibly a "compatibility hack" with production: we haven't checked - // whether the call is valid, but the production scope resolver doesn't - // care and assumes `ident` points to this parenless function. Setting - // the toId also helps determine if this is a method call and should - // have `this` inserted, as well as whether or not to turn this - // into a parenless call. - - // Fall through to validateAndSetToId - } else if (scopeResolveOnly && type.kind() == QualifiedType::FUNCTION) { - // Possibly a "compatibility hack" with production: we haven't checked - // whether the call is valid, but the production scope resolver doesn't - // care and assumes `ident` points to this function. Setting - // the toId also helps determine if this is a method call - - // Fall through to validateAndSetToId - } - - validateAndSetToId(result, ident, id); - result.setType(type); - // if there are multiple ids we should have gotten - // a multiple definition error at the declarations. -} - void Resolver::resolveIdentifier(const Identifier* ident, llvm::ArrayRef receiverScopes) { ResolvedExpression& result = byPostorder.byAst(ident); @@ -3045,39 +2967,9 @@ void Resolver::resolveIdentifier(const Identifier* ident, } else if (vec.size() == 0) { result.setType(QualifiedType()); } else if (vec.size() > 1 || vec[0].numIds() > 1) { - // See if ambiguity can be resolved by considering method receiver types. - ID potentialResult; - for (auto& ids : vec) { - for (auto idIt = ids.begin(); idIt != ids.end(); ++idIt) { - auto cur = idIt.curIdAndFlags(); - // Skip considering methods where receiver type doesn't match. - if (cur.isMethod()) { - auto ast = parsing::idToAst(context, cur.id())->toFunction(); - ResolvedExpression& r = byPostorder.byAst(ast->thisFormal()); - QualifiedType curTy = r.type(); - if (curTy.type()) { - if (curTy != methodReceiverType()) { - continue; - } - } - } - - if (potentialResult.isEmpty()) { - potentialResult = cur.id(); - } else { - potentialResult = ID(); - break; - } - } - } - - if (potentialResult.isEmpty()) { - // can't establish the type. If this is in a function - // call, we'll establish it later anyway. - result.setType(QualifiedType()); - } else { - resolveIdentifierHandleResult(ident, potentialResult, result); - } + // can't establish the type. If this is in a function + // call, we'll establish it later anyway. + result.setType(QualifiedType()); } else { // vec.size() == 1 and vec[0].numIds() <= 1 const IdAndFlags& idv = vec[0].firstIdAndFlags(); @@ -3107,9 +2999,79 @@ void Resolver::resolveIdentifier(const Identifier* ident, result.setType(type); return; - } else { - resolveIdentifierHandleResult(ident, id, result); } + + // use the type established at declaration/initialization, + // but for things with generic type, use unknown. + type = typeForId(id, /*localGenericToUnknown*/ true); + + maybeEmitWarningsForId(this, type, ident, id); + + // Record uses of outer variables. + if (isOuterVariable(this, id) && outerVars) { + const ID& mention = ident->id(); + const ID& var = id; + outerVars->add(context, mention, var); + } + + if (type.kind() == QualifiedType::TYPE) { + // now, for a type that is generic with defaults, + // compute the default version when needed. e.g. + // record R { type t = int; } + // var x: R; // should refer to R(int) + bool computeDefaults = true; + bool resolvingCalledIdent = nearestCalledExpression() == ident; + + // For calls like + // + // type myType = anotherType(int) + // + // Use the generic version of anotherType to feed as receiver. + if (resolvingCalledIdent) { + computeDefaults = false; + } + + // Other special exceptions like 'r' in: + // + // proc r.init() { ... } + // + if (!genericReceiverOverrideStack.empty()) { + auto& topEntry = genericReceiverOverrideStack.back(); + if ((topEntry.first.isEmpty() || topEntry.first == ident->name()) && + topEntry.second == parsing::parentAst(context, ident)) { + computeDefaults = false; + } + } + + if (computeDefaults) { + type = computeTypeDefaults(*this, type); + } + // Do not resolve function calls under 'scopeResolveOnly' + } else if (type.kind() == QualifiedType::PARENLESS_FUNCTION) { + CHPL_ASSERT(scopeResolveOnly && "resolution of parenless functions should've happened above"); + + // Possibly a "compatibility hack" with production: we haven't checked + // whether the call is valid, but the production scope resolver doesn't + // care and assumes `ident` points to this parenless function. Setting + // the toId also helps determine if this is a method call and should + // have `this` inserted, as well as whether or not to turn this + // into a parenless call. + + // Fall through to validateAndSetToId + } else if (scopeResolveOnly && + type.kind() == QualifiedType::FUNCTION) { + // Possibly a "compatibility hack" with production: we haven't checked + // whether the call is valid, but the production scope resolver doesn't + // care and assumes `ident` points to this function. Setting + // the toId also helps determine if this is a method call + + // Fall through to validateAndSetToId + } + + validateAndSetToId(result, ident, id); + result.setType(type); + // if there are multiple ids we should have gotten + // a multiple definition error at the declarations. } } diff --git a/frontend/lib/resolution/Resolver.h b/frontend/lib/resolution/Resolver.h index c0e902002806..ab6ccec358bc 100644 --- a/frontend/lib/resolution/Resolver.h +++ b/frontend/lib/resolution/Resolver.h @@ -542,8 +542,6 @@ struct Resolver { const uast::Identifier* ident, llvm::ArrayRef receiverScopes); - void resolveIdentifierHandleResult(const uast::Identifier* ident, ID id, - ResolvedExpression& result); void resolveIdentifier(const uast::Identifier* ident, llvm::ArrayRef receiverScopes); From b77ee731201b22c27b53d7ae3135d85e6e00e4cf Mon Sep 17 00:00:00 2001 From: Anna Rift Date: Wed, 12 Jun 2024 12:50:03 -0700 Subject: [PATCH 06/19] Restore function resolution attempt, plus more work Signed-off-by: Anna Rift --- frontend/lib/parsing/parsing-queries.cpp | 6 ++-- frontend/lib/resolution/Resolver.cpp | 42 ++++++++++++++++++++++-- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/frontend/lib/parsing/parsing-queries.cpp b/frontend/lib/parsing/parsing-queries.cpp index ae33f5387388..a7ac84c14dad 100644 --- a/frontend/lib/parsing/parsing-queries.cpp +++ b/frontend/lib/parsing/parsing-queries.cpp @@ -1315,10 +1315,10 @@ idContainsFieldWithNameQuery(Context* context, ID typeDeclId, UniqueString field bool result = false; auto ast = parsing::idToAst(context, typeDeclId); - if (ast && ast->isAggregateDecl()) { - auto ad = ast->toAggregateDecl(); + if (ast && (ast->isAggregateDecl() || ast->isFunction())) { + /* auto ad = ast->toAggregateDecl(); */ - for (auto child: ad->children()) { + for (auto child: ast->children()) { // Ignore everything other than VarLikeDecl, MultiDecl, TupleDecl if (child->isVarLikeDecl() || child->isMultiDecl() || diff --git a/frontend/lib/resolution/Resolver.cpp b/frontend/lib/resolution/Resolver.cpp index aec2d35a886c..6dc9e682e8fc 100644 --- a/frontend/lib/resolution/Resolver.cpp +++ b/frontend/lib/resolution/Resolver.cpp @@ -2715,7 +2715,7 @@ Resolver::lookupIdentifier(const Identifier* ident, } else if (ambiguous && !outParenlessOverloadInfo.areCandidatesOnlyParenlessProcs() && !resolvingCalledIdent) { - issueAmbiguityErrorIfNeeded(ident, scope, receiverScopes, config); + /* issueAmbiguityErrorIfNeeded(ident, scope, receiverScopes, config); */ } } @@ -2953,8 +2953,8 @@ void Resolver::resolveIdentifier(const Identifier* ident, auto parenlessInfo = ParenlessOverloadInfo(); if (ident->name() == "size" || ident->name() == "this.size") { gdbShouldBreakHere(); - auto& asdf = receiverScopes.front(); - (void)asdf; + /* auto& asdf = receiverScopes.front(); */ + /* (void)asdf; */ } auto vec = lookupIdentifier(ident, receiverScopes, parenlessInfo); @@ -2970,6 +2970,42 @@ void Resolver::resolveIdentifier(const Identifier* ident, // can't establish the type. If this is in a function // call, we'll establish it later anyway. result.setType(QualifiedType()); + + QualifiedType receiverType; + ID receiverId; + if (getMethodReceiver(&receiverType, &receiverId) && receiverType.type()) { + // resolve a.x where a is a record/class and x is a field or parenless + // method + std::vector actuals; + actuals.push_back(CallInfoActual(receiverType, USTR("this"))); + auto ci = CallInfo(/* name */ ident->name(), + /* calledType */ QualifiedType(), + /* isMethodCall */ true, + /* hasQuestionArg */ false, + /* isParenless */ true, actuals); + auto inScope = scopeStack.back(); + auto inScopes = CallScopeInfo::forNormalCall(inScope, poiScope); + auto c = resolveGeneratedCall(context, ident, ci, inScopes); + for (const auto* scope : receiverScopes) { + if (parsing::idContainsFieldWithName(context, scope->id(), + ident->name())) { + context->error(ident, "asdf parenless proc redeclares the field '%s'", + ident->name().c_str()); + return; + } + } + if (parsing::idContainsFieldWithName( + context, receiverType.type()->toCompositeType()->id(), + ident->name())) { + context->error(ident, "parenless proc redeclares the field '%s'", + ident->name().c_str()); + return; + } + // save the most specific candidates in the resolution result for the id + if (!handleResolvedCallWithoutError(result, ident, ci, c)) { + return; + } + } } else { // vec.size() == 1 and vec[0].numIds() <= 1 const IdAndFlags& idv = vec[0].firstIdAndFlags(); From 00d539046cf287e119b4ae694a19604950eefa71 Mon Sep 17 00:00:00 2001 From: Anna Rift Date: Wed, 12 Jun 2024 13:13:25 -0700 Subject: [PATCH 07/19] Fix failing to emit ambiguity for parenless field redeclaration Signed-off-by: Anna Rift --- frontend/lib/parsing/parsing-queries.cpp | 19 +++++++++++++++--- frontend/lib/resolution/Resolver.cpp | 25 +++++++++++++++--------- 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/frontend/lib/parsing/parsing-queries.cpp b/frontend/lib/parsing/parsing-queries.cpp index a7ac84c14dad..7ab093ffaa88 100644 --- a/frontend/lib/parsing/parsing-queries.cpp +++ b/frontend/lib/parsing/parsing-queries.cpp @@ -1315,10 +1315,10 @@ idContainsFieldWithNameQuery(Context* context, ID typeDeclId, UniqueString field bool result = false; auto ast = parsing::idToAst(context, typeDeclId); - if (ast && (ast->isAggregateDecl() || ast->isFunction())) { - /* auto ad = ast->toAggregateDecl(); */ + if (ast && ast->isAggregateDecl()) { + auto ad = ast->toAggregateDecl(); - for (auto child: ast->children()) { + for (auto child: ad->children()) { // Ignore everything other than VarLikeDecl, MultiDecl, TupleDecl if (child->isVarLikeDecl() || child->isMultiDecl() || @@ -1330,6 +1330,19 @@ idContainsFieldWithNameQuery(Context* context, ID typeDeclId, UniqueString field } } } + } else if (ast && ast->isFunction()) { + auto fn = ast->toFunction(); + for (auto child: fn->stmts()) { + if (child->isVarLikeDecl() || + child->isMultiDecl() || + child->isTupleDecl()) { + bool found = helpFieldNameCheck(child, fieldName); + if (found) { + result = true; + break; + } + } + } } return QUERY_END(result); diff --git a/frontend/lib/resolution/Resolver.cpp b/frontend/lib/resolution/Resolver.cpp index 6dc9e682e8fc..ae34b0e6f4bb 100644 --- a/frontend/lib/resolution/Resolver.cpp +++ b/frontend/lib/resolution/Resolver.cpp @@ -2986,16 +2986,23 @@ void Resolver::resolveIdentifier(const Identifier* ident, auto inScope = scopeStack.back(); auto inScopes = CallScopeInfo::forNormalCall(inScope, poiScope); auto c = resolveGeneratedCall(context, ident, ci, inScopes); - for (const auto* scope : receiverScopes) { - if (parsing::idContainsFieldWithName(context, scope->id(), - ident->name())) { - context->error(ident, "asdf parenless proc redeclares the field '%s'", - ident->name().c_str()); - return; - } - } + /* for (const auto* scope : receiverScopes) { */ + /* if (parsing::idContainsFieldWithName(context, scope->id(), */ + /* ident->name())) { */ + /* context->error(ident, "asdf parenless proc redeclares the field '%s'", */ + /* ident->name().c_str()); */ + /* return; */ + /* } */ + /* } */ + /* if (parsing::idContainsFieldWithName( */ + /* context, receiverType.type()->toCompositeType()->id(), */ + /* ident->name())) { */ + /* context->error(ident, "parenless proc redeclares the field '%s'", */ + /* ident->name().c_str()); */ + /* return; */ + /* } */ if (parsing::idContainsFieldWithName( - context, receiverType.type()->toCompositeType()->id(), + context, typedSignature->untyped()->id(), ident->name())) { context->error(ident, "parenless proc redeclares the field '%s'", ident->name().c_str()); From f942f3ca4d50e4fe0192604dccf21c1fe3d571e4 Mon Sep 17 00:00:00 2001 From: Anna Rift Date: Thu, 13 Jun 2024 10:51:50 -0700 Subject: [PATCH 08/19] Test this.foo local parenless vs local var disambiguation case The behavior of this case is unchanged by this PR, but I just noticed it and thought it should be covered. Signed-off-by: Anna Rift --- .../test/resolution/testMethodFieldAccess.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/frontend/test/resolution/testMethodFieldAccess.cpp b/frontend/test/resolution/testMethodFieldAccess.cpp index 4188620d0390..58d1482a7600 100644 --- a/frontend/test/resolution/testMethodFieldAccess.cpp +++ b/frontend/test/resolution/testMethodFieldAccess.cpp @@ -725,6 +725,24 @@ static void testExample5a() { "" /* ambiguity */); } +// Like 5a but this.foo instead of foo +static void testExample5b() { + testCall("example5b.chpl", + R""""( + module M { + record r { } + proc r.test() { + var foo: int; + proc r.foo { } + this.foo; + } + } + )"""", + "M.test", + "M.test@5", + "M.test.foo"); +} + static void testExample6() { testCall("example6.chpl", R""""( @@ -1230,6 +1248,7 @@ int main() { testExample4a(); testExample5(); testExample5a(); + testExample5b(); testExample6(); testExample7(); testExample8(); From dbb04290d0d2ac4eed874da84cbeeb8577f807c7 Mon Sep 17 00:00:00 2001 From: Anna Rift Date: Thu, 13 Jun 2024 11:03:15 -0700 Subject: [PATCH 09/19] Test fixed disambiguation cases Signed-off-by: Anna Rift --- .../test/resolution/testMethodFieldAccess.cpp | 107 ++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/frontend/test/resolution/testMethodFieldAccess.cpp b/frontend/test/resolution/testMethodFieldAccess.cpp index 58d1482a7600..119ea7219ba3 100644 --- a/frontend/test/resolution/testMethodFieldAccess.cpp +++ b/frontend/test/resolution/testMethodFieldAccess.cpp @@ -1208,6 +1208,112 @@ static void testExample27() { "" /* both false; no matches. */); } +// Check diambiguation of identifiers in methods, between a +// field and a proc on a different receiver. +static void testExample28() { + { + testCall("example28a.chpl", + R""""( + module M { + record foo { + var size : int; + } + record bar { + } + proc bar.size do return 3; + proc foo.asdf() { + return size; + } + } + )"""", + "M.asdf", + "M.asdf@2", + "M.foo@1"); + } + + // Same as 28a but with primary calling method + { + testCall("example28b.chpl", + R""""( + module M { + record foo { + var size : int; + proc asdf() { + return size; + } + } + record bar { + } + proc bar.size do return 3; + } + )"""", + "M.foo.asdf", + "M.foo.asdf@1", + "M.foo@1"); + } + + // Same as 28a but with primary called method + { + testCall("example28c.chpl", + R""""( + module M { + record foo { + var size : int; + proc asdf() { + return size; + } + } + record bar { + proc size do return 3; + } + } + )"""", + "M.foo.asdf", + "M.foo.asdf@1", + "M.foo@1"); + } + + // Same as 28b but with primary called method + { + testCall("example28d.chpl", + R""""( + module M { + record foo { + var size : int; + proc asdf() { + return size; + } + } + record bar { + proc size do return 3; + } + } + )"""", + "M.foo.asdf", + "M.foo.asdf@1", + "M.foo@1"); + } + + // Sanity check: 2a but with same receiver, clearly ambiguous + { + testCall("example28e.chpl", + R""""( + module M { + record foo { + var size : int; + } + proc foo.size do return 3; + proc foo.asdf() { + return size; + } + } + )"""", + "M.asdf", + "M.asdf@2", + "" /* ambiguous */); + } +} + int main() { test1r(); test1c(); @@ -1271,6 +1377,7 @@ int main() { testExample25(); testExample26(); testExample27(); + testExample28(); return 0; } From 1e2e43829fa50ff7c72e71139fd22751b5fff7a6 Mon Sep 17 00:00:00 2001 From: Anna Rift Date: Thu, 13 Jun 2024 11:33:16 -0700 Subject: [PATCH 10/19] Clarify and refactor adjusted idContainsFieldWithName Signed-off-by: Anna Rift --- .../include/chpl/parsing/parsing-queries.h | 7 +-- frontend/lib/parsing/parsing-queries.cpp | 45 ++++++++----------- 2 files changed, 23 insertions(+), 29 deletions(-) diff --git a/frontend/include/chpl/parsing/parsing-queries.h b/frontend/include/chpl/parsing/parsing-queries.h index eab09da7b855..4759257bf2ea 100644 --- a/frontend/include/chpl/parsing/parsing-queries.h +++ b/frontend/include/chpl/parsing/parsing-queries.h @@ -478,10 +478,11 @@ bool idIsFunctionWithWhere(Context* context, ID id); ID idToContainingMultiDeclId(Context* context, ID id); /** - Given an ID for a Record/Union/Class Decl, - returns 'true' if the passed name is the name of a field contained in it. + Given an ID for an aggregate decl or function, + returns 'true' if the passed name is the name of a field/variable contained in + it. */ -bool idContainsFieldWithName(Context* context, ID typeDeclId, +bool idContainsFieldWithName(Context* context, ID declId, UniqueString fieldName); /** diff --git a/frontend/lib/parsing/parsing-queries.cpp b/frontend/lib/parsing/parsing-queries.cpp index 7ab093ffaa88..8e4b8b2db023 100644 --- a/frontend/lib/parsing/parsing-queries.cpp +++ b/frontend/lib/parsing/parsing-queries.cpp @@ -1309,34 +1309,27 @@ static bool helpFieldNameCheck(const AstNode* ast, return false; } -static const bool& -idContainsFieldWithNameQuery(Context* context, ID typeDeclId, UniqueString fieldName) { - QUERY_BEGIN(idContainsFieldWithNameQuery, context, typeDeclId, fieldName); +static const bool& idContainsFieldWithNameQuery(Context* context, ID declId, + UniqueString fieldName) { + QUERY_BEGIN(idContainsFieldWithNameQuery, context, declId, fieldName); bool result = false; - auto ast = parsing::idToAst(context, typeDeclId); - if (ast && ast->isAggregateDecl()) { - auto ad = ast->toAggregateDecl(); + if (auto ast = parsing::idToAst(context, declId)) { + AstList dummy; + AstListIteratorPair potentialDecls(dummy.cbegin(), dummy.cend()); + if (auto ad = ast->toAggregateDecl()) { + potentialDecls = ad->children(); + } else if (auto fn = ast->toFunction()) { + potentialDecls = fn->stmts(); + } else { + // Empty list, nothing to check + } - for (auto child: ad->children()) { + for (auto potentialDecl : potentialDecls) { // Ignore everything other than VarLikeDecl, MultiDecl, TupleDecl - if (child->isVarLikeDecl() || - child->isMultiDecl() || - child->isTupleDecl()) { - bool found = helpFieldNameCheck(child, fieldName); - if (found) { - result = true; - break; - } - } - } - } else if (ast && ast->isFunction()) { - auto fn = ast->toFunction(); - for (auto child: fn->stmts()) { - if (child->isVarLikeDecl() || - child->isMultiDecl() || - child->isTupleDecl()) { - bool found = helpFieldNameCheck(child, fieldName); + if (potentialDecl->isVarLikeDecl() || potentialDecl->isMultiDecl() || + potentialDecl->isTupleDecl()) { + bool found = helpFieldNameCheck(potentialDecl, fieldName); if (found) { result = true; break; @@ -1348,8 +1341,8 @@ idContainsFieldWithNameQuery(Context* context, ID typeDeclId, UniqueString field return QUERY_END(result); } -bool idContainsFieldWithName(Context* context, ID typeDeclId, UniqueString fieldName) { - return idContainsFieldWithNameQuery(context, typeDeclId, fieldName); +bool idContainsFieldWithName(Context* context, ID declId, UniqueString fieldName) { + return idContainsFieldWithNameQuery(context, declId, fieldName); } static bool helpFindFieldId(const AstNode* ast, From 07085758f3f166029c1435471d6ab308cacd4d57 Mon Sep 17 00:00:00 2001 From: Anna Rift Date: Thu, 13 Jun 2024 11:35:01 -0700 Subject: [PATCH 11/19] Remove leftover debugging breakpoints Signed-off-by: Anna Rift --- frontend/lib/resolution/Resolver.cpp | 8 -------- frontend/lib/resolution/resolution-queries.cpp | 2 -- 2 files changed, 10 deletions(-) diff --git a/frontend/lib/resolution/Resolver.cpp b/frontend/lib/resolution/Resolver.cpp index ae34b0e6f4bb..5875ba6ef7aa 100644 --- a/frontend/lib/resolution/Resolver.cpp +++ b/frontend/lib/resolution/Resolver.cpp @@ -2652,7 +2652,6 @@ void Resolver::issueAmbiguityErrorIfNeeded(const Identifier* ident, ident->name(), prevConfig, traceResult); - gdbShouldBreakHere(); // emit an ambiguity error if this is not resolving a called ident CHPL_REPORT(context, AmbiguousIdentifier, ident, printFirstMention, vec, traceResult); @@ -2951,11 +2950,6 @@ void Resolver::resolveIdentifier(const Identifier* ident, // lookupIdentifier reports any errors that are needed auto parenlessInfo = ParenlessOverloadInfo(); - if (ident->name() == "size" || ident->name() == "this.size") { - gdbShouldBreakHere(); - /* auto& asdf = receiverScopes.front(); */ - /* (void)asdf; */ - } auto vec = lookupIdentifier(ident, receiverScopes, parenlessInfo); if (parenlessInfo.areCandidatesOnlyParenlessProcs() && !scopeResolveOnly) { @@ -3123,7 +3117,6 @@ bool Resolver::enter(const Identifier* ident) { std::ignore = initResolver->handleUseOfField(ident); return false; } else { - if (ident->name() == "size" || ident->name() == "this.size") gdbShouldBreakHere(); resolveIdentifier(ident, methodReceiverScopes()); return false; } @@ -3803,7 +3796,6 @@ void Resolver::exit(const Dot* dot) { if (initResolver && initResolver->handleUseOfField(dot)) return; ResolvedExpression& receiver = byPostorder.byAst(dot->receiver()); - if (dot->field() == "size") gdbShouldBreakHere(); bool deferToFunctionResolution = false; bool resolvingCalledDot = nearestCalledExpression() == dot; diff --git a/frontend/lib/resolution/resolution-queries.cpp b/frontend/lib/resolution/resolution-queries.cpp index 5b63ddf80b01..68dd34e7a8c7 100644 --- a/frontend/lib/resolution/resolution-queries.cpp +++ b/frontend/lib/resolution/resolution-queries.cpp @@ -4247,8 +4247,6 @@ CallResolutionResult resolveFnCall(Context* context, PoiInfo poiInfo; MostSpecificCandidates mostSpecific; - if (ci.name().str() == "size") gdbShouldBreakHere(); - // Note: currently type constructors are not implemented as methods if (ci.calledType().kind() == QualifiedType::TYPE && ci.isMethodCall() == false) { From d9255417bf4140a484b21c7ccaefd8948fcde1a1 Mon Sep 17 00:00:00 2001 From: Anna Rift Date: Thu, 13 Jun 2024 11:36:23 -0700 Subject: [PATCH 12/19] Slight refactor and remove unused code for new method resolution Signed-off-by: Anna Rift --- frontend/lib/resolution/Resolver.cpp | 25 +++---------------------- 1 file changed, 3 insertions(+), 22 deletions(-) diff --git a/frontend/lib/resolution/Resolver.cpp b/frontend/lib/resolution/Resolver.cpp index 5875ba6ef7aa..fa51caa4fb23 100644 --- a/frontend/lib/resolution/Resolver.cpp +++ b/frontend/lib/resolution/Resolver.cpp @@ -2980,31 +2980,12 @@ void Resolver::resolveIdentifier(const Identifier* ident, auto inScope = scopeStack.back(); auto inScopes = CallScopeInfo::forNormalCall(inScope, poiScope); auto c = resolveGeneratedCall(context, ident, ci, inScopes); - /* for (const auto* scope : receiverScopes) { */ - /* if (parsing::idContainsFieldWithName(context, scope->id(), */ - /* ident->name())) { */ - /* context->error(ident, "asdf parenless proc redeclares the field '%s'", */ - /* ident->name().c_str()); */ - /* return; */ - /* } */ - /* } */ - /* if (parsing::idContainsFieldWithName( */ - /* context, receiverType.type()->toCompositeType()->id(), */ - /* ident->name())) { */ - /* context->error(ident, "parenless proc redeclares the field '%s'", */ - /* ident->name().c_str()); */ - /* return; */ - /* } */ if (parsing::idContainsFieldWithName( - context, typedSignature->untyped()->id(), - ident->name())) { + context, typedSignature->untyped()->id(), ident->name())) { context->error(ident, "parenless proc redeclares the field '%s'", ident->name().c_str()); - return; - } - // save the most specific candidates in the resolution result for the id - if (!handleResolvedCallWithoutError(result, ident, ci, c)) { - return; + } else { + std::ignore = handleResolvedCallWithoutError(result, ident, ci, c); } } } else { From 08728fc139eef1bb9ad68510d09a7b36609b7c2a Mon Sep 17 00:00:00 2001 From: Anna Rift Date: Thu, 13 Jun 2024 11:37:25 -0700 Subject: [PATCH 13/19] Remove now-unused ambiguity error case in lookupIdentifier Signed-off-by: Anna Rift --- frontend/lib/resolution/Resolver.cpp | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/frontend/lib/resolution/Resolver.cpp b/frontend/lib/resolution/Resolver.cpp index fa51caa4fb23..ae3d76477770 100644 --- a/frontend/lib/resolution/Resolver.cpp +++ b/frontend/lib/resolution/Resolver.cpp @@ -2681,18 +2681,17 @@ Resolver::lookupIdentifier(const Identifier* ident, auto vec = lookupNameInScopeWithWarnings(context, scope, receiverScopes, ident->name(), config, ident->id()); - bool notFound = vec.empty(); - bool ambiguous = !notFound && (vec.size() > 1 || vec[0].numIds() > 1); - if (!vec.empty()) { + if (!notFound) { // We might be ambiguous, but due to having found multiple parenless procs. // It's not certain that this is an error; in particular, some parenless // procs can be ruled out if their 'where' clauses are false. If even // one identifier is not a parenless proc, there's an ambiguity. // // outParenlessOverloadInfo will be falsey if we found non-parenless-proc - // IDs, in which case we should emit an ambiguity error. + // IDs. In that case we may emit an ambiguity error later, after filtering + // out incorrect receivers. outParenlessOverloadInfo = ParenlessOverloadInfo::fromBorrowedIds(context, vec); } @@ -2711,10 +2710,6 @@ Resolver::lookupIdentifier(const Identifier* ident, CHPL_REPORT(context, UnknownIdentifier, ident, mentionedMoreThanOnce); } } - } else if (ambiguous && - !outParenlessOverloadInfo.areCandidatesOnlyParenlessProcs() && - !resolvingCalledIdent) { - /* issueAmbiguityErrorIfNeeded(ident, scope, receiverScopes, config); */ } } From 9242178d7bee0f21d0628f34ec76659b086e5223 Mon Sep 17 00:00:00 2001 From: Anna Rift Date: Thu, 13 Jun 2024 11:43:20 -0700 Subject: [PATCH 14/19] Clarify resolution done in resolveIdentifier Signed-off-by: Anna Rift --- frontend/lib/resolution/Resolver.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/frontend/lib/resolution/Resolver.cpp b/frontend/lib/resolution/Resolver.cpp index ae3d76477770..8ce3e7f1960d 100644 --- a/frontend/lib/resolution/Resolver.cpp +++ b/frontend/lib/resolution/Resolver.cpp @@ -2956,15 +2956,15 @@ void Resolver::resolveIdentifier(const Identifier* ident, } else if (vec.size() == 0) { result.setType(QualifiedType()); } else if (vec.size() > 1 || vec[0].numIds() > 1) { - // can't establish the type. If this is in a function + // Ambiguous, can't establish the type. If this is in a function // call, we'll establish it later anyway. result.setType(QualifiedType()); + // Check if we can break ambiguity by performing function resolution with + // an implicit 'this' receiver, to filter based on receiver type. QualifiedType receiverType; ID receiverId; if (getMethodReceiver(&receiverType, &receiverId) && receiverType.type()) { - // resolve a.x where a is a record/class and x is a field or parenless - // method std::vector actuals; actuals.push_back(CallInfoActual(receiverType, USTR("this"))); auto ci = CallInfo(/* name */ ident->name(), @@ -2975,11 +2975,14 @@ void Resolver::resolveIdentifier(const Identifier* ident, auto inScope = scopeStack.back(); auto inScopes = CallScopeInfo::forNormalCall(inScope, poiScope); auto c = resolveGeneratedCall(context, ident, ci, inScopes); + // Ensure we error out for redeclarations within the method itself, which + // function resolution doesn't catch. if (parsing::idContainsFieldWithName( context, typedSignature->untyped()->id(), ident->name())) { context->error(ident, "parenless proc redeclares the field '%s'", ident->name().c_str()); } else { + // Save resolution result if this was sufficient to break ambiguity. std::ignore = handleResolvedCallWithoutError(result, ident, ci, c); } } From 2b7f8e5d0b6da1476966d9122067a01be72110f2 Mon Sep 17 00:00:00 2001 From: Anna Rift Date: Thu, 13 Jun 2024 12:10:11 -0700 Subject: [PATCH 15/19] Move resolution to inside lookupIdentifier to preserve previous error Signed-off-by: Anna Rift --- frontend/lib/resolution/Resolver.cpp | 76 +++++++++++++++------------- 1 file changed, 40 insertions(+), 36 deletions(-) diff --git a/frontend/lib/resolution/Resolver.cpp b/frontend/lib/resolution/Resolver.cpp index 8ce3e7f1960d..7c6e678359fe 100644 --- a/frontend/lib/resolution/Resolver.cpp +++ b/frontend/lib/resolution/Resolver.cpp @@ -2658,10 +2658,9 @@ void Resolver::issueAmbiguityErrorIfNeeded(const Identifier* ident, } } -std::vector -Resolver::lookupIdentifier(const Identifier* ident, - llvm::ArrayRef receiverScopes, - ParenlessOverloadInfo& outParenlessOverloadInfo) { +std::vector Resolver::lookupIdentifier( + const Identifier* ident, llvm::ArrayRef receiverScopes, + ParenlessOverloadInfo& outParenlessOverloadInfo) { CHPL_ASSERT(scopeStack.size() > 0); const Scope* scope = scopeStack.back(); outParenlessOverloadInfo = ParenlessOverloadInfo(); @@ -2670,7 +2669,7 @@ Resolver::lookupIdentifier(const Identifier* ident, // Super is a keyword, and should't be looked up in scopes. Return // an empty ID to indicate that this identifier points to something, // but that something has a special meaning. - return { BorrowedIdsWithName::createWithBuiltinId() }; + return {BorrowedIdsWithName::createWithBuiltinId()}; } bool resolvingCalledIdent = nearestCalledExpression() == ident; @@ -2679,9 +2678,9 @@ Resolver::lookupIdentifier(const Identifier* ident, if (!resolvingCalledIdent) config |= LOOKUP_INNERMOST; auto vec = lookupNameInScopeWithWarnings(context, scope, receiverScopes, - ident->name(), config, - ident->id()); + ident->name(), config, ident->id()); bool notFound = vec.empty(); + bool ambiguous = !notFound && (vec.size() > 1 || vec[0].numIds() > 1); if (!notFound) { // We might be ambiguous, but due to having found multiple parenless procs. @@ -2692,7 +2691,8 @@ Resolver::lookupIdentifier(const Identifier* ident, // outParenlessOverloadInfo will be falsey if we found non-parenless-proc // IDs. In that case we may emit an ambiguity error later, after filtering // out incorrect receivers. - outParenlessOverloadInfo = ParenlessOverloadInfo::fromBorrowedIds(context, vec); + outParenlessOverloadInfo = + ParenlessOverloadInfo::fromBorrowedIds(context, vec); } // TODO: these errors should be enabled for scope resolution @@ -2710,6 +2710,37 @@ Resolver::lookupIdentifier(const Identifier* ident, CHPL_REPORT(context, UnknownIdentifier, ident, mentionedMoreThanOnce); } } + } else if (ambiguous && + !outParenlessOverloadInfo.areCandidatesOnlyParenlessProcs() && + !resolvingCalledIdent) { + // Check if we can break ambiguity by performing function resolution with + // an implicit 'this' receiver, to filter based on receiver type. + QualifiedType receiverType; + ID receiverId; + if (getMethodReceiver(&receiverType, &receiverId) && + receiverType.type()) { + std::vector actuals; + actuals.push_back(CallInfoActual(receiverType, USTR("this"))); + auto ci = CallInfo(/* name */ ident->name(), + /* calledType */ QualifiedType(), + /* isMethodCall */ true, + /* hasQuestionArg */ false, + /* isParenless */ true, actuals); + auto inScope = scopeStack.back(); + auto inScopes = CallScopeInfo::forNormalCall(inScope, poiScope); + auto c = resolveGeneratedCall(context, ident, ci, inScopes); + // Ensure we error out for redeclarations within the method itself, + // which function resolution doesn't catch. + if (parsing::idContainsFieldWithName( + context, typedSignature->untyped()->id(), ident->name())) { + context->error(ident, "parenless proc redeclares the field '%s'", + ident->name().c_str()); + } else { + // Save result if successful (emitting error otherwise). + ResolvedExpression& result = byPostorder.byAst(ident); + handleResolvedCall(result, ident, ci, c); + } + } } } @@ -2956,36 +2987,9 @@ void Resolver::resolveIdentifier(const Identifier* ident, } else if (vec.size() == 0) { result.setType(QualifiedType()); } else if (vec.size() > 1 || vec[0].numIds() > 1) { - // Ambiguous, can't establish the type. If this is in a function + // can't establish the type. If this is in a function // call, we'll establish it later anyway. result.setType(QualifiedType()); - - // Check if we can break ambiguity by performing function resolution with - // an implicit 'this' receiver, to filter based on receiver type. - QualifiedType receiverType; - ID receiverId; - if (getMethodReceiver(&receiverType, &receiverId) && receiverType.type()) { - std::vector actuals; - actuals.push_back(CallInfoActual(receiverType, USTR("this"))); - auto ci = CallInfo(/* name */ ident->name(), - /* calledType */ QualifiedType(), - /* isMethodCall */ true, - /* hasQuestionArg */ false, - /* isParenless */ true, actuals); - auto inScope = scopeStack.back(); - auto inScopes = CallScopeInfo::forNormalCall(inScope, poiScope); - auto c = resolveGeneratedCall(context, ident, ci, inScopes); - // Ensure we error out for redeclarations within the method itself, which - // function resolution doesn't catch. - if (parsing::idContainsFieldWithName( - context, typedSignature->untyped()->id(), ident->name())) { - context->error(ident, "parenless proc redeclares the field '%s'", - ident->name().c_str()); - } else { - // Save resolution result if this was sufficient to break ambiguity. - std::ignore = handleResolvedCallWithoutError(result, ident, ci, c); - } - } } else { // vec.size() == 1 and vec[0].numIds() <= 1 const IdAndFlags& idv = vec[0].firstIdAndFlags(); From c3cfbb604bcc7e20cfaf6ee87c86a918eb01fefe Mon Sep 17 00:00:00 2001 From: Anna Rift Date: Thu, 13 Jun 2024 13:10:35 -0700 Subject: [PATCH 16/19] Move lookup back into resolveIdentifier This better preserves the purity of lookupIdentifier Signed-off-by: Anna Rift --- frontend/lib/resolution/Resolver.cpp | 106 +++++++++++++-------------- 1 file changed, 51 insertions(+), 55 deletions(-) diff --git a/frontend/lib/resolution/Resolver.cpp b/frontend/lib/resolution/Resolver.cpp index 7c6e678359fe..6ff3d7b65240 100644 --- a/frontend/lib/resolution/Resolver.cpp +++ b/frontend/lib/resolution/Resolver.cpp @@ -2679,10 +2679,8 @@ std::vector Resolver::lookupIdentifier( auto vec = lookupNameInScopeWithWarnings(context, scope, receiverScopes, ident->name(), config, ident->id()); - bool notFound = vec.empty(); - bool ambiguous = !notFound && (vec.size() > 1 || vec[0].numIds() > 1); - if (!notFound) { + if (!vec.empty()) { // We might be ambiguous, but due to having found multiple parenless procs. // It's not certain that this is an error; in particular, some parenless // procs can be ruled out if their 'where' clauses are false. If even @@ -2695,55 +2693,6 @@ std::vector Resolver::lookupIdentifier( ParenlessOverloadInfo::fromBorrowedIds(context, vec); } - // TODO: these errors should be enabled for scope resolution - // but for now, they are off, as a temporary measure to enable - // the production compiler handle these cases. To enable this, - // we will have to adjust the dyno scope resolver to handle 'domain', - // and probably a few other features. - if (!scopeResolveOnly) { - if (notFound) { - if (!resolvingCalledIdent) { - auto pair = namesWithErrorsEmitted.insert(ident->name()); - if (pair.second) { - // insertion took place so emit the error - bool mentionedMoreThanOnce = identHasMoreMentions(ident); - CHPL_REPORT(context, UnknownIdentifier, ident, mentionedMoreThanOnce); - } - } - } else if (ambiguous && - !outParenlessOverloadInfo.areCandidatesOnlyParenlessProcs() && - !resolvingCalledIdent) { - // Check if we can break ambiguity by performing function resolution with - // an implicit 'this' receiver, to filter based on receiver type. - QualifiedType receiverType; - ID receiverId; - if (getMethodReceiver(&receiverType, &receiverId) && - receiverType.type()) { - std::vector actuals; - actuals.push_back(CallInfoActual(receiverType, USTR("this"))); - auto ci = CallInfo(/* name */ ident->name(), - /* calledType */ QualifiedType(), - /* isMethodCall */ true, - /* hasQuestionArg */ false, - /* isParenless */ true, actuals); - auto inScope = scopeStack.back(); - auto inScopes = CallScopeInfo::forNormalCall(inScope, poiScope); - auto c = resolveGeneratedCall(context, ident, ci, inScopes); - // Ensure we error out for redeclarations within the method itself, - // which function resolution doesn't catch. - if (parsing::idContainsFieldWithName( - context, typedSignature->untyped()->id(), ident->name())) { - context->error(ident, "parenless proc redeclares the field '%s'", - ident->name().c_str()); - } else { - // Save result if successful (emitting error otherwise). - ResolvedExpression& result = byPostorder.byAst(ident); - handleResolvedCall(result, ident, ci, c); - } - } - } - } - return vec; } @@ -2978,6 +2927,14 @@ void Resolver::resolveIdentifier(const Identifier* ident, auto parenlessInfo = ParenlessOverloadInfo(); auto vec = lookupIdentifier(ident, receiverScopes, parenlessInfo); + bool resolvingCalledIdent = nearestCalledExpression() == ident; + // TODO: these errors should be enabled for scope resolution + // but for now, they are off, as a temporary measure to enable + // the production compiler handle these cases. To enable this, + // we will have to adjust the dyno scope resolver to handle 'domain', + // and probably a few other features. + bool emitLookupErrors = !resolvingCalledIdent && !scopeResolveOnly; + if (parenlessInfo.areCandidatesOnlyParenlessProcs() && !scopeResolveOnly) { // Ambiguous, but a special case: there are many parenless functions. // This might be fine, if their 'where' clauses leave only one. @@ -2985,11 +2942,50 @@ void Resolver::resolveIdentifier(const Identifier* ident, // Call resolution will issue an error if the overload selection fails. tryResolveParenlessCall(parenlessInfo, ident, receiverScopes); } else if (vec.size() == 0) { + if (emitLookupErrors) { + auto pair = namesWithErrorsEmitted.insert(ident->name()); + if (pair.second) { + // insertion took place so emit the error + bool mentionedMoreThanOnce = identHasMoreMentions(ident); + CHPL_REPORT(context, UnknownIdentifier, ident, mentionedMoreThanOnce); + } + } + result.setType(QualifiedType()); } else if (vec.size() > 1 || vec[0].numIds() > 1) { - // can't establish the type. If this is in a function - // call, we'll establish it later anyway. - result.setType(QualifiedType()); + // Ambiguous. Check if we can break ambiguity by performing function resolution with + // an implicit 'this' receiver, to filter based on receiver type. + QualifiedType receiverType; + ID receiverId; + if (getMethodReceiver(&receiverType, &receiverId) && receiverType.type()) { + std::vector actuals; + actuals.push_back(CallInfoActual(receiverType, USTR("this"))); + auto ci = CallInfo(/* name */ ident->name(), + /* calledType */ QualifiedType(), + /* isMethodCall */ true, + /* hasQuestionArg */ false, + /* isParenless */ true, actuals); + auto inScope = scopeStack.back(); + auto inScopes = CallScopeInfo::forNormalCall(inScope, poiScope); + auto c = resolveGeneratedCall(context, ident, ci, inScopes); + // Ensure we error out for redeclarations within the method itself, + // which resolution with an implicit 'this' currently does not catch. + if (parsing::idContainsFieldWithName( + context, typedSignature->untyped()->id(), ident->name())) { + context->error(ident, "parenless proc redeclares the field '%s'", + ident->name().c_str()); + } else { + // Save result if successful + if (handleResolvedCallWithoutError(result, ident, ci, c) && + emitLookupErrors) { + issueErrorForFailedCallResolution(ident, ci, c); + } + } + } else { + // Can't establish the type. If this is in a function + // call, we'll establish it later anyway. + result.setType(QualifiedType()); + } } else { // vec.size() == 1 and vec[0].numIds() <= 1 const IdAndFlags& idv = vec[0].firstIdAndFlags(); From 2b167d103f7445510c99c29c83a507a2aef1ddb6 Mon Sep 17 00:00:00 2001 From: Anna Rift Date: Thu, 27 Jun 2024 11:12:03 -0700 Subject: [PATCH 17/19] Replace use of idContainsFieldWithName with lookupInScope Signed-off-by: Anna Rift --- frontend/lib/resolution/Resolver.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/frontend/lib/resolution/Resolver.cpp b/frontend/lib/resolution/Resolver.cpp index 6ff3d7b65240..16daa2de52f7 100644 --- a/frontend/lib/resolution/Resolver.cpp +++ b/frontend/lib/resolution/Resolver.cpp @@ -2970,8 +2970,10 @@ void Resolver::resolveIdentifier(const Identifier* ident, auto c = resolveGeneratedCall(context, ident, ci, inScopes); // Ensure we error out for redeclarations within the method itself, // which resolution with an implicit 'this' currently does not catch. - if (parsing::idContainsFieldWithName( - context, typedSignature->untyped()->id(), ident->name())) { + std::vector redeclarations; + inScope->lookupInScope(ident->name(), redeclarations, IdAndFlags::Flags(), + IdAndFlags::FlagSet()); + if (!redeclarations.empty()) { context->error(ident, "parenless proc redeclares the field '%s'", ident->name().c_str()); } else { From 74ebe06876ac357c25e4a13bdf81e004ecb93bd9 Mon Sep 17 00:00:00 2001 From: Anna Rift Date: Thu, 27 Jun 2024 11:12:14 -0700 Subject: [PATCH 18/19] Undo modifications to idContainsFieldWithName Signed-off-by: Anna Rift --- .../include/chpl/parsing/parsing-queries.h | 7 ++-- frontend/lib/parsing/parsing-queries.cpp | 32 ++++++++----------- 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/frontend/include/chpl/parsing/parsing-queries.h b/frontend/include/chpl/parsing/parsing-queries.h index 4759257bf2ea..eab09da7b855 100644 --- a/frontend/include/chpl/parsing/parsing-queries.h +++ b/frontend/include/chpl/parsing/parsing-queries.h @@ -478,11 +478,10 @@ bool idIsFunctionWithWhere(Context* context, ID id); ID idToContainingMultiDeclId(Context* context, ID id); /** - Given an ID for an aggregate decl or function, - returns 'true' if the passed name is the name of a field/variable contained in - it. + Given an ID for a Record/Union/Class Decl, + returns 'true' if the passed name is the name of a field contained in it. */ -bool idContainsFieldWithName(Context* context, ID declId, +bool idContainsFieldWithName(Context* context, ID typeDeclId, UniqueString fieldName); /** diff --git a/frontend/lib/parsing/parsing-queries.cpp b/frontend/lib/parsing/parsing-queries.cpp index 8e4b8b2db023..ae33f5387388 100644 --- a/frontend/lib/parsing/parsing-queries.cpp +++ b/frontend/lib/parsing/parsing-queries.cpp @@ -1309,27 +1309,21 @@ static bool helpFieldNameCheck(const AstNode* ast, return false; } -static const bool& idContainsFieldWithNameQuery(Context* context, ID declId, - UniqueString fieldName) { - QUERY_BEGIN(idContainsFieldWithNameQuery, context, declId, fieldName); +static const bool& +idContainsFieldWithNameQuery(Context* context, ID typeDeclId, UniqueString fieldName) { + QUERY_BEGIN(idContainsFieldWithNameQuery, context, typeDeclId, fieldName); bool result = false; - if (auto ast = parsing::idToAst(context, declId)) { - AstList dummy; - AstListIteratorPair potentialDecls(dummy.cbegin(), dummy.cend()); - if (auto ad = ast->toAggregateDecl()) { - potentialDecls = ad->children(); - } else if (auto fn = ast->toFunction()) { - potentialDecls = fn->stmts(); - } else { - // Empty list, nothing to check - } + auto ast = parsing::idToAst(context, typeDeclId); + if (ast && ast->isAggregateDecl()) { + auto ad = ast->toAggregateDecl(); - for (auto potentialDecl : potentialDecls) { + for (auto child: ad->children()) { // Ignore everything other than VarLikeDecl, MultiDecl, TupleDecl - if (potentialDecl->isVarLikeDecl() || potentialDecl->isMultiDecl() || - potentialDecl->isTupleDecl()) { - bool found = helpFieldNameCheck(potentialDecl, fieldName); + if (child->isVarLikeDecl() || + child->isMultiDecl() || + child->isTupleDecl()) { + bool found = helpFieldNameCheck(child, fieldName); if (found) { result = true; break; @@ -1341,8 +1335,8 @@ static const bool& idContainsFieldWithNameQuery(Context* context, ID declId, return QUERY_END(result); } -bool idContainsFieldWithName(Context* context, ID declId, UniqueString fieldName) { - return idContainsFieldWithNameQuery(context, declId, fieldName); +bool idContainsFieldWithName(Context* context, ID typeDeclId, UniqueString fieldName) { + return idContainsFieldWithNameQuery(context, typeDeclId, fieldName); } static bool helpFindFieldId(const AstNode* ast, From dc83fe6584b133f80095d4133e25162729b6055c Mon Sep 17 00:00:00 2001 From: Anna Rift Date: Thu, 27 Jun 2024 12:25:03 -0700 Subject: [PATCH 19/19] Add TODO to check for redeclarations of formal names Signed-off-by: Anna Rift --- frontend/lib/resolution/Resolver.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/frontend/lib/resolution/Resolver.cpp b/frontend/lib/resolution/Resolver.cpp index 16daa2de52f7..bc87a7df9b1d 100644 --- a/frontend/lib/resolution/Resolver.cpp +++ b/frontend/lib/resolution/Resolver.cpp @@ -2970,6 +2970,7 @@ void Resolver::resolveIdentifier(const Identifier* ident, auto c = resolveGeneratedCall(context, ident, ci, inScopes); // Ensure we error out for redeclarations within the method itself, // which resolution with an implicit 'this' currently does not catch. + // TODO: Also catch redeclarations of formal names. std::vector redeclarations; inScope->lookupInScope(ident->name(), redeclarations, IdAndFlags::Flags(), IdAndFlags::FlagSet());