| Commit message (Collapse) | Author | Age |
|---|
| |
|
|
|
|
|
| |
* Improve lookup performance.
* Cleanup.
* Improve autocompletion latency.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Fix segfault with Ptr<T> extension using 'This' type reference
Set LookupOptions::NoDeref when looking up target type members within
extension declarations to prevent automatic pointer dereferencing that
was causing breadcrumb handling issues and segfaults.
Fixes #7656
Co-authored-by: Yong He <csyonghe@users.noreply.github.com>
* Enhance test for ptr-extension-this-type to verify correctness with interpreter
- Changed function to return sizeof(This) - sizeof(Ptr<int>) + 1
- Modified test to use interpreter with filecheck to verify result equals 1
- Added main() function that calls the extension method and prints result
- Verifies that 'This' correctly refers to Ptr<int> in extension context
Co-authored-by: Yong He <csyonghe@users.noreply.github.com>
---------
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Yong He <csyonghe@users.noreply.github.com>
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Most of what this change does is straightforward: take all the places in the code that used to operate directly on `ContainerDecl::members` and related fields, and instead have them call into a smaller set of accessor methods defined on `ContainerDecl`.
The primary motivation for making this change is that in order to implement on-demand loading of members from serialized AST modules, we need a way to identify and intercept the "demand" for those members.
On-demand loading benefits from having all accesses to the members of a `ContainerDecl` be as narrow as possible.
If a part of the code only need a member at a specific index, it should say so.
If it only needs access to members with a specific name, or a given subclass of `Decl`, then it should say so.
A secondary motivation for this change is that there have recently been several changes that added complexity and special cases by introducing code that operated on (and *mutated*) the member list of a container decl in ways that the existing code had never done before.
Any code that mutates the member list of a `ContainerDecl` needs to be sure to not disrupt the invariants that the lookup acceleration structures currently rely on.
One of the recent changes added a declaration-to-index map to the set of acceleration structures (with different validation/invalidation behavior than the others...) while other recent changes would remove or insert declarations in ways that could change the indices of other declarations in the same container.
It is not clear if any of these pieces of code were aware of the others, and the invariants that might be expected or broken along the way.
This change bottlenecks the vast majority of accesses to the members of a `ContainerDecl` through the following operations:
* Getting a `List` of all of the direct member declarations of a container
* Get the number of direct member declarations, and accessing them by index.
* Looking up the list of direct member declarations with a given name.
* Adding a new direct member declaration to the end of the list.
Some other operations are layered on top of those (e.g., getting a list of all the direct member declarations of a given C++ class).
These layered operations are still centralized on the `ContainerDecl`, with the intention that we *can* change them to be non-layered implementations if we ever need to for performance (e.g., by building a lookup structure for finding member declarations by their type).
The exceptional cases of access/mutation on the direct members of a `ContainerDecl` have also been encapsulated, but rather than expose what would risk appearing like general-purpose accessors (e.g., `removeDecl(d)`, `setDecl(index)`, etc.), these operations have been explicitly named after the specific use case that they serve in the codebase today, to discourage others from using them for more kinds of operations we'd rather not support.
These operations have also been given parameter signatures that match their use cases, to make it so that even somebody determined to abuse them would have to invent suitable arguments out of thin air.
In the case of the declaration-to-index mapping, this change eliminates that acceleration structure, in favor or slightly more complicated (and possibly inefficient, yes) code at the use site.
Over time, it would be good to closely scrutinize each of the use cases that requires more complicated interaction with the members of a `ContainerDecl`, to see whether any of them can be reframed in terms of the more basic operations, or if there is some clean abstraction we can introduce to make operations that mutate the member list feel like... hacky.
|
| |
|
|
|
|
|
|
|
| |
* Fix modifier parsing.
* Fix.
* Fix.
* Fix.
|
| |
|
|
|
|
|
| |
* Use two-stage parsing to disambiguate generic app and comparison.
* Typo fix.
* Update doc.
|
| |
|
|
|
|
|
|
|
| |
* Fix bug: IgnoreInheritance in lookup
When specifying IgnoreInheritance in lookup, it will ignore
all members in the self extension for generic, the reason is
that it doesn't specialize the target type of the extension
decl when comparing with self type, so it will result that every
type is unequal to the target type.
|
| |
|
|
|
|
|
|
|
| |
* Fix cyclic lookups with UnscopedEnums
* Add test with multiple unscoped enums with explicit types
---------
Co-authored-by: Yong He <yonghe@outlook.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Initial implementation of `ResourcePtr<T>`.
* Update docs
* Fix build error.
* Add more discussion.
* Update documentation.
* Update TOC.
* Fix.
* Fix.
* Add test case for custom `getResourceFromBindlessHandle`.
* Add namehint to generated descriptor heap param.
* Fix.
* Fix.
* format code
* Rename to `DescriptorHandle`, and add `T.Handle` alias.
* Fix compiler error.
* Fix.
* Fix build.
* Renames.
* Fix documentation.
* Documentation fix.
---------
Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
|
| |
|
|
|
|
|
| |
* format
* Minor test fixes
* enable checking cpp format in ci
|
| |
|
|
|
| |
(#5415)
This commit changes the word "stdlib" or "standard library" to "core module" in the source code.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
| |
* Tuple swizzling and element access.
* Update proposal status.
* Cleanup.
* Fix merrge error.
* Address review.
|
| | |
|
| |
|
| |
Fixes #4110.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Init expressions for struct members
Following commit handles init expressions of struct's.
The general implementation follows C++ init expression rules for classes & inherited classes.
The logic was implemented after type resolution (`SemanticsDeclAttributesVisitor`):
1. Create a default constructor if missing.
2. Check all member variables (`this` and `super`) for if a member has an init expression, continue to *3* if found.
3. For each constructor, insert a member variable's init expression at the beginning of a constructor. This is to follow how C++ does construction of objects.
Some important notes about implementation:
* We must handle the scenario that there is inheritance. To handle the inheritance information processing `findLevelsOfInheritance` was created.
* If a user manually sets overload rank's of constructor expression's we have no way to assume new default constructor overload ranks.
* address feedback
- moved all scope bound variables into if statment initializers
- added indent
- changed logic for overloadRank to be centered around positive numbers rather than negative
* Inheritance fixes universally & for struct field init
1. reimplemented struct field logic
2. implemented inheritance through calling a "super->init()" inisde a constructor for each "this".
3. implemented support for multi level inheritance (4+) and accessing members without a crash.
* add a way to ignore Forward declared constructors.
* a test and fix for a falcor failiure
the following case was not handled: creating an default Ctor due to a non L-Value struct field. Having an empty Ctor causes a warning.
* remove texture/sampler from test since it will break glsl
* get inheritance info using existing lookup logic
modified Facet lookups to store relative depth rather than arbitrary ::Self or' ::Direct for inheritance (which was 'wong' since depth 2 is not Direct, but was considered a Direct inheritance)
* cleanup unused
* cleanup unused functions and whitespace
* fix compile warning
* clean up, reorder, addressed language server fail
changed logic to safeguard bad code --> no longer breaks language server if code is incomplete.
remove the "semi-ordering" logic because caused a crash (and this code does nothing functionally, just thought it would be nice to add if '0 cost').
Remove rank setting for constructors, in place use an addition to the overload system: "this" expressions have calling priority over "super" expressions.
* undo all inheritance depth checks & code added to the inheritance checking algorithm
Reorder default ctor creation and auto-generation of constructor body.
* Handle same struct types during overload resolution
Changed overload resolution logic to properly handle same struct types; added test to check for multi-param same type function overload.
* remove unused ast object
Used unused object in an incorrect way. This caused the compiler to not flag a warning.
* extension support for default constructors
specialization is not supported with default constructors yet.
* fix bugs
Fix bug in override/overload logic with type comparisons.
used wrong type for ctor list construction
Specialization has not been added yet
* disallow default ctor inside extension
* adjust comment, add new tests
* add explicit types to invoke, use faster default ctor lookup.
* adjust syntax & naming as recomended
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* 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 <yhe@nvidia.com>
|
| |
|
|
|
|
|
|
|
|
|
| |
* Support `include` for pulling file into the current module.
* Add auto-completion, hover info and goto-def support.
* Disable warning for missing `module` declaration for now.
---------
Co-authored-by: Yong He <yhe@nvidia.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Unify Texture types in stdlib into 1 generic type.
* Fixes.
* Fix.
* Fixes.
* Fix reflection.
* Fix binding reflection.
* Add gather intrinsics.
* Fix gather intrinsics.
* Fix texture type toText.
* Fix intrinsic.
* fix cuda intrinsic.
* Fix project files.
* cleanup.
* Fix.
* Fix.
* Fix sampler feedback test.
* Fix getDimension intrinsics.
* Fix spirv sample image intrinsics.
* Fix test.
* Fix GLSL intrinsic.
* Cleanup.
---------
Co-authored-by: Yong He <yhe@nvidia.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Add all RayQuery SPIRV Intrinsics.
* Fix
* Fix.
* fix.
* Fix.
* Fix.
* Fix.
---------
Co-authored-by: Yong He <yhe@nvidia.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Redesign DeclRef + Deduplicate Val.
* Update project files
* Fix warning.
* Fix.
* Fix.
* Remove `Val::_equalsImplOverride`.
* Rmove `Val::_getHashCodeOverride`.
* Remove `semanticVisitor` param from `resolve`.
* Cleanups.
---------
Co-authored-by: Yong He <yhe@nvidia.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Simplify lookup.
* Various bug fixes.
* Report type dictionary size in perf benchmark.
* Remove type duplication.
* increase initial dict size.
* Bug fix.
* Fix bugs.
* Fixup.
* Revert type legalization looping.
* Fix specialization pass.
---------
Co-authored-by: Yong He <yhe@nvidia.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Create and cache flattened inheritance lists
The basic change here is to have a cached lookup that can map a `Type`,
or a `DeclRef` that might refer to a type or `extension`, to a list of
the *facets* that comprise it.
The notion of a *facet* here is similar to what the C++ standard calls
"sub-objects".
A declared type like a `struct` has:
* a facet for its own direct members
* one facet for each of its (transitive) base `struct` types
* one facet for each `interface` it conforms to
* one facet for each `extension` that applies to that type
The set of facets for a type is de-duplicated (so that "diamond"
inheritance patterns don't cause issues) and deterministically ordered,
using a variation of the C3 linearization algorithm.
The creation of a linearized list of facets should help the compiler
implementation in two key places:
* Testing if a type implements an interface (or inherits from a base
type) should now only take time linear in the number of (transitive)
bases of that type. We can simply scan the linearized facet list to
see if it contains a facet corresponding to the given base.
* Looking up the members of a type (or a value of a given type) should
be greatly simplified, since all of the members can be found in a
single linear scan of the facet list. In addition, those facets will
be ordered so that facets for "more derived" types will precede those
for "less derived" types, so that shadowing in the case of overrides
should be easier to implement.
This change only implements the first of these two improvements, since
there is already a *lot* of churn involved.
Notes and caveats:
* The handling of conjunction types (e.g., `IFoo & IBar`) complicates
the implementation, both because the simple approach to subtype
testing alluded to above is no longer complete, and also because
we need to be more careful about what forms of subtype witnesses
we construct, so that we can maintain the currently-required invariant
that two witnesses are only equal if they have matching structure.
* We don't implement the full/"proper" C3 algorithm here because it has
some failure cases that we'd still like to support. In particular if
we have both `IX : IA, IB` and `IY : IB, IA`, the C3 algorithm says it
is illegal to have `IZ : IX, IY` because the two bases it inherits
from disagree on the relative ordering of `IA` and `IB` in their
own linearizations. Handling such cases may make our implementation
less efficient, and it will also require testing of those corner
caes.
* When it comes time to revamp the implementation of lookup, we will
need to deal with the fact that a single linear list (seemingly)
cannot give us sufficient information to decide which of two members
of the same name should shadow the other, or if there is an ambiguity.
Or rather, it *can* give us that information if we are willing to
accept some very user-unfriendly behavior and simply say that
declarations earlier in the linearization always shadow later
declarations, even if the facets involved are not related by an
inheritance relationship of any kind.
* In order to remove one kind of vicious circularity from the approach,
the linearization that we are computing for `extension` declarations
will not be sufficient for lookups in the body of such an `extension`.
A future change may need to have support for creating and caching
two distinct linearizations for each `extension`: one that is to be
used when that `extension` is pulled into the linearization for a
type that it applies to, and another for when lookup will be performed
in the context of the `extension` itself.
* This change does *not* include the simple expedient of adding a direct
cache for subtype tests to the `SharedSemanticsContext`, although
adding such a cache would be a simple matter.
* This change introduces more deduplication for subtype witnesses,
which should enable more deduplication for other `Val`s (including
`Type`s), but it does not introduce any assumptions that equal
`Val`s or `Type`s must have identical pointer representations.
* Eventually we may find that, similar to the situation with `Type`s,
we will want to have a split between surface-level and canonicalized
versions of other `Val`s, including subtype witnesses.
* Fix clang error.
* remove debugging code.
---------
Co-authored-by: Yong He <yonghe@outlook.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Make DeclRefBase a Val, and DeclRef<T> a helper class.
* Fixes.
* Workaround gcc parser issue.
* Revert NodeOperand change.
* Fix.
* Fix clang incomplete class complains.
* Fix code review.
* Small cleanups and improvements.
---------
Co-authored-by: Yong He <yhe@nvidia.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
(#2958)
* Simplify type of diagnoseImpl
* Show source line for Note diagnostics, opting out of this where appropriate
* Make declared after use diagnostic clearer
* Fix erroneous error claiming variable is being used before its declaration
Closes https://github.com/shader-slang/slang/issues/2936
* Fix build on msvc
---------
Co-authored-by: jsmall-nvidia <jsmall@nvidia.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Bottleneck DeclRef creation through ASTBuilder.
* Fix clang error.
* Fix.
* Fix.
* More fix.
* Rebase on top of tree.
---------
Co-authored-by: Yong He <yhe@nvidia.com>
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* #include an absolute path didn't work - because paths were taken to always be relative.
* WIP lowerCamel Dictionary.
* WIP more lowerCamel fixes for Dictionary.
* Add/Remove/Clear
* GetValue/Contains
* Fix tabs in dictionary.
Count -> getCount
* Fix fields with caps.
* Key -> key
Value -> value
Use m_ for members where appropriate.
Use lowerCamel in linked list.
* Some small fixes/improvements to Dictionary.
* Kick CI.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
`[DerivativeMember(DiffType.field)]` (#2460)
* wip: remove auto-diff for member access, add diff through property accessors.
* Fix getter-setter test.
* Fix getter-setter-multi test.
* Fix nested-jvp test.
* Use [DerivativeMember] attribute to differentiate through member access.
* Clean up.
* More cleanup.
Co-authored-by: Yong He <yhe@nvidia.com>
|
| |
|
|
|
|
|
| |
* Fix missing implementations in ConjunctionSubtypeWitness.
* Fix.
Co-authored-by: Yong He <yhe@nvidia.com>
|
| |
|
|
|
|
|
|
|
|
|
| |
* wip: dedup AST type nodes and cache lookup.
* Fix.
* Remove profiling.
* Fixes.
Co-authored-by: Yong He <yhe@nvidia.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Language server pointer type support.
+ Natvis for AST.
* Add completion suggestion for GUID.
* Make executable test able to use slang-rt.
* Fix gcc argument for rpath.
* Fix DLLImport on linux.
* Fix windows.
* Fix.
Co-authored-by: Yong He <yhe@nvidia.com>
|
| |
|
|
|
|
|
|
|
| |
* Implicit pointer dereference when using member operator.
* Add expected test result
* Fix lookup.
Co-authored-by: Yong He <yhe@nvidia.com>
|
| | |
|
| |
|
|
|
|
|
| |
* #include an absolute path didn't work - because paths were taken to always be relative.
* Fix for overloaded name lookup.
* Small improvements.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Fixes #1990
The underlying problem here is in the `ExtractExistentialType` AST node class.
An "existential" in current Slang is typically a value of interface type. When such a value is used in an operation, the type-checker "opens" the extistential so that subsequent type-checking steps can work with the (statically unknown) specific type of the value stored inside. The `ExtractExistentialType` AST node represents the type of an existential that has been "opened" in this way.
When the front-end performs lookup "into" a value with one of these types, it nees to use a reference to the original interface declaration with a "this-type substitution" that refers to the "opened" type (a this-type substitution tells the compiler the concrete type it should use in place of `This` in signatures within the interface; it allows compiler to "see" the right associated type definitions to use in a context).
Prior to this change, the implementation would store the specialized reference to the original interface declaration in the `ExtractExistentialType` node as part of its state. The catch there is that the specialized interface reference indirectly refers to the `ExtractExistentialType` AST node itself, creating a circularity. As soon as the front-end performs any operation that tries to recurse over that structure, it would go into an infinite loop.
The fix here sounds kind of like a hack, but seems to be pretty nice in practice. Instead of always storing the specialized interface reference, we instead store the few values that are needed to construct it, and then create and cache the actual reference on-demand. The on-demand created fields are not considered part of the state of the AST node for any kind of recursion or serialization, so they avoid the original problem.
A single test case was added that represents the original bug, and confirms the fix.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Add debug symbols for release build.
* Hack to try and capture failing compilation.
* Typo fix for capture hack.
* Specify return type on lambdas.
* Added const.
* Try breakpoint.
* Up count
* Let's capture everything so we can valgrind.
* Disable always writing repros.
* Make Scope non RefCounted.
* Fix issue with not serializing Scope.
* More comments around changes to Scope.
Remove Scope* from serialization.
* Remove code used for testing original issue.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* #include an absolute path didn't work - because paths were taken to always be relative.
* Split out compiler-core initially with just slang-source-loc.cpp
* More lexer, name, token to compiler-core.
* Split Lexer and Core diagnostics.
* Move slang-file-system to core.
* Add slang-file-system to core.
* More DownstreamCompiler into compiler-core
* Fix typo.
* Add compiler-core to bootstrap proj.
* Small fixes to premake
* For linux try with compiler-core
* Remove compiler-core from examples.
* Added NameConventionUtil to compiler-core
* Add global function to CharUtil to *hopefully* avoid linking issue.
* Hack to make linkage of CharUtil work on linux.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The basic feature here is the ability to use the `&` operator to produce the conjunction/intersection of two interfaces. That is, you can have interfaces:
interface IFirst { int getFirst(); }
interface ISecond { int getSecoond(); }
and if you need a generic function where the type parameter `T` must conform to *both* of these interfaces, you express that by constraining the parameter to the intersection of the interfaces:
void someFunction<T : IFirst & ISecond>(T value) { ... }
Without this feature, the main alternative an application would have is to define an intermediate interface, like:
interface IBoth : IFirst, ISecond {}
Forcing users to deal with an intermediate interface creates more work for type authors (they need to remember to inherit from the right combined interface(s)), or for `extension` authors (when you add `ISecond` to a type that used to just support `IFirst`, you had better also add `IBoth`). In the worst case, a family of N related "leaf" interfaces would give rise to an exponential number of intermediate interfaces to represnt the possible combinations.
A conjunction like `IFirst & ISecond` is officially its own type, and can be used to declare a type alias:
typealias IBoth = IFirst & ISecond;
This change only includes the first pass of work on this feature, so there are several caveats to be aware of:
* Using a conjunction as part of an inheritance clause is not yet supported (e.g., `struct X : IFirst & ISecond`). This is true even if the conjunction was introduced by an intermediate `typealias`
* The `&` syntax introduced here is only parsed in places where only a type (not an expression) is possible. This means you cannot do things like cast to a conjunction with `(IFirst & ISecond)(someValue)`.
* This work *should* apply to conjunctions of more than two interfaces (like `IA & IB & IC`) but that has not yet been tested
* In the long run it may be sensible to allow conjunctions that use concrete types, but we really ought to have the semantic checking logic rule that out for now.
* During testing, I encountered compiler crashes when trying to use this feature together with `property` declarations. Further investigation and debugging is called for.
* The handling of conjunction types is currently incomplete, in that there are many equivalences the compiler does not yet understand. For example, it is clear that `IA & IB` is equivalent to `IB & IA`, but the compiler currently does not understand this and will treat them as different types. A deeper implementation approach is called for.
* Conjunctions are currently only supported for generic type parameter constraints, when performing full specialization. Use of conjunctions for existential-type value parameters or with dynamic dispatch is not yet supported.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The big picture here is that an `extension` can now apply to an interface type and provide convenience methods for all types that implement that interface. Suppose you have an interface for counters:
interface ICounter { [mutating] void add(int val); }
and a type that implements it:
struct SimpleCounter : ICounter { int _state = 0; ... }
If a common operation in your codebase is to increment a counter by adding one, you would be faced with the problem of either:
* Add the `increment()` operation to `ICounter`, and force every implementation to implement the new requirement
* Add the `increment()` operation to concrete counter types as needed, and thus not be able to use it in generic code
* Make `increment()` a global ("free") function, and force clients of counters to have to know which operations use member syntax (`c.add(...)`) and which use global function call syntax (`increment(c)`).
The whole idea of `extension`s is to allow for another option that is better than all of the above:
extension ICounter { [mutating] void increment() { this.add(1); } }
The core of the implementation is relatively straightforward, and consists of two complementary pieces.
The first piece is that when emitting a concrete IR entity (function/type/whatever) we treat any enclosing `interface` type (or `extension` thereof) a bit like an enclosing `GenericDecl`, and introduce an `IRGeneric` to wrap things. The generic `IRGeneric` has parameters representing the `This` type for the interface, along with the witness table that shows how `This` conforms to the interface itself.
We thus end up with an IR version of `increment()` something like:
void increment<This : ICounter>(This this) { this.add(1); }
The second (complementary) fix is that when there is code that references this `increment()` operation, we don't treat it like an interface requirement (look up based on its key), and instead treat it like a generic (since that is how it is lowered now) and speciaize it to the information we can glean from the `ThisTypeSubstitution`.
A related fix that is required here is that within the body of `increment`, when we perform `this.add`, we need to ensure that the lookup of `add` in the base interface properly takes into account the subtype relationship (`This : ICounter`) and encodes it into the lookup result, so that we get `((ICounter) this).add`, and properly generate code that looks up the `add` method in the witness table for `This`.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Clean up the way that lookup "through" a base type is encoded
In order to undestand this change, it is important to undestand how lookup through base interfaces works prior to this change. In order to understand *that* it helps to be reminded of how inheritance relationships get encoded in the AST.
Suppose the user writes:
struct Base { int val; }
struct Derived : Base { ... }
...
Derived d = ...;
int v = d.val;
The question is how an expression like `d.val` gets semantically checked, and how it is encoded into the IR after semantic checking. You might assume it gets checked and encoded so that we end up with:
int v = ((Base) d).val;
and that seems like it should Just Work... so of course that isn't what Slang has been doing. Instead, we relied on the fact that the inheritance relationship `Derived : Base` is represented as an `InheritanceDecl` member of the `Derived` type, and we ended up checking the code into something like:
int v = d.<anonymous>.val;
where `<anonymous>` stands in for the name of the `InheritanceDecl` that represents inheritance from `Base`. This design choice makes a limited amount of sense when you consider how inheritance would typically be lowered to a C-like output language:
// struct Derived : Base { ... }
// =>
struct Derived { Base base; ... }
The problem with that encoding is that it really doesn't make sense for almost any other scenario. In particular, if you have a generic type parameter `T` that was constrianed with `T : ISomething`, then the constraint isn't even technically a *member* of the type parameter `T`, so expressing thing as a member reference in the AST is completely incorrect. Unfortunately, by the time it was clear that we needed something better, a bunch of implementation work was done based on the existing representation.
This change tries to clean things up so that lookup of a super-type member through a value of a sub-type does the obvious thing: cast the value to the super-type and then look up the member (as in `((Base) d).val`).
The core of the change is that in lookup, instead of creating `Constraint` breadcrumbs whenever we are looking up in a super-type (with a reference to the `TypeConstraintDecl` being used) we instead use `SuperType` breadcrumbs (with a reference to a `SubtypeWitness`). Then when we create the expression from a `LookupResultItem`, we translate any `SuperType` breadcrumbs into `CastToSuperTypeExpr`s (an expression type that already existed).
This change also adds support for lookup through the `This` type in the context of an interface, and in order for that to work we need a new kind of subtype witness to represent the knowledge that a `This` type is a subtype of the enclosing interface. Making that work forces us to change the representation of `TransitiveSubtypeWitness` so that it takes a pair of subtype witnesses (and not one subtype witness plus one `TypeConstraintDecl`). For the most part this is a small change, but it raises the possibility that some pieces of the code aren't going to be robust against all possible shapes of subtype witnesses.
The IR lowering logic has relied on the weird `d.<anonymous>` representation in order to ensure that when looking up interface members we weren't always casting to the interface type (which would create a `makeExistential` instruction), and then calling using that. Basically, the IR lowering would ignore the `d.<anonymous>` part and just emit `d`, but we can't do that for `((Base) d)` or `((IThing) d)` because whehter or not we should actually perform the cast depends on context.
For now we solve that problem by adding specific logic to ignore up-casts to interface types when they appear in member expressions or method calls. A more robust solution might be needed down the line, but this seems to work in practice.
All of this work is cleanup that I found was needed in order to make `extension`s of `interface` types workable.
* fixup: disable an incorrect test
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
dispatch) (#1508)
* Allow calling a generic function with an existential value (dynamic dispatch).
* Fixes per review comments.
* Clean up implementation by having `openExistential` return `ExtractExistentialType` instead of a DeclRef to the interface with a `ThisTypeSubstitution`.
* More cleanups
Co-authored-by: Tim Foley <tfoleyNV@users.noreply.github.com>
Co-authored-by: Yong He <yhe@nvidia.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The basic problem here was that in a declaration like:
```hlsl
enum Color : uint { Red, Orange, ... }
```
The `: uint` bit is represented as an `InheritanceDecl`, because that is what we use to represent the syntactic form of inheritance clauses like that. At the point where we parse the `InheritanceDecl` we don't yet know whether it represents a base interface or a "tag type" like `uint` in this case.
The root problem that is then created is: an `enum` type is *not* a subtype of its "tag type," and treating it like a subtype can create problems.
The main problem that arises is that looking in a type like `Color` will find both the members of color *and* the members of `uint`. In the case of things like `__init` declarations, that creates a problem where the `Color` type has two different `__init`s that take a `uint`:
* The one it inherits from `uint` via that `InheritanceDecl` (even though it shouldn't)
* The one it gets via an extension just for conforming to `__EnumType` (a non-user-exposed `interface` in the standard library)
Because both of those `__init`s are inherited, neither is preferred over the other one and they create an ambiguity if somebody tries to write:
```hlsl
uint u = ...;
Colorc = Color(u);
```
The solution used in this PR is to add a compiler-internal modifier to the `InheritanceDecl` that introduces a "tag type" to an `enum`, in an early phase of checking (one of the ones that occurs before it is legal to enumerate the bases of a type). Then the lookup process is modified to ignore `InheritanceDecl`s with that modifier when doing lookup in super-types (since the declaration does *not* indicate a subtype/supertype relationship).
This appears to get the basic feature working again, although it is possible that there are other parts of the compiler that use `InheritanceDecl`s and mistakently assume that all `InheritanceDecl`s introduce subtype/supertype relationships. We probably need to do a significant audit of the code to start being more clear about the nature of the relationships such declarations introduce. Such steps are left to future changes.
Co-authored-by: Yong He <yonghe@outlook.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
During semantic checking, the compiler used to link together `ExtensionDecl`s into a singly-linked list dangling off of the `AggTypeDecl` that they applied to. This approach made lookup relatively easy, because given a `DeclRef` to an `AggTypeDecl` one could easily find and walk the list of candidate extensions.
Unfortunately, the simple approach has two major strikes against it:
* First, as we recently ran into, it creates a lifetime/ownership problem, in cases where the `ExtensionDecl` is outlived by the `AggTypeDecl` it applies to. This creates the one and only place in the compiler today where an "old" AST node might point to a "new" AST node, and it resulted in use-after-free problems in client code.
* Second, the scoping of `extension`s ends up being completely wrong. All of the `extension` methods on a type end up being visible in all cases, instead of just in the context of modules where the `extension` itself is visible. The comparable feature in C# (static extension methods) is careful to not make scoping mistakes like this. The Swift langauge has loose scoping for `extension` more akin to what we have in Slang today, but the maintainers seem to consider it a misfeature.
This change attempts to clean up both issues by changing the way that extension declarations are stored. There are two main pieces:
1. The primary "source of truth" for extension lookup has been moved to the `ModuleDecl`, where a module is responsible for storing a cache of the extensions declared within that module (keyed by the declaration of the type being extended). This cache is updated at the same point where the old code would mutate the AST node being depended on.
2. A secondary aggregated cache is added to the `SharedSemanticsContext` used during semantic checking. This cache includes entries from across multiple modules, and is intended to be invalidated and rebuilt on demand if new modules are added during checking.
Access to the candidate extensions has now been put behind subroutines that require a semantics-checking context to be passed in (there was always one available in contexts that care about extensions).
In addition, the operation for looking up members including those from extensions was refactored heavily to involve internal rather than external iteration and, more importantly, was changed so that it actually tests whether the `ExtensionDecl`s it loops over apply to the type in question, rather than blindly letting extensions members be looked up in ways that don't make sense.
There are three test cases added here to confirm aspects of the fix:
* First, I added a test that reproduces the crash that was being seen, so that we have a regression test for the fix.
* Second, I added a basic semantic-checking test to confirm that an `extension` from an `import`ed module is still visible/usable, to confirm that I didn't break existing valid uses of extensions.
* Third, I added a diagnostic test that ensures that we correctly ignore extensions that should not be visible in a given context as a result of `import` declarations.
Co-authored-by: jsmall-nvidia <jsmall@nvidia.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Initial work on property declarations
Introduction
============
The main feature added here is support for `property` declarations, which provide a nicer experience for working with getter/setter pairs.
If existing code had something like this:
```hlsl
struct Sphere
{
float4 centerAndRadius; // xyz: center, w: radius
float3 getCenter() { return centerAndRadius.xyz; }
void setCenter(float3 newValue) { centerAndRadius.xyz = newValue; }
// similarly for radius...
}
void someFunc(in out Sphere s)
{
float3 c = s.getCenter();
s.setCenter(c + offset);
}
```
It can be expressed instead using a `property` declaration for `center`:
```hlsl
struct Sphere
{
float4 centerAndRadius; // xyz: center, w: radius
property center : float3
{
get { return centerAndRadius.xyz; }
set(newValue) { centerAndRadius.xyz = newValue; }
}
// similarly for radius...
}
void someFunc(in out Sphere s)
{
float3 c = s.center;
s.center = c + offset;
}
```
The benefits at the declaration site aren't that signficiant (e.g., in the example above we actually have slightly more lines of code), but the improvement in code clarity for users is significant.
Having `property` declarations should also make it easier to migrate from a simple field to a property with more complex logic without having to first abstract the use-site code using a getter and setter.
An important future benefit of `property` syntax will be if we allow `interface`s to include `property` requirements, and then also allow those requirements to be satisfied by ordinary fields in concrete types.
Subscripts
----------
The Slang compiler already has limited (stdlib-use-only) support for `__subscript` declarations, which are conceptually similar to `operator[]` from the C++ world, but are expressed in a way that is more in line with `subscript` declarations in Swift. A `SubscriptDecl` in the AST contains zero or more `AccessorDecl`s, which correspond to the `get` and `set` clauses inside the original declaration (there is also a case for a `__ref` accessor, to handle the case where access needs to return a single address/reference that can be atomically mutated).
A major goal of the implementation here is to re-use as much of the infrastructure as possible for `__subscript` declarations when implementing `property` declarations.
Nonmutating Setters
-------------------
One additional thing added in this change is the ability to mark a `set` accessor on either a subscript or a property as `[nonmutating]`, and indeed all of the existing `set` accessors declared in the stdlib have been marked this way.
The need for this modifier is a bit subtle. If we think about a typical subscript or property:
```hlsl
struct MyThing
{
int f;
property p : int
{
get { return f; }
set(newValue) { f = newValue; }
}
}
```
it is clear we want the `set` accessor to translate to output HLSL as something like:
```
void MyThing_p_set(inout MyThing this, int newValue)
{
this.f = newValue;
}
```
Note how the implicit `this` parameter is `inout` even though we didn't mark anything as `[mutating]`. This is the obvious thing a user would expect us to generate given a property declaration.
Now consider a case like the following:
```hlsl
struct MyThing
{
RWStructuredBuffer<int> storage;
property p : int
{
get { return storage[0]; }
set(newValue) { storage[0] = newValue; }
}
}
```
This new declaration doesn't require (or want) an `inout` `this` parameter at all:
```
void MyThing_p_set(MyThing this, int newValue)
{
this.storage[0] = newValue;
}
```
In fact, given the limitations in the current Slang compiler around functions that return resource types (or use them for `inout` parameters), we can only support a `set` operation like this if we can ensure that the `this` parameter is considered to be `in` instead of `inout`. This is exactly the behavior we allow users to opt into with a `[nonmutating] set` declaration.
All of the subscript operations in the stdlib today have `set` accessors that don't actually change the value of `this` that they act on (e.g., storing into a `RWStructuredBuffer` using its `operator[]` doesn't change the value of the `RWStructuredBuffer` variable -- just its contents).
We'd gotten away without this detail so far just because `set` accessors were only being declared in the stdlib and they were all implicitly `[nonmutating]` anyway, so it never surfaced as an issue that the code we generated assumed a setter wouldn't change `this`.
Implementation
==============
Parser and AST
--------------
Adding a new AST node for `PropertyDecl` and the relevant parsing logic was mostly straightforward. The biggest change was allowing a `set` declaration to introduce an explicit name for the parameter that represents the new value to be set.
This change also adds a `[nonmutating]` attribute as a dual to `[mutating]`, for reasons I will get to later.
Semantic Checking
-----------------
The `getTypeForDeclRef` logic was updated to allow references to `property` declarations.
Some of the semantic checking work for subscripts was pulled out into re-usable subroutines to allow it to be shared by `__subscript` and `property` declarations.
The checking of accessor declarations, which sets their result type based on the type of the outer `__subscript` was changed to also handle an outer `property`.
Some special-case logic was added for checking of `set` declarations to make sure that their parameter is given the expected type.
Some logic around deciding whether or not `this` is mutable had to be updated to correctly note that `this` should be mutable by default in a `set` accessor, with an explicit `[nonmutating]` modifier required to opt out of this default. (This is the inverse of how a typical method or `get` accessor works).
IR Lowering
-----------
The good news is that after IR lowering, access to properties turns into ordinary function calls (equivalent to what hand-written getters and setters would produce), so that subsequent compiler steps (including all the target-specific emit logic) doesn't have to care about the new feature.
The bad news is that adding `property` declarations has revealed a few holes in how IR lowering was handling `__subscript` declarations and their accessors, so that it didn't trivially work for the new case as-is.
The IR lowering pass already has the `LoweredValInfo` type that abstractly represents a value that resulted from lowering some AST code to the IR. One of the cases of `LoweredValInfo` was `BoundSubscript` that represented an expression of the form `baseVal[someIndex]` where the AST-level expression referenced a `__subscript` declaration. The key feature of `BoundSubscript` is that it avoided deciding whether to invoke the getter, the setter, or both "too early" and instead tried to only invoke the expected/required operations on-demand.
This change generalizes `BoundSubscript` to handle `property` references as well, so it changes to `BoundStorage`. Making the type handle user-defined property declarations required fixing a bunch of issues:
* When building up argument lists in the IR, we need to know whether an argument corresponds to an `in` or an `out`/`inout` parameter, to decide whether to pass the value directly or a pointer to the value. Some of the logic in the lowering pass had been playing fast and loose with this, so this change tries to make sure that whenever we care computing a list of `IRInst*` that represent the arguments to a call we have the information about the corresponding parameter.
* Similarly, when emitting a call to an accessor in the IR, the information about the expected type of the callee was missing/unavailable, and the code was incorrectly building up the expected type of the callee based on the types of the arguments at the call site. The logic has been changed so that we can extract the expected signature of an accessor (how it will be translated to the IR) using the same logic that is used to produce the actual `IRFunc` for the accessor (so hopefully both will always agree).
* Dealing with `in` vs. `inout` differences around parameters means also dealing with the "fixup" code that is used to assign from the temporary used to pass an `inout` argument back into the actual l-value expression that was used. That logic has all been hoisted out of the expression visitor(s) and into the global scope.
Future Work
===========
The entire approach to handling l-values in the IR lowering pass is broken, and it is in need a of a complete rewrite based on new first-principles design goals. While something like `LoweredValInfo` is decent for abstracting over the easy cases of r-values, addresses, and a few complicated l-value cases like swizzling, it just doesn't scale to highly abstract l-values like we get from `__subcript` and `property` declarations, nor other corner cases of l-values that we need to handle (e.g., passing an `int` to an `inout float` parameter is allowed in HLSL, and performs conversions in both directions!).
It Should be Easy (TM) to extend the logic that tries to synthesize an interface conformance witness method when there isn't an exact match to also support synthesizing a property declaration (plus its accessors) to witness a required property when the type has a field of the same name/type.
* fixup: pedantic template parsing error (thanks, clang!)
* fixup: cleanups and review feedback
* Removed some `#ifdef`'d out code from merge change
* Added proper diagnostics for accessor parameter constraints, which led to some fixes/refactorings
* Added a test case for the accessor-related diagnostics
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The main new feature that works here is that a derived `struct` type can satisfy one or more interface requirements using methods it inherited from a base `struct` type:
```hlsl
interface ICounter { [mutating] void increment(); }
struct CounterBase { int val; [mutating] void increment() { val++; } }
struct ResetableCounter : CounterBase, ICounter
{
[mutating] void reset() { val = 0; }
}
```
Here the derived `ResetableCounter` type is satisfying the `increment()` requirement from `ICounter` using the inherited `CounterBase` method instead of one defined on `ResetableCounter`.
The crux of the problem here was that after lowering to HLSL/GLSL, the above code looks something like:
```hlsl
struct CounterBase { int val; };
void CounterBase_increment(in out CounterBase this) { this.val++; }
struct ResetableCounter { CounterBase base; }
void ResetableCounter_reset(in out ResetableCounter this) { this.base.val = 0; }
```
The central problem is that `CounterBase_increment` here is not type-compatible what we expect to find in the witness table for `ResetableCounter : ICounter`: the `this` parameter has the wrong type!
The basic solution strategy here is to intercept the search for a witness to sastify an interface requirement in `findWitnessForInterfaceRequirement` (those witnesses get collected into a witness table). The revised logic first looks for an exact match, which will only consider members introduced for the type itself, and not those introduced by base types.
If an exact match for a method requirement is not found, the semantic checker then tries to *synthesize* a witness for the requirement, which more or less amounts to generating a function like:
```hlsl
[mutating] void ResetableCounter::synthesized_increment()
{
this.increment();
}
```
The body of that synthesized method will type-check just fine in this case (because it desugars into `this.base.increment()`, more or less), and thus the synthesized method declaration can be used as the actual witness that drives downstream code generation.
Details:
* I added some options to lookup to allow us to explicitly skip member lookup through base interfaces; this should make sure that we don't accidentally satisfy an interface requirement using a member of the same or another interface (since such members are conceptually `abstract`).
* As it originally stood, the semantic checker was allowing `CounterBase.increment()` to satisfy the `increment()` requirement of `ResetableCounter` directly, with the result that we got invalid HLSL/GLSL code as output. In order to avoid this and other bad cases, I made sure that the "exact match" case of requirement satisfaction ignores members that included any "breadcrumbs" in the lookup result item (since the breadcrumbs would all indicate transformations that needed to be applied to `this` to find the right member).
* If we eventually have targets where `this` is passed by pointer/reference in all cases, then all of this work is not needed for the common case of single inheritance, and the base-type method should be usable as a witness directly. I don't see any easy way to handle that special case without producing target-dependent code in the front-end. It might be that we need an IR pass that can detect functions that are trivial "forwarding" functions and replace them with the function they forward to.
* This change includes a test case that should have come along with the original PR that started adding struct inheritance
Caveats:
* The comments in this change talk about things like allowing a method with a default parameter to satisfy a requirement without that parameter. That scenario won't actually work at present because we still have an enormous hack in our logic for checking methods against requirements: we don't actually consider their signatures! I couldn't fold a fix for that issue into this change because there are subtle corner cases around associated types that we need to handle correctly (which were part of the reason why the checking is as hacked as it is)
* This change does *not* try to test or address the case where we want to have a `Derived` type conform to `ISomething` because it inherits from `Base` and `Base : ISomething`. That case has its own details that need to be worked out, but ideally can follow a similar implementation strategy when it comes to re-using methods from `Base` to satisfy requirement on `Derived`.
|
