From 11111e5733b189127dc2c4934d67693b9bc6e764 Mon Sep 17 00:00:00 2001 From: Yong He Date: Wed, 6 Dec 2023 12:05:07 -0800 Subject: Support visibility control and default to `internal`. (#3380) * Support visibility control and default to `internal`. * Fix wip. * Fixes. * Fix. * Fix test. * Add legacy language detection and compatibility for existing code. * Add doc. --------- Co-authored-by: Yong He --- source/slang/slang-check-expr.cpp | 463 ++++++++++++++++++++++++++------------ 1 file changed, 318 insertions(+), 145 deletions(-) (limited to 'source/slang/slang-check-expr.cpp') diff --git a/source/slang/slang-check-expr.cpp b/source/slang/slang-check-expr.cpp index 7edf27b30..dd868f70c 100644 --- a/source/slang/slang-check-expr.cpp +++ b/source/slang/slang-check-expr.cpp @@ -225,6 +225,20 @@ namespace Slang return expr; } + Scope* SemanticsVisitor::getScope(SyntaxNode* node) + { + while (auto declBase = as(node)) + { + if (auto container = as(node)) + { + if (container->ownedScope) + return container->ownedScope; + } + node = declBase->parentDecl; + } + return nullptr; + } + static SourceLoc _getMemberOpLoc(Expr* expr) { if (auto m = as(expr)) @@ -794,6 +808,121 @@ namespace Slang } } + DeclVisibility SemanticsVisitor::getDeclVisibility(Decl* decl) + { + if (as(decl) || as(decl) || as(decl)) + { + auto genericDecl = as(decl->parentDecl); + if (!genericDecl) + return DeclVisibility::Default; + if (genericDecl->inner) + return getDeclVisibility(genericDecl->inner); + return DeclVisibility::Default; + } + if (auto genericDecl = as(decl)) + decl = genericDecl->inner; + for (; decl; decl = getParentDecl(decl)) + { + if (as(decl)) + continue; + if (as(decl)) + continue; + break; + } + if (!decl) + return DeclVisibility::Public; + + for (auto modifier : decl->modifiers) + { + if (as(modifier)) + return DeclVisibility::Public; + else if (as(modifier)) + return DeclVisibility::Internal; + else if (as(modifier)) + return DeclVisibility::Private; + } + + if (auto parentModule = getModuleDecl(decl)) + return parentModule->isInLegacyLanguage ? DeclVisibility::Public : DeclVisibility::Internal; + + return DeclVisibility::Default; + } + + DeclVisibility SemanticsVisitor::getTypeVisibility(Type* type) + { + if (auto declRefType = as(type)) + { + auto v = getDeclVisibility(declRefType->getDeclRef().getDecl()); + auto args = findInnerMostGenericArgs(SubstitutionSet(declRefType->getDeclRef())); + for (auto arg : args) + { + if (auto typeArg = as(arg)) + v = Math::Min(v, getTypeVisibility(typeArg)); + } + return v; + } + return DeclVisibility::Public; + } + + bool SemanticsVisitor::isDeclVisibleFromScope(DeclRef declRef, Scope* scope) + { + auto visibility = getDeclVisibility(declRef.getDecl()); + if (visibility == DeclVisibility::Public) + return true; + if (visibility == DeclVisibility::Internal) + { + // Check that the decl is in the same module as the scope. + auto declModule = getModuleDecl(declRef.getDecl()); + if (declModule == getModuleDecl(scope)) + return true; + } + if (visibility == DeclVisibility::Private) + { + // Check that the decl is in the same or parent container decl as scope. + Decl* parentContainer = declRef.getDecl(); + for (;parentContainer; parentContainer = parentContainer->parentDecl) + { + if (as(parentContainer)) + break; + if (as(parentContainer)) + break; + } + + for (auto s = scope; s; s = s->parent) + { + if (s->containerDecl == parentContainer) + return true; + } + return false; + } + return false; + } + + LookupResult SemanticsVisitor::filterLookupResultByVisibility(const LookupResult& lookupResult) + { + if (!m_outerScope) + return lookupResult; + LookupResult filteredResult; + for (auto item : lookupResult) + { + if (isDeclVisibleFromScope(item.declRef, m_outerScope)) + AddToLookupResult(filteredResult, item); + } + return filteredResult; + } + + LookupResult SemanticsVisitor::filterLookupResultByVisibilityAndDiagnose(const LookupResult& lookupResult, SourceLoc loc, bool& outDiagnosed) + { + outDiagnosed = false; + auto result = filterLookupResultByVisibility(lookupResult); + if (lookupResult.isValid() && !result.isValid()) + { + getSink()->diagnose(loc, Diagnostics::declIsNotVisible, lookupResult.item.declRef); + outDiagnosed = true; + } + return result; + } + LookupResult SemanticsVisitor::resolveOverloadedLookup(LookupResult const& inResult) { // If the result isn't actually overloaded, it is fine as-is @@ -987,6 +1116,7 @@ namespace Slang this, getName("Differential"), type, + nullptr, Slang::LookupMask::type, Slang::LookupOptions::None); @@ -1141,7 +1271,7 @@ namespace Slang // a scope in place. If we do, we will re-use it for any sub-expressions. // If not, we need to create one. // - if(getExprLocalScope()) + if (getExprLocalScope()) { return dispatchExpr(term, *this); } @@ -1860,11 +1990,15 @@ namespace Slang this, operatorName, baseType, + m_outerScope, LookupMask::Default, LookupOptions::NoDeref); + bool diagnosed = false; + lookupResult = filterLookupResultByVisibilityAndDiagnose(lookupResult, subscriptExpr->loc, diagnosed); if (!lookupResult.isValid()) { - getSink()->diagnose(subscriptExpr, Diagnostics::subscriptNonArray, baseType); + if (!diagnosed) + getSink()->diagnose(subscriptExpr, Diagnostics::subscriptNonArray, baseType); return CreateErrorExpr(subscriptExpr); } auto subscriptFuncExpr = createLookupResultExpr( @@ -2333,6 +2467,9 @@ namespace Slang auto lookupResult = lookUp( m_astBuilder, this, expr->name, expr->scope); + + bool diagnosed = false; + lookupResult = filterLookupResultByVisibilityAndDiagnose(lookupResult, expr->loc, diagnosed); if (expr->name == getSession()->getCompletionRequestTokenName()) { @@ -2353,7 +2490,8 @@ namespace Slang expr); } - getSink()->diagnose(expr, Diagnostics::undefinedIdentifier2, expr->name); + if (!diagnosed) + getSink()->diagnose(expr, Diagnostics::undefinedIdentifier2, expr->name); return expr; } @@ -3387,161 +3525,183 @@ namespace Slang Expr* SemanticsVisitor::_lookupStaticMember(DeclRefExpr* expr, Expr* baseExpression) { - auto& baseType = baseExpression->type; + LookupResult globalLookupResult; + bool hasErrors = false; + Expr* base = nullptr; + auto handleLeafCase = [&](DeclRef baseDeclRef, Type* type) + { + auto aggTypeDeclRef = as(baseDeclRef); - // TODO: Need to handle overloaded case (in case we - // have multiple visible types and/or namespaces - // with the same name). + if (auto namespaceDeclRef = as(baseDeclRef)) + { + // We are looking up a namespace member. + // + // This ought to be the easy case, because + // there are no restrictions on whether + // we can reference the declaration here. + // + LookupResult nsLookupResult = lookUpDirectAndTransparentMembers( + m_astBuilder, + this, + expr->name, + namespaceDeclRef.getDecl(), + namespaceDeclRef); + AddToLookupResult(globalLookupResult, nsLookupResult); - if (auto namespaceType = as(baseType)) - { - // We are looking up a namespace member. - // - auto namespaceDeclRef = namespaceType->getDeclRef(); + } + else if (aggTypeDeclRef || type) + { + // We are looking up a member inside a type. + // We want to be careful here because we should only find members + // that are implicitly or explicitly `static`. + // + if (type == nullptr) + type = DeclRefType::create(m_astBuilder, aggTypeDeclRef); - // This ought to be the easy case, because - // there are no restrictions on whether - // we can reference the declaration here. - // - LookupResult lookupResult = lookUpDirectAndTransparentMembers( - m_astBuilder, - this, - expr->name, - namespaceDeclRef.getDecl(), - namespaceDeclRef); - if (!lookupResult.isValid()) - { - return lookupMemberResultFailure(expr, baseType); - } + if (as(type)) + { + return; + } - if (expr->name == getSession()->getCompletionRequestTokenName()) - { - suggestCompletionItems(CompletionSuggestions::ScopeKind::Member, lookupResult); - } - return createLookupResultExpr( - expr->name, - lookupResult, - nullptr, - expr->loc, - expr); - } - else if (auto typeType = as(baseType)) - { - // We are looking up a member inside a type. - // We want to be careful here because we should only find members - // that are implicitly or explicitly `static`. - // - // TODO: this duplicates a *lot* of logic with the case below. - // We need to fix that. - auto type = typeType->getType(); + LookupResult lookupResult = lookUpMember( + m_astBuilder, + this, + expr->name, + type, + m_outerScope); - if (as(type)) - { - return CreateErrorExpr(expr); - } + // We need to confirm that whatever member we + // are trying to refer to is usable via static reference. + // + // TODO: eventually we might allow a non-static + // member to be adapted by turning it into something + // like a closure that takes the missing `this` parameter. + // + // E.g., a static reference to a method could be treated + // as a value with a function type, where the first parameter + // is `type`. + // + // The biggest challenge there is that we'd need to arrange + // to generate "dispatcher" functions that could be used + // to implement that function, in the case where we are + // making a static reference to some kind of polymorphic declaration. + // + // (Also, static references to fields/properties would get even + // harder, because you'd have to know whether a getter/setter/ref-er + // is needed). + // + // For now let's just be expedient and disallow all of that, because + // we can always add it back in later. - LookupResult lookupResult = lookUpMember( - m_astBuilder, - this, - expr->name, - type); - if (!lookupResult.isValid()) - { - return lookupMemberResultFailure(expr, baseType); - } + // If the lookup result is valid, then we want to filter + // it to just those candidates that can be referenced statically, + // and ignore any that would only be allowed as instance members. + // + if (lookupResult.isValid()) + { + // We track both the usable items, and whether or + // not there were any non-static items that need + // to be ignored. + // + bool anyNonStatic = false; + List staticItems; + for (auto item : lookupResult) + { + // Is this item usable as a static member? + if (isUsableAsStaticMember(item)) + { + // If yes, then it will be part of the output. + staticItems.add(item); + } + else + { + // If no, then we might need to output an error. + anyNonStatic = true; + } + } - // We need to confirm that whatever member we - // are trying to refer to is usable via static reference. - // - // TODO: eventually we might allow a non-static - // member to be adapted by turning it into something - // like a closure that takes the missing `this` parameter. - // - // E.g., a static reference to a method could be treated - // as a value with a function type, where the first parameter - // is `type`. - // - // The biggest challenge there is that we'd need to arrange - // to generate "dispatcher" functions that could be used - // to implement that function, in the case where we are - // making a static reference to some kind of polymorphic declaration. - // - // (Also, static references to fields/properties would get even - // harder, because you'd have to know whether a getter/setter/ref-er - // is needed). - // - // For now let's just be expedient and disallow all of that, because - // we can always add it back in later. + // Was there anything non-static in the list? + if (anyNonStatic) + { + // If we had some static items, then that's okay, + // we just want to use our newly-filtered list. + if (staticItems.getCount()) + { + lookupResult.items = staticItems; + lookupResult.item = staticItems[0]; + } + else + { + // Otherwise, it is time to report an error. + getSink()->diagnose( + expr->loc, + Diagnostics::staticRefToNonStaticMember, + type, + expr->name); + hasErrors = true; + return; + } + } + // If there were no non-static items, then the `items` + // array already represents what we'd get by filtering... - // If the lookup result is valid, then we want to filter - // it to just those candidates that can be referenced statically, - // and ignore any that would only be allowed as instance members. - // - if(lookupResult.isValid()) - { - // We track both the usable items, and whether or - // not there were any non-static items that need - // to be ignored. - // - bool anyNonStatic = false; - List staticItems; - for (auto item : lookupResult) - { - // Is this item usable as a static member? - if (isUsableAsStaticMember(item)) - { - // If yes, then it will be part of the output. - staticItems.add(item); - } - else - { - // If no, then we might need to output an error. - anyNonStatic = true; + AddToLookupResult(globalLookupResult, lookupResult); + base = baseExpression; } } + }; - // Was there anything non-static in the list? - if (anyNonStatic) - { - // If we had some static items, then that's okay, - // we just want to use our newly-filtered list. - if (staticItems.getCount()) - { - lookupResult.items = staticItems; - lookupResult.item = staticItems[0]; - } - else - { - // Otherwise, it is time to report an error. - getSink()->diagnose( - expr->loc, - Diagnostics::staticRefToNonStaticMember, - type, - expr->name); - return CreateErrorExpr(expr); - } - } - // If there were no non-static items, then the `items` - // array already represents what we'd get by filtering... - } - if (expr->name == getSession()->getCompletionRequestTokenName()) + auto handleLeafExpr = [&](Expr* e) { - suggestCompletionItems(CompletionSuggestions::ScopeKind::Member, lookupResult); + if (auto nsType = as(e->type)) + handleLeafCase(nsType->getDeclRef(), nsType); + else if (auto aggType = as(e->type)) + handleLeafCase(aggType->getDeclRef(), aggType); + else if (auto typetype = as(e->type)) + handleLeafCase(DeclRef(), typetype->getType()); + }; + + auto& baseType = baseExpression->type; + if (as(baseType)) + { + return CreateErrorExpr(expr); + } + + if (auto overloaded = as(baseExpression)) + { + for (auto candidate : overloaded->lookupResult2.items) + handleLeafCase(candidate.declRef, nullptr); + } + else if (auto overloaded2 = as(baseExpression)) + { + for (auto candidate : overloaded2->candidiateExprs) + { + handleLeafExpr(candidate); } - return createLookupResultExpr( - expr->name, - lookupResult, - baseExpression, - expr->loc, - expr); } - else if (as(baseType)) + else { - return CreateErrorExpr(expr); + handleLeafExpr(baseExpression); + } + + bool diagnosed = false; + globalLookupResult = filterLookupResultByVisibilityAndDiagnose(globalLookupResult, expr->loc, diagnosed); + diagnosed |= hasErrors; + if (!globalLookupResult.isValid()) + { + return lookupMemberResultFailure(expr, baseType, diagnosed); } - // Failure - return lookupMemberResultFailure(expr, baseType); + if (expr->name == getSession()->getCompletionRequestTokenName()) + { + suggestCompletionItems(CompletionSuggestions::ScopeKind::Member, globalLookupResult); + } + return createLookupResultExpr( + expr->name, + globalLookupResult, + base, + expr->loc, + expr); } Expr* SemanticsExprVisitor::visitStaticMemberExpr(StaticMemberExpr* expr) @@ -3565,12 +3725,14 @@ namespace Slang Expr* SemanticsVisitor::lookupMemberResultFailure( DeclRefExpr* expr, - QualType const& baseType) + QualType const& baseType, + bool supressDiagnostic) { // Check it's a member expression SLANG_ASSERT(as(expr) || as(expr)); - getSink()->diagnose(expr, Diagnostics::noMemberOfNameInType, expr->name, baseType); + if (!supressDiagnostic) + getSink()->diagnose(expr, Diagnostics::noMemberOfNameInType, expr->name, baseType); expr->type = QualType(m_astBuilder->getErrorType()); return expr; } @@ -3678,6 +3840,14 @@ namespace Slang { return _lookupStaticMember(expr, expr->baseExpression); } + else if (as(expr->baseExpression)) + { + return _lookupStaticMember(expr, expr->baseExpression); + } + else if (as(expr->baseExpression)) + { + return _lookupStaticMember(expr, expr->baseExpression); + } else if (as(baseType)) { return CreateErrorExpr(expr); @@ -3688,10 +3858,13 @@ namespace Slang m_astBuilder, this, expr->name, - baseType.Ptr()); + baseType.Ptr(), + m_outerScope); + bool diagnosed = false; + lookupResult = filterLookupResultByVisibilityAndDiagnose(lookupResult, expr->loc, diagnosed); if (!lookupResult.isValid()) { - return lookupMemberResultFailure(expr, baseType); + return lookupMemberResultFailure(expr, baseType, diagnosed); } if (expr->name == getSession()->getCompletionRequestTokenName()) { -- cgit v1.2.3