Skip to content

Commit

Permalink
Diagnose for invalid decl nesting + namespace lookup fixes. (#3397)
Browse files Browse the repository at this point in the history 
* Diagnose for invalid decl nesting.

* Fix.

* Fix.

* Fix.

* Fix `namespace` lookup and `using` resolution.

* fix project files.

* revert project files.

* Enhance namespace syntax, docs.

* Fixes.

---------

Co-authored-by: Yong He <yhe@nvidia.com>
  • Loading branch information
@csyonghe
csyonghe and Yong He authored Dec 12, 2023
1 parent 12fcffa commit ec0224e
Show file tree
Hide file tree
Showing 26 changed files with 866 additions and 144 deletions.
62 changes: 62 additions & 0 deletions docs/user-guide/03-convenience-features.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,68 @@ let a = 5;
a = 6; // Error, `a` is immutable.
```


## Namespaces

You can use the `namespace` syntax to define symbols in a namespace:
```csharp
namespace ns
{
int f();
}
```

Slang also supports the abbreviated syntax for defining nested namespaces:
```csharp
namespace ns1.ns2
{
int f();
}
// equivalent to:
namespace ns1::ns2
{
int f();
}
// equivalent to:
namespace ns1
{
namespace ns2
{
int f();
}
}
```

To access symbols defined in a namespace, you can use its qualified name with namespace prefixes:
```csharp
void test()
{
ns1.ns2.f();
ns1::ns2::f(); // equivalent syntax.
}
```

Symbols defined in the same namespace can access each other without a qualified name, this is true even if the referenced symbol is defined in a different file or module:
```csharp
namespace ns
{
int f();
int g() { f(); } // OK.
}
```

You can also use the `using` keyword to pull symbols defined in a different namespace to
the current scope, removing the requirement for using fully qualified names.
```csharp
namespace ns1.ns2
{
int f();
}

using ns1.ns2;
void test() { f(); } // OK.
```

## Member functions

Slang supports defining member functions in `struct`s. For example, it is allowed to write:
Expand Down
1 change: 1 addition & 0 deletions docs/user-guide/toc.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
<ul class="toc_list">
<li data-link="03-convenience-features#type-inference-in-variable-definitions"><span>Type Inference in Variable Definitions</span></li>
<li data-link="03-convenience-features#immutable-values"><span>Immutable Values</span></li>
<li data-link="03-convenience-features#namespaces"><span>Namespaces</span></li>
<li data-link="03-convenience-features#member-functions"><span>Member functions</span></li>
<li data-link="03-convenience-features#properties"><span>Properties</span></li>
<li data-link="03-convenience-features#initializers"><span>Initializers</span></li>
Expand Down
6 changes: 5 additions & 1 deletion source/slang/slang-ast-iterator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ template <typename CallbackFunc>
void ASTIterator<CallbackFunc>::visitDecl(DeclBase* decl)
{
// Don't look at the decl if it is defined in a different file.
if (!as<ModuleDecl>(decl) && !sourceManager->getHumaneLoc(decl->loc, SourceLocType::Actual)
if (!as<NamespaceDeclBase>(decl) && !sourceManager->getHumaneLoc(decl->loc, SourceLocType::Actual)
.pathInfo.foundPath.getUnownedSlice()
.endsWithCaseInsensitive(fileName))
return;
Expand Down Expand Up @@ -468,6 +468,10 @@ void ASTIterator<CallbackFunc>::visitDecl(DeclBase* decl)
{
visitExpr(extDecl->targetType.exp);
}
else if (auto usingDecl = as<UsingDecl>(decl))
{
visitExpr(usingDecl->arg);
}
if (auto container = as<ContainerDecl>(decl))
{
for (auto member : container->members)
Expand Down
5 changes: 5 additions & 0 deletions source/slang/slang-ast-support-types.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ namespace Slang
void printDiagnosticArg(StringBuilder& sb, QualType const& type);
void printDiagnosticArg(StringBuilder& sb, Val* val);
void printDiagnosticArg(StringBuilder& sb, DeclRefBase* declRefBase);
void printDiagnosticArg(StringBuilder& sb, ASTNodeType nodeType);

struct QualifiedDeclPath
{
Expand Down Expand Up @@ -391,6 +392,10 @@ namespace Slang
///
ModifiersChecked,

/// Wiring up scopes of namespaces with their siblings defined in different
/// files/modules, and other namespaces imported via `using`.
ScopesWired,

/// The type/signature of the declaration has been checked.
///
/// For a value declaration like a variable or function, this means that
Expand Down
160 changes: 111 additions & 49 deletions source/slang/slang-check-decl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,23 @@ namespace Slang
}
};

struct SemanticsDeclScopeWiringVisitor : public SemanticsDeclVisitorBase, public DeclVisitor<SemanticsDeclScopeWiringVisitor>
{
SemanticsDeclScopeWiringVisitor(SemanticsContext const& outer)
: SemanticsDeclVisitorBase(outer)
{}

void visitDeclGroup(DeclGroup*) {}

void visitDecl(Decl*) {}

void visitUsingDecl(UsingDecl* decl);

void visitImplementingDecl(ImplementingDecl* decl);

void visitNamespaceDecl(NamespaceDecl* decl);
};

struct SemanticsDeclAttributesVisitor
: public SemanticsDeclVisitorBase
, public DeclVisitor<SemanticsDeclAttributesVisitor>
Expand Down Expand Up @@ -87,10 +104,6 @@ namespace Slang

void visitIncludeDecl(IncludeDecl* decl);

void visitImplementingDecl(ImplementingDecl* decl);

void visitUsingDecl(UsingDecl* decl);

void visitGenericTypeParamDecl(GenericTypeParamDecl* decl);

void visitGenericValueParamDecl(GenericValueParamDecl* decl);
Expand Down Expand Up @@ -1864,6 +1877,10 @@ namespace Slang
{
ensureDecl(implementingDecl, DeclCheckState::Checked);
}
else if (auto importDecl = as<ImportDecl>(decl))
{
ensureDecl(importDecl, DeclCheckState::Checked);
}
}
};
visitIncludeDecls(moduleDecl);
Expand Down Expand Up @@ -1927,7 +1944,7 @@ namespace Slang
//
DeclCheckState states[] =
{
DeclCheckState::ModifiersChecked,
DeclCheckState::ScopesWired,
DeclCheckState::ReadyForReference,
DeclCheckState::ReadyForLookup,
DeclCheckState::ReadyForConformances,
Expand Down Expand Up @@ -6831,14 +6848,9 @@ namespace Slang
{
// Create a new sub-scope to wire the module
// into our lookup chain.
auto subScope = getASTBuilder()->create<Scope>();
subScope->containerDecl = fileDecl;

subScope->nextSibling = scope->nextSibling;
scope->nextSibling = subScope;
addSiblingScopeForContainerDecl(scope, fileDecl);
}


void SemanticsVisitor::importModuleIntoScope(Scope* scope, ModuleDecl* moduleDecl)
{
// If we've imported this one already, then
Expand All @@ -6859,11 +6871,7 @@ namespace Slang
if (moduleScope->containerDecl != moduleDecl && moduleScope->containerDecl->parentDecl != moduleDecl)
continue;

auto subScope = getASTBuilder()->create<Scope>();
subScope->containerDecl = moduleScope->containerDecl;

subScope->nextSibling = scope->nextSibling;
scope->nextSibling = subScope;
addSiblingScopeForContainerDecl(scope, moduleScope->containerDecl);
}

// Also import any modules from nested `import` declarations
Expand Down Expand Up @@ -7014,7 +7022,7 @@ namespace Slang
getSink()->diagnose(decl->moduleNameAndLoc.loc, Diagnostics::includedFileMissingImplementing, name);
}

void SemanticsDeclHeaderVisitor::visitImplementingDecl(ImplementingDecl* decl)
void SemanticsDeclScopeWiringVisitor::visitImplementingDecl(ImplementingDecl* decl)
{
// Don't need to do anything unless we are in a language server context.
if (!getShared()->isInLanguageServer())
Expand Down Expand Up @@ -7044,10 +7052,8 @@ namespace Slang
if (auto moduleDeclaration = as<ModuleDeclarationDecl>(firstMember))
{
// We are trying to implement a file that defines a module, this is expected.
return;
}

if (auto implementing = as<ImplementingDecl>(firstMember))
else if (auto implementing = as<ImplementingDecl>(firstMember))
{
getSink()->diagnose(decl->moduleNameAndLoc.loc, Diagnostics::implementingMustReferencePrimaryModuleFile);
return;
Expand All @@ -7057,7 +7063,7 @@ namespace Slang
importFileDeclIntoScope(moduleDecl->ownedScope, fileDecl);
}

void SemanticsDeclHeaderVisitor::visitUsingDecl(UsingDecl* decl)
void SemanticsDeclScopeWiringVisitor::visitUsingDecl(UsingDecl* decl)
{
// First, we need to look up whatever the argument of the `using`
// declaration names.
Expand All @@ -7067,45 +7073,98 @@ namespace Slang
// Next, we want to ensure that whatever is being named by `decl->arg`
// is a namespace (or a module, since modules are namespace-like).
//
// TODO: The logic here assumes that we can't have multiple `NamespaceDecl`s
// with the same name in scope, but that assumption is only valid in the
// context of a single module (where we deduplicate `namespace`s during
// parsing). If a user `import`s multiple modules that all have namespaces
// If a user `import`s multiple modules that all have namespaces
// of the same name, it would be possible for `decl->arg` to be overloaded.
// In that case we should really iterate over all the entities that are
// To handle that case, we will iterate over all the entities that are
// named and import any that are namespace-like.
//
NamespaceDeclBase* namespaceDecl = nullptr;
if( auto declRefExpr = as<DeclRefExpr>(decl->arg) )
bool scopesAdded = false;
bool hasValidNamespace = false;

// TODO: consider caching the scope set in NamespaceDecl.
HashSet<ContainerDecl*> addedScopes;
for (auto s = decl->scope; s; s = s->nextSibling)
addedScopes.add(s->containerDecl);

auto addAllSiblingScopesFromDecl = [&](Scope* scope, ContainerDecl* containerDecl)
{
for (auto s = containerDecl->ownedScope; s; s = s->nextSibling)
{
if (addedScopes.add(s->containerDecl))
{
scopesAdded = true;
addSiblingScopeForContainerDecl(scope, s->containerDecl);
}
}
};

if (auto declRefExpr = as<DeclRefExpr>(decl->arg))
{
if (auto namespaceDeclRef = declRefExpr->declRef.as<NamespaceDeclBase>())
{
auto namespaceDecl = namespaceDeclRef.getDecl();
addAllSiblingScopesFromDecl(decl->scope, namespaceDecl);
hasValidNamespace = true;
}
}
else if (auto overloadedExpr = as<OverloadedExpr>(decl->arg))
{
if( auto namespaceDeclRef = declRefExpr->declRef.as<NamespaceDeclBase>() )
for (auto item : overloadedExpr->lookupResult2)
{
namespaceDecl = namespaceDeclRef.getDecl();
if (auto namespaceDeclRef = item.declRef.as<NamespaceDeclBase>())
{
addAllSiblingScopesFromDecl(decl->scope, namespaceDeclRef.getDecl());
hasValidNamespace = true;
}
}
}
if( !namespaceDecl )

if (!scopesAdded)
{
getSink()->diagnose(decl->arg, Diagnostics::expectedANamespace, decl->arg->type);
if (!hasValidNamespace)
getSink()->diagnose(decl->arg, Diagnostics::expectedANamespace, decl->arg->type);
return;
}
}

// Once we have identified the namespace to bring into scope,
// we need to create a new sibling sub-scope to add to the
// lookup scope that was in place when the `using` was parsed.
//
// Subsequent lookup in that scope will walk through our new
// sub-scope and see the namespace.
//
// TODO: If we update the `containerDecl` in a scope to allow
// for a more general `DeclRef`, or even a full `DeclRefExpr`,
// then it would be possible for `using` to apply to more kinds
// of entities than just namespaces.
//
auto scope = decl->scope;
auto subScope = getASTBuilder()->create<Scope>();
subScope->containerDecl = namespaceDecl;
subScope->nextSibling = scope->nextSibling;
scope->nextSibling = subScope;
void SemanticsDeclScopeWiringVisitor::visitNamespaceDecl(NamespaceDecl* decl)
{
// We need to wire up the scope of namespaces with other namespace decls of the same name
// that is accessible from the current context.
auto parent = as<ContainerDecl>(getParentDecl(decl));
if (!parent)
return;
for (auto parentScope = parent->ownedScope; parentScope; parentScope = parentScope->parent)
{
for (auto scope = parentScope; scope; scope = scope->nextSibling)
{
auto container = scope->containerDecl;
auto nsDeclPtr = container->getMemberDictionary().tryGetValue(decl->getName());
if (!nsDeclPtr) continue;
auto nsDecl = *nsDeclPtr;
for (auto ns = nsDecl; ns; ns = ns->nextInContainerWithSameName)
{
if (ns == decl)
continue;
auto otherNamespace = as<NamespaceDeclBase>(ns);
if (!otherNamespace)
continue;

if (!ns->checkState.isBeingChecked())
{
ensureDecl(ns, DeclCheckState::ScopesWired);
}
addSiblingScopeForContainerDecl(decl, otherNamespace);
}
}
// For file decls, we need to continue searching up in the parent module scope.
if (!as<FileDecl>(parentScope->containerDecl))
break;
}
for (auto usingDecl : decl->getMembersOfType<UsingDecl>())
{
ensureDecl(usingDecl, DeclCheckState::ScopesWired);
}
}

/// Get a reference to the candidate extension list for `typeDecl` in the given dictionary
Expand Down Expand Up @@ -7470,6 +7529,9 @@ namespace Slang
case DeclCheckState::ModifiersChecked:
SemanticsDeclModifiersVisitor(shared).dispatch(decl);
break;
case DeclCheckState::ScopesWired:
SemanticsDeclScopeWiringVisitor(shared).dispatch(decl);
break;

case DeclCheckState::SignatureChecked:
SemanticsDeclHeaderVisitor(shared).dispatch(decl);
Expand Down
Loading

0 comments on commit ec0224e

Please sign in to comment.