-
Notifications
You must be signed in to change notification settings - Fork 342
Initial dyn
keyword support & -std <std-revision>
compiler option
#7172
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Initial dyn
keyword support & -std <std-revision>
compiler option
#7172
Conversation
…aces missing the keyword
Implement sp024 lang 2026
/format |
🌈 Formatted, please merge the changes from this PR |
🌈 Formatted, please merge the changes from this PR |
…support Format code for PR shader-slang#7172
/regenerate-cmdline-ref |
🌈 Regenerated command line reference, please merge the changes from this PR |
dyn
keyword supportdyn
keyword support & -lang 2026
dyn
keyword support & -lang 2026
dyn
keyword support & -lang 2026
compiler option
…ement-SP024-dyn-support Regenerate command line reference for PR shader-slang#7172
include/slang.h
Outdated
@@ -765,6 +765,7 @@ typedef uint32_t SlangSizeT; | |||
SLANG_SOURCE_LANGUAGE_SPIRV, | |||
SLANG_SOURCE_LANGUAGE_METAL, | |||
SLANG_SOURCE_LANGUAGE_WGSL, | |||
SLANG_SOURCE_LANGUAGE_SLANG_2026, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we remove this? use a new option to track language version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should be translating -lang 2026
to -lang slang -version 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-version
is already a reserved keyword. We could make the option named something else like -std 2026
or something ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, -std
is fine.
include/slang.h
Outdated
@@ -765,6 +765,7 @@ typedef uint32_t SlangSizeT; | |||
SLANG_SOURCE_LANGUAGE_SPIRV, | |||
SLANG_SOURCE_LANGUAGE_METAL, | |||
SLANG_SOURCE_LANGUAGE_WGSL, | |||
SLANG_SOURCE_LANGUAGE_SLANG_2026, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, -std
is fine.
source/slang/slang-check-decl.cpp
Outdated
} | ||
else if (auto inheritanceDecl = as<InheritanceDecl>(m)) | ||
{ | ||
auto inheritedInterfaceDeclRefType = as<DeclRefType>(inheritanceDecl->base.type); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
auto inheritedInterfaceDeclRef = isDeclRefTypeOf<InterfaceDecl>(inheritanceDecl->base.type);
source/slang/slang-check-decl.cpp
Outdated
auto interfaceDecl = as<InterfaceDecl>(decl); | ||
if (!interfaceDecl) | ||
return; | ||
validateDynInterfaceUsage(visitor, sink, optionSet, interfaceDecl); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't need to call this from validateDynUsage
, instead validateDynInterfaceUsage
should be called from visitInterfaceDecl
when we check the interface decl itself.
{ | ||
visitDecl(interfaceDecl); | ||
|
||
if (interfaceDecl->hasModifier<AnyValueSizeAttribute>()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check all the dyn
interface restrictions here, also make sure to check that dyn
interface itself cannot be a generic.
source/slang/slang-diagnostic-defs.h
Outdated
@@ -827,6 +827,59 @@ DIAGNOSTIC( | |||
DIAGNOSTIC(33070, Error, expectedFunction, "expected a function, got '$0'") | |||
DIAGNOSTIC(33071, Error, expectedAStringLiteral, "expected a string literal") | |||
|
|||
// `dyn` and `some` errors | |||
DIAGNOSTIC(33071, Error, cannotHaveGenericDynInterface, "dyn interfaces cannot be generic: '$0'.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
33071 is duplicated.
source/slang/slang-check-decl.cpp
Outdated
inheritedInterfaceDecl); | ||
|
||
// `dyn interface` cannot be a generic | ||
auto innerInterfaceDecl = as<InterfaceDecl>(genericDecl->inner); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be moved to visitInterfaceDecl
.
source/slang/slang-check-decl.cpp
Outdated
return; | ||
|
||
// Ensure not inheriting from `dyn` interface | ||
auto inheritedInterfaceDecl = findDynInheritance(visitor, innerAggTypeDecl); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be moved to visitInheritanceDecl
.
source/slang/slang-check-decl.cpp
Outdated
return; | ||
|
||
// cannot have extension provide conformance to dyn interface | ||
auto inheritedInterfaceDecl = findDynInheritance(visitor, extensionDecl); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move to visitInheritanceDecl
.
source/slang/slang-check-decl.cpp
Outdated
@@ -9519,6 +9736,8 @@ void SemanticsDeclBodyVisitor::visitAggTypeDecl(AggTypeDecl* aggTypeDecl) | |||
getSink()->diagnose(aggTypeDecl->loc, Diagnostics::cannotExportIncompleteType, aggTypeDecl); | |||
} | |||
|
|||
validateDynUsage(this, getSink(), getOptionSet(), aggTypeDecl); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we calling validateDynUsage
from so many different places? If we call it here, then we shouldn't need to call it from SemanticsDeclHeaderVisitor::visitGenericDecl
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will reduce the calls to validateDynUsage
to simplify things.
I just thought of this as a way to "organize" validateDynUsage
.
source/slang/slang-check-decl.cpp
Outdated
@@ -10057,6 +10276,8 @@ void SemanticsDeclBasesVisitor::visitExtensionDecl(ExtensionDecl* decl) | |||
|
|||
_validateCrossModuleInheritance(decl, inheritanceDecl); | |||
} | |||
|
|||
validateDynUsage(this, getSink(), getOptionSet(), decl); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think if you move the related checks to visitInheritanceDecl
you shouldn't need to call validateDynUsage
here.
… yet. re-organize the validation as per review, did not fix cmd-line-option yet, that is next.
/format |
🌈 Formatted, please merge the changes from this PR |
…support Format code for PR shader-slang#7172
dyn
keyword support & -lang 2026
compiler optiondyn
keyword support & -std <std-revision>
compiler option
fixes: #7143
fixes: #7146
Goal of PR:
-std <std-revision>
flag.dyn
keyword support in AST. This does not includevarDecl
support since most of these interactions requiresome
keyword support.Future PR(s) goal:
some
keyword in AST. With this we will also implement all varDecl interactions betweendyn
andsome
.some
anddyn
.Breakdown of PR:
validateDyn.*
. This was done so that in the future when we implement more features we will have an easy time removing/adding restrictions todyn
interfaces.Breaking changes:
dyn
interface errors if member list contains one of the following: opaque type, non copyable type, or unsized type.tests\compute\dynamic-dispatch-bindless-texture.slang
is incorrect. This has been fixed.