| Commit message (Collapse) | Author | Age |
|---|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
(#8603)
This change achieves link-time type resolution with a different
mechanism.
For `extern struct Foo : IFoo = FooImpl;`,
instead of synthesizing a wrapper type `Foo` that has a `FooImpl inner`
field and dispatches all interface method calls to `inner.method()`,
this PR completely removes this synthesis step, and instead just lower
such `extern`/`export` types as `IRSymbolAlias` instructions that is
just a reference to the type being wrapped.
Then we extend the linker logic to clone the referenced symbol instead
of the SymbolAlias insts itself during linking.
By doing so, we greatly simply the logic need to support link-time
types, and achieves higher robustness by not having to deal with many
AST synthesis scenarios.
Closes #8554.
---------
Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Note that while this change touched a large numer of files, there are no
changes to functionality being made here. The only things being done are
renaming various symbols and, in a few cases, updating or adding
comments for consistency with the new names.
The core of the naming changes are:
* Most things named to refer to `OutType` (e.g., `IROutType`,
`IRBuilder::getOutType()`, etc.) have been consistently renamed to refer
to `OutParamType`, to emphasize that the relevant AST/IR node types are
only intended for use to represent `out` parameters.
* The same change as described above for `OutType` is also made for
`RefType`, which becomes `RefParamType` in most cases. One mess that
this exposes is the way that the `ExplicitRef<T>` type in the core
module currently lowers to `IRRefParamType`. This change sticks to the
rule of not making functional changes, so that mess is left as-is for
now.
* Names referring to `InOutType` have been changed to instead refer to
`BorrowInOutType`. The intention with this naming change is to emphasize
that the Slang rules for `inout` are semantically those of a borrow (or
at least our interpretation of what a borrow means).
* Names referring to `ConstRefType` have been changed to instead refer
to `BorrowInType`. This change starts work on clarifying that the
existing `__constref` modifier was never intended to be a read-only
analogue of `__ref`, and instead is the input-only analogue of `inout`.
* The `ParameterDirection` enum type has been changed to
`ParamPassingMode`, to reflect the fact that the concept of "direction"
fails to capture what is actually being encoded, particularly once we
have modes beyond simple `in`/`out`/`inout`.
While this change does not alter behavior in any case (the user-exposed
Slang language is unchanged), it is intended to set up subsequence
changes that will work to make the handling of these types in the
compiler more nuanced and correct. Breaking this part of the change out
separately is primarily motivated by a desire to minimize the effort for
reviewers.
---------
Co-authored-by: slangbot <186143334+slangbot@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.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
* Initial support for immutable lambda expressions.
* More diagnostics, and langauge server fix.
* Language server fix.
* Fix bug identified in review.
* Add expected result.
* Update expected result.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* A new approach to AST serialization
This change completely overhauls the way that AST nodes are being serialized, and the offline source-code generation steps that enable that serialization.
In practice, this ends up being a complete overhaul of the way that *modules* are being serialized (not just the AST part), although things like the serialization format for the Slang IR and for source locations are not affected.
The rest of this commit message is broken down in to sections, in an attempt to help guide anybody looking at the code in how to make sense of all the changes.
The Old C++ Extractor
---------------------
AST serialization used to be driven by information scraped using the `slang-cpp-extractor` tool, which did an ad hoc parse of the C++ declarations of the AST node types and then generated a set of "X macros" that could be for macro-based code generation within the rest of the compiler.
While the existing approach was functional, it wasn't easy to understand or maintain, and it has been getting in the way of forward progress on other features we'd like to work on in the language and compiler.
This change removes the `slang-cpp-extractor` tool entirely.
Marking Up the AST Declarations
-------------------------------
The most notable change that contributors to the compiler may notice is the large number of invocations of a macro `FIDDLE()` on the declarations of the AST node types.
The basic idea is that only declarations (namespaces, types, fields) that are preceded by `FIDDLE()` are visible to the code generator tool.
So if somebody is working with the AST and wondering why a new node type isn't working, or why a field they added isn't being serialized correctly, it is probably because they need to add `FIDDLE()` in front of it.
Generating the Boilerplate Code
-------------------------------
The file `slang-ast-boilerplate.cpp` provides a good example of how the information extracted from the marked-up AST declarations gets used.
In that file, the `FIDDLE TEMPLATE` construct is used to generate type information for each of the AST node types.
Similar logic is used in `slang-ast-forward-declarations.h` to generate the declaration of the `ASTNodeType` enumeration, and forward-declare all the AST node classes.
For many parts of the code, simply including that file replaces the need for the old `slang-generated-*.h` files.
Replacing Visitors and Related Logic
------------------------------------
The old visitor types for the AST used the macros that were generated by `slang-cpp-extractor`, so something new was needed to replace them.
The same goes for the `SLANG_AST_NODE_VIRTUAL_CALL` macros.
The core of the solution implemented here is in `slang-ast-dispatch.h`.
Given a "dispatchable" AST node type (say, `Expr`), a call like:
```
ASTNodeDispatcher<Expr,R>(expr, [&](auto e) { return doSomething(e); })
```
is an expression of type `R`, which does the equivalent of something like:
```
switch(expr->getTag())
{
case ASTNodeType::VarExpr: return doSomething(static_cast<VarExpr*>(expr));
// ...
}
```
The `SLANG_AST_NODE_VIRTUAL_CALL` macro is now implemented in terms of `ASTNodeDispatcher`.
The implementation of the visitor types is more involved.
The code in this change retains some of the macro names from the original version, just to try and make the parallels more clear.
The visitor types are all implemented on top of the `ASTNodeDispatcher` approach, and use `FIDDLE TEMPLATE` to generate all the boilerplate `visit*()` method declarations.
Refactoring of `Linkage` Module Loading
---------------------------------------
Needing to revisit all the places where modules get deserialized made it clear that there is a lot of complexity and apparent duplication in the core routines on the `Linkage` that get used for loading modules.
This change tries to clean up some of that logic, but it is worth noting that there are two legacy features that get in the way of making things as clean as they should be:
* The `LoadedModuleDictionary` type that gets passed around a lot exists entirely to handle the corner case where somebody uses the Slang API to perform a compilation with multiple `TranslationUnitRequest`s in the same `FrontEndCompileRequest`, and one of the translation units `import`s the module defined by another of the translation units.
* There are a lot of special-case behaviors and routines entirely there to support the `ModuleLibrary` feature, although that feature should be considered deprecated (or at least subject to getting entirely re-designed down the line).
The basic idea of the cleanup is that all of the (non-deprecated) ways load a module from a serialized binary, or compile one from source should now bottleneck through `loadModuleImpl`, which then bifurcates into `loadSourceModuleImpl` for the compilation case and `loadBinaryModuleImpl` for the deserialization case.
High-Level Serialization Approach
---------------------------------
The old serialization logic used the [RIFF](https://en.wikipedia.org/wiki/Resource_Interchange_File_Format) format to encode the high-level structure of things, and this change retains that usage (and actually doubles down on the RIFF usage).
The old serialization system relied on the idea that for any given type `Foo` that wants to support serialization, there should be something like a `SerialFooData` type in C++, that can represent the state of a `Foo`, and then the actual serialization applied to that `SerialFooData`. This means that in most cases there are four pieces of code written:
* During serialization:
* Copying the data of a `Foo` in memory over to a `SerialFooData` in memory
* Writing the state of a `SerialFooData` into the serialized data stream
* During deserialization:
* Reading the state of a `SerialFooData` from a serialized data stream
* Copying the data of the `SerialFooData` in memory over to a `Foo`
The new logic gets rid of the intermediate `SerialFooData`.
In the serialization direction, we take a `Foo` and write it to the `RIFFContainer` directly, or using some other utilities layered on top of it.
In the deserialization direction, we have additional flexibility. Given a `RIFFContainer::Chunk*` that represents a serialized `Foo`, we often navigate through the in-memory representation of the RIFF data to get to the parts of the serialized value that we actually want/need, without needing to deserialize the entire `Foo`.
To support this kind of operation, this change introduces a few helper types like `ContainerChunkRef` an `ModuleChunkRef`, that are little more than typed wrappers around a `RIFFContainer::Chunk*`.
The Module "Container" Part
---------------------------
A serialized `Module` is encoded as a RIFF chunk, using logic in `slang-serialize-container.cpp` - both before and after this change.
This change reorganizes a lot of the code in that file, to account for the way that eliminating the intermediate `SerialContainerData` type streamlines the overall task of writing out the parts of the module.
In the deserialization logic... there isn't really much to do in `slang-serialize-container.cpp`. Most of the logic in `slang.cpp` and `slang-module-library.cpp` that pertains to deserializing modules uses the `ModuleChunkRef`-based approach, and simply extracts the pieces of the serialized module that it needs.
The Actual Serialization of the AST
-----------------------------------
The actual AST serialization logic is in `slang-serialize-ast.cpp`.
The basic approach in both the writing and reading directions is:
* Use the `FIDDLE TEMPLATE` system to generate a set of functions, one for each AST node type, that recursively invoke the read/write logic on each field of that node (after recursively invoking the case for its direct superclass)
* Use the `ASTNodeDispatcher` system to dispatch out to those functions whene reading or writing anything derived from `NodeBase`
* For now, handle all types *not* derived from `NodeBase` by hand.
There's a lot of room for improvement around that last item: it should be just as easy to generate the serialization and deserialization logic for other types that don't inherit from `NodeBase`, but the current change tries to err on the side of making the logic as explicit and simplistic as possible, rather than trying to get too clever too soon.
The actual serialization *format* used for the AST is almost comically simplistic: the code uses hierarchical RIFF chunks to emulate a JSON-like structure. This is a very wasteful representation (e.g., a `bool` or a null pointer each take up *8 bytes*), but the goal for now is to start with the simplest thing that could possibly work, and only add more cleverness once we are sure it won't get in the way of important future improvements (like lazy/on-demand deserialization or IR and AST, to improve compiler startup times).
The files `slang-serialize.{h,cpp}` have been co-opted to define a new pair of types `Encoder` and `Decoder` that are used for a more-or-less stream-oriented way or reading or writing RIFF chunks for the JSON-like structure.
Almost everything related to the actual AST serialization could do with a cleanup pass, and some time spent on picking good/better names for everything.
Smaller Stuff
-------------
* Cleaned up a lot of code that was using bare `ASTNodeType` or the extractor's `ReflectClassInfo` type to consistently use `SyntaxClass`.
* Fixed an apparent bug in how the destination-driven code genarator was handling `TryExpr`s
* Fixed an apparent bug in how the GLSL legalization pass was handling translation of certain `SV_*` semantics.
* format code
* fixup: template errors caught by non-VS compilers
* format code
* fixup: more template errors
* fixup: more stuff VS didn't catch
* fixup: it's amazing VS doesn't catch these...
* fixup: yet more template stuff VS ignores
* fixup: more VS template nonsense
* fixup: unreachable return macro usage
* fixup: more unreacable returns
* fixup: unused parameter
* fixup: strict aliasing
* fixup: allow missing entry point list chunk
* fixup: wasm build script
* fixup: AST changes since this PR was created
---------
Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
Co-authored-by: Yong He <yonghe@outlook.com>
|
| |
|
|
| |
inheritance decl. (#5965)
|
| |
|
|
|
|
|
| |
* format
* Minor test fixes
* enable checking cpp format in ci
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
(#4909)
* More reflection API features.
+ Lookup methods and members (by string) on types
+ Fix issue with looking up non-static members through the scope operator '::'
+ `GenericReflection`: Cast a decl to generic to access unspecialized generic parameter names and constraints
+ `GenericReflection`: Use `getGenericContainer()` from function, variable or type to access the 'nearest' generic parent along with specialization info
+ `GenericReflection::getConcreteType` and `GenericReflection::getConcreteIntVal`: to get the concrete type of a param in the context of the reflection object
+ `GenericReflection::getOuterGenericContainer` to go up one level and get the outer generic declarations (if there are more than one enclosing generic scopes)
+ `DeclReflection::getParent`: go to parent declaration.
+ Change `VariableReflection` to be a `DeclRef` rather than a decl (allows us to return properly substituted types for methods, members, and more)
* Fix Falcor issue
* Initial namespace reflection support
* FIx issue with specializing witness tables
* Add API method for specializing parameters of a generic decl
* Add ability to specialize generic references to functions, types and more
This PR adds the following end-points:
- `specializeGeneric()` method that can be called on a generic reflection to substitute arguments for generic type and value parameters. It returns another generic reflection, but this time with the appropriate substitution.
- `applySpecializations()` method to then copy these specializations onto an existing type or function reflection.
- `isSubType()` to check if a type is a subtype of another type (useful to check if a type is differentiable by checking `IDifferentiable`)
This PR also:
- Adds `DeclReflection::Kind::Namespace` so that namespace containers are correctly reflected when walking the decl-tree. the name can be obtained through `getName()` but there's no need to cast to a namespace (since there's nothing else we can do with a namespace decl)
- Fixes an issue with name-based lookups that fail if a type or function is referenced without specializations. Its helpful to be able to form a reference to a function with default substitutions, so that we can we can specialize it later (either directly, or via argument types).
* Update slang.h
* Fix up naming
* Update slang-compiler.h
* Update slang-reflection-api.cpp
* Update slang.cpp
* Update slang.cpp
* Update slang.cpp
* Use `checkGenericAppWithCheckedArgs` to do specialization
* Update slang-reflection-api.cpp
* Update slang-check-decl.cpp
|
| |
|
|
|
| |
Currently we only allow variable, struct, and function as the target
for the user-defined attribute, this change adds the function parameter
to the target as well.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Support loading serialized modules.
* Fix.
* Fix vs solution files
* Fix glsl module loading.
* C++ fix.
* Fix.
* Try fix c++ error.
* Try fix.
* Fix.
* Fix.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
* Improve capability system.
* Update documentation.
* Tuning semantics.
* LSP: hierarchical diagnostics.
* Fix test.
* Fix test.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* 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>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Initial support for generic interfaces.
* Cleanup.
* Add generic syntax for interfaces.
* Add `IArray`.
* 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>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* 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>
|
| |
|
| |
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>
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
defining custom derivatives (#2947)
* Issue diagnostic for incorrect directionality when defining custom derivative
* Better diagnostics on invalid custom derivatives
* Avoid duplicating `getParameterDirection()`
---------
Co-authored-by: Yong He <yonghe@outlook.com>
|
| |
|
|
|
|
|
|
|
|
|
| |
* Fix intellisense crash.
* Fix a bug in updateElement simplification.
* cleanup.
---------
Co-authored-by: Yong He <yhe@nvidia.com>
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
stdlib. (#2615)
* Allow array parameters in forward diff.
* Use type canonicalization instead of coersion.
* Reimplement array type.
* Fix.
* Update test case.
---------
Co-authored-by: Yong He <yhe@nvidia.com>
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
| |
* Autodiff through simple dynamic dispatch.
* Revert changes.
* Fix.
Co-authored-by: Yong He <yhe@nvidia.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Clean up type checking of higher order expressions.
* Replace `goto` with `break` to pacify clang.
* Fix.
* Fixes.
* Fix more tests.
* Fix lowerWitnessTable parameter error.
* Exclude attributes from ast printing.
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>
|
| |
|
|
|
|
|
| |
* Clean up `IRReturnVoid`.
* Update gitignore.
Co-authored-by: Yong He <yhe@nvidia.com>
|
| |
|
|
|
|
|
|
|
| |
* New language feature: basic error handling.
* Fix.
* Fix `tryCall` encoding according to code review.
Co-authored-by: Yong He <yhe@nvidia.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
If the user has a derived `struct` type:
```hlsl
struct Base { int b = 1; }
struct Derived : Base { int d = 2; }
```
Then it is still reasonable for them to want to use initializer lists when declaring variables using the `Derived` type:
```hlsl
Derived x = {};
Derived y = { 7, 8 };
```
This change implements two missing pieces of functionality in the Slang compiler to allow this case:
* First, when the front-end semantic checks are applied to an initializer list, if the type being initialized is a derived `struct` type it always expects to find initialization arguments for its base type before those for its fields.
* Second, when lowering an initializer-list expression from the AST to the IR, the compiler expects the first argument in the list to be the initial value for the base field (if any). This also applies to default-initialization of fields/variables.
This change slightly entangles front-end logic with the logic for how struct inheritance is lowered to the IR, but the behavior is unlikely to confuse users who expect C++-like layout.
It is worth noting that with this change it should be possible to initialize the base type using either a nested initializer list or flat arguments:
```hlsl
struct BigBase { int x; int y; int z; }
struct BigDerived : BigBase { int w; }
BigDerived a = { {1,2,3}, 4 };
BigDerived b = { 1, 2, 3, 4 };
```
This behavior should Just Work because of the existing C-like rules for initializer lists where an aggregate can be initialized by either a `{}`-enclosed block or distinct values for its leaf fields.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The basic bug here was that `enum` types with an explicit tag type:
enum Color : int32_t { ... }
would have an `InheritanceDecl` implying that `Color` inherits from
`int32_t`. The problem is that this is *not* actually an inheritance
relationship, since a `Color` needs to be explicitly cast to/from an
`int32_t`.
Various parts of the compiler currently treat this case like real
inheritance, and as a result the operations taht would apply to an
`int32_t` end up applying to a `Color` as well. This particularly leads
to an ambiguity between applying the `==` operator, because it has
overloads for both the `__EnumType` and `__Builtin{something}`
interfaces.
The fix here is to explicitly exclude the `InheritanceDecl` that
represents an enumeration tag type when considering declared subtype
relationships. A more complete version of this fix would need to go
through all places in the code where `InheritanceDecl`s are used and
make sure that any places using them for true inheritnace relationships
ignore those that represent an enumeration tag type.
(An alternative option would be to use a distinct kind of `Decl` to
represent the tag-type relationship, perhaps even going so far as to
modifying the type of the relevant AST node as part of semantic
checking)
This change includes a regression test for the way this bug surfaced in
user code.
Co-authored-by: jsmall-nvidia <jsmall@nvidia.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This change originally started with the simple goal of allowing generic functions with default argument values on their parameters to work:
```
void someFunction<T>(T value, int optional = 0);
```
The core problem there was that the compiler code was (correctly) anticipate the case where the default argument value for a parameter depends on a generic parameter, such as:
```
interface IDefaultable { static This getDefault(); }
void anotherFunction<T : IDefaultable>(T first, T second = T.getDefault());
```
Supporting this latter case requires some kind of ability to apply subsitutions to an `Expr`, but our compiler logic simply errored out in that case. The first major fix that went into this change was to add a new `SubstExpr<T>` type that behaves a lot like `DeclRef<T>` in that it stores a `T*` plus a set of substititions that need to be applied to it.
In addition, it was found that even if `anotherFunction<ConcreteType>(...)` might work, when generic argument inference was used for just `anotherFunction(...)` would fail because it includes a strict match on the number of arguments/parameters in the call expression.
The next problem that arose was that the test I'd created used an interace with an `__init` requirement, and it appeared that our code generation didn't work for that case:
```
interface IStuff { __init(int val); }
void f<T : IStuff>(T x = T(0));
```
In this case, the `T(0)` initialization would get compiled to `(ConcreteType) 0` in the output rather than calling the function generated for the `__init` inside `ConcreteType`. The basic problem there was a bit of crufty old logic we have in place to work around the large number of `__init` declarations in the stdlib that don't have proper `__intrinsic_op` modifiers on them. We really need to fix the underlying problem there, but I worked around it by having the IR lowering pass only do its workaround magic on stdlib declarations.
The next problem down this line was that my test had two different `__init` declarations in the concrete type and the logic for checking interface conformance was picking the wrong one to satisfying an interface requirement despite it being obviously wrong (not even the right number of parameter).
This last problem led me down the rabbit-hole of trying to actually get our semantic checking for interface requirements right. There were a few pieces to that work:
* Actually checking that the parameter and result types for two callables match is the simple part. If that was all that would be required we would have implement this logic a long time ago.
* Next we have to deal with functions that make use of the `This` type, associated types, etc. We have to know that when the interface uses `This`, we want to treat that as equivalent to `ConcreteType`, and similarly for associated types. Getting that working is mostly a matter of setting up a this-type subsitution for the interface member being checked.
* Finally, when comparing generic declarations like `IBase::doThing<T>` and `Derived::doThing<U>` we need to deal with the way that `T` and `U` represent the "same" logical type parameter, but are distinct `Decl`s. This is handled by specializing the base declaration to the parameters of the derived one (e.g., forming `IBase::doThing<U>` using the `U` from `Derived::doThing`).
The result seems to be passing our tests, but there are still a few gotchas lurking, I'm sure.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Add a ASTBuilder to a Module
Only construct on valid ASTBuilder (was being called on nullptr on occassion)
* Add nodes to ASTBuilder.
* Compiles with RefPtr removed from AST node types.
* Initialize all AST node pointer variables in headers to nullptr;
* Initialize AST node variables as nullptr.
Make ASTBuilder keep a ref on node types.
Make SyntaxParseCallback returns a NodeBase
* Don't release canonicalType on dtor (managed by ASTBuilder).
* Give ASTBuilders a name and id, to help in debugging.
For now destroy the session TypeCache, to stop it holding things released when the compile request destroys ASTBuilders.
* Moved the TypeCheckingCache over to Linkage from Session.
* NodeBase no longer derived from RefObject.
* Only add/dtor nodes that need destruction.
First pass compile on linux.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Make getSup work with more general non-virtual 'virtual' mechanism.
* WIP: Non virtual AST types.
* Project change.
* Type doesn't implement equalsImpl
* Fix macro invocation
Make Overridden functions public to make simply accessible by base types.
* Use SLANG_UNEXPECTED.
* GetScalarType -> getScalarType
Use SLANG_UNEXPECTED instead on ASSERT in NamedExpressionType and TypeType
|
| |
|
|
|
|
|
|
|
| |
* Small improvements to documentation and code around DiagnosticSink
* Made methods/functions in slang-syntax.h be lowerCamel
Removed some commented out source (was placed elsewhere in code)
* Making AST related methods and function lowerCamel.
Made IsLeftValue -> isLeftValue.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Compiles.
* Small tidy up around session/ASTBuilder.
* Tests are now passing.
* Fix Visual Studio project.
* Fix using new X to use builder when protectedness of Ctor is not enough.
Substitute->substitute
* Add some missing ast nodes created outside of ASTBuilder.
* Compile time check that ASTBuilder is making an AST type.
* Moced findClasInfo and findSyntaxClass (essentially the same thing) to SharedASTBuilder from Session.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Fields from upper to lower case in slang-ast-decl.h
* Lower camel field names in slang-ast-stmt.h
* Fix fields in slang-ast-expr.h
* slang-ast-type.h make fields lowerCamel.
* slang-ast-base.h members functions lowerCamel.
* Method names in slang-ast-type.h to lowerCamel.
* GetCanonicalType -> getCanonicalType
* Substitute -> substitute
* Equals -> equals
ToString -> toString
* ParentDecl -> parentDecl
Members -> members
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Extractor builds without any reference to syntax (as it will be helping to produce this!).
* Change macros to include the super class.
* WIP replacing defs files.
* Added indexOf(const UnownedSubString& in) to UnownedSubString.
Refactored extractor
* Output a macro for each type with the extracted info - can be used during injection in class
* Simplify the header file - as can get super type and last from macro now
* Store the 'origin' of a definition
* Some small tidy ups to the extractor.
* Improve comments on the extractor options.
* Made CPPExtractor own SourceOrigins
* Small fixes around SourceOrigin.
* Small tidy up around macroOrign
* WIP Visitor seems now to work correctly.
Split out types used by ast into slang-ast-support-types.h
* Fix remaining problems with C++ extractor being used with AST nodes.
Add CountOf to extractor type ids.
Added ReflectClassInfo::getInfo to turn an ASTNodeType into a ReflectClassInfo
* Fix compiling on linux.
Fix typo in memset.
* Small tidy up around comments/layout.
Moved NodeBase casting to NodeBase.
* Make premake generate project that builds with cpp-extractor for AST.
* Get the source directory from the filter in premake.
* Fix typo in source path
* Explicitly set the source path for premake generation for AST.
* Special case handling of override to apease Clang.
* Use a more general way to find the slang-ast-reflect.h file to run the extractor.
* Appveyor is not triggering slang-cpp-extractor - try putting dependson together.
* Put building slang-cpp-extractor first.
* Disable some project options to stop MSBuild producing internal compiler errors.
* Try reordering the projects in premake5.lua
* Hack to try and make slang-cpp-extractor built on appveyor.
* Disable flags - not required for MSBuild on appveyor.
* Disable flags not required for build on AppVeyor.
* Updated Visual Studio projects with slang-cpp-extractor.
* Added Visual Studio slang-cpp-extractor project.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Extractor builds without any reference to syntax (as it will be helping to produce this!).
* Change macros to include the super class.
* Added indexOf(const UnownedSubString& in) to UnownedSubString.
Refactored extractor
* Output a macro for each type with the extracted info - can be used during injection in class
* Simplify the header file - as can get super type and last from macro now
* Store the 'origin' of a definition
* Some small tidy ups to the extractor.
* Improve comments on the extractor options.
* Made CPPExtractor own SourceOrigins
* Small fixes around SourceOrigin.
* Small tidy up around macroOrign
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
Currently we fail to diagnose code that calls an instance method from a static method using implicit `this`, and instead crash during lowering of the AST to the IR.
This change introduces a bit more detail to the "this parameter mode" that is computed during lookup, so that it differentiates three cases. The existing two cases of a mutable `this` and immutable `this` remain, but we add a third case where the "this parameter mode" only allows for a reference to the `This` type.
When turning lookup "breadcrumb" information into actual expressions, we respect this setting to construct either a `This` or `this` expression.
In order to actually diagnose the incorrect reference, I had to add code around an existing `TODO` comment that noted how we should diagnose attempts to refer to instance members through a type. Enabling that diagnostic revealed a missing case needed by generics (including those in the stdlib) - a type-constraint member is always referenced statically.
Putting the diagnostic for a static reference to a non-static member in its new bottleneck location meant that some code higher up the call static that handles explicit static member references had to be tweaked to not produce double error messages.
This change includes a new diagnostic test to show that we now give an error message on code that makes this mistake, instead of crashing.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
* * Added MemberFilterStyle - controls action of FilteredMemberList and FilteredMemberRefList
* Splt out template implementations
* Use more standard method names dofr FilteredMemberRefList
* Added reflect-static.slang test
* Added isNotEmpty/isEmpty to filtered lists
* Added ability to index into filtered list (so not require building of array)
* Default MemberFilterStyle to All.
* Remove explicit MemberFilterStyle::All
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This change adds logic for parsing `namespace` declarations, referencing them, and looking up their members.
* The parser changes are a bit subtle, because that is where we deal with the issue of "re-opening" a namespace. We kludge things a bit by re-using an existing `NamespaceDecl` in the same parent if one is available, and thereby ensure that all the members in the same namespace can see on another.
* In order to allow namespaces to be referenced by name they need to have a type so that a `DeclRefExpr` to them can be formed. For this purpose we introduce `NamespaceType` which is the (singleton) type of a reference to a given namespace.
* The new `NamespaceType` case is detected in the `MemberExpr` checking logic and routed to the same logic that `StaticMemberExpr` uses, and the static lookup logic was extended with support for looking up in a namespace (a thin wrapper around one of the existing worker routines in `slang-lookup.cpp`.
* I made `NamespaceDecl` have a shared base class with `ModuleDecl` in the hopes that this would allow us to allow references to modules by name in the future. That hasn't been tested as part of this change.
* I cleaned up a bunch of logic around `ModuleDecl` holding a `Scope` pointer that was being used for some of the more ad hoc lookup routines in the public API. Those have been switched over to something that is a bit more sensible given the language rules and that doesn't rely on keeping state sititng around on the `ModuleDecl`.
* I added a test case to make sure the new funcitonality works, which includes re-opening a namespace, and it also tests both `.` and `::` operations for lookup in a namespace.
* The main missing feature here is the ability to do something like C++ `using`. It would probably be cleanest if we used `import` for this, since we already have that syntax (and having both `import` and `using` seems like a recipe for confusion). Most of the infrastructure is present to support `import`ing one namespace into another (in a way that wouldn't automatically pollute the namespace for clients), but some careful thought needs to be put into how import of namespaces vs. modules should work.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Define compound intrinsic ops in the standard library
The current stdlib code has a notion of "compound" intrinsic ops, which use the `__intrinsic_op` modifier but don't actually map to a single IR instruction.
Instead, most* of these map to multiple IR instructions using hard-coded logic in `slang-ir-lower.cpp`.
(* One special case is `kCompoundOp_Pos` that is used for unary `operator+` and that maps to *zero* IR instructions)
All of the opcodes that used to use the `kCompoundOp_` enumeration values now have definitions directly in the stdlib and use the new `[__unsafeForceInlineEarly]` attribute to ensure that they get inlined into their use sites so that the output code is as close as possible to the original.
For the most part, generating the stdlib definitions for the compound ops is straightforward, but here's some notes:
* The unary `operator+` I just defined directly in Slang code, since it doesn't share much structure with other cases
* The unary increment/decrement ops are generated as a cross product of increment/decrement and prefix/postfix. The logic is a bit messy but given that we have scalar, vector, and matrix versions to deal with it still saves code overall
* Because all the compound/assignment cases got moved out, the existing code for generating unary/binary ops can be simplified a bit
* All the no-op bit-cast operations like `asfloat(float)` are now inline identity functions
* A few other small cleanups are made by not having to worry about the compound ops (which used to be called "pseudo ops") sometimes being encoded in to the same type of value as a real IR opcode.
The one big detail here is a fix for how IR lowering works for `let` declarations: they were previously being `materialize()`d which only guarantees that they've been placed in a contiguous and addressable location, but doesn't actually convert them to an r-value. As a result a `let` declaration could accidentally capture a mutable location by reference, which is definitely *not* what we wanted it to do. Fixing this was needed to make the new postfix `++` definition work (several existing tests end up covering this).
One important forward-looking note:
One subtle (but significant) choice in this change is that we actually reduce the number of declarations in the stdlib, because instead of having the compound operators include both per-type and generic overloads (just listing scalar cases for now):
float operator+=(in out float left, float right) { ... }
int operator+=(in out int left, int right) { ... }
...
T operator+= <T:__BuiltinBlahBlah>(in out T left, T right) { ... }
We now have *only* the single generic version:
T operator+= <T:__BuiltinBlahBlah>(in out T left, T right) { ... }
In running our current tests, this change didn't lead to any regressions (perhaps surprisingly).
Given that we were able to reduce the number of overloads for `operator+=` by a factor of N (where N is the number of built-in types), it seems worth considering whether we could also reduce the number of overloads of `operator+` by the same factor by only having generic rather than per-type versions.
One concern that this forward-looking question raises is whether the quality of diagnostic messages around bad calls to the operators might suffer when there are only generic overloads instead of per-type overloads. In order to feel out this problem I added a test case that includes some bad operator calls both to `+=` (which is now only generic with this change) and `+` (which still has per-type overloads). Overall, I found the quality of the error messages (in terms of the candidates that get listed) isn't perfect for either, but personally I prefer the output in the generic case.
As part of adding that test, I also added some fixups to how overload resolution messages get printed, to make sure the function name is printed in more cases, and also that the candidates print more consistently. These changes affected the expected output for one other diagnostic test.
* fixup: disable bad operator test on non-Windows targets
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The actual definitions that got moved into the stdlib here are pretty few:
* `clip()`
* `cross()`
* `dxx()`, `ddy()` etc.
* `degrees()`
* `distance()`
* `dot()`
* `faceforward()`
The meat of the change is infrastructure changes required to support these new declarations
* Generic versions of the standard operators (e.g., `operator+`) were added that are generic for a type `T` that implements the matching `__Builtin`-prefixed interface. An open question is whether we can now drop the non-generic versions in favor of just having these generic operators.
* A `__BuiltinLogicalType` interface was added to capture the commonality between integers and `bool`
* `__BuiltinArithmeticType` was extended so that implementations must support initialization from an `int`
* `__BuiltinFloatingPointType` was extended to require an accessor that returns the value of pi for the given type, and the concrete floating-point types were extended to provide definitions of this value.
* It turns out that our logic for checking if two functions have the same signature (and should thus count as redeclarations/redefinitions) wasn't taking generic constraints into account at all. That was fixed with a stopgap solution that checks if the generic constraints are pairwise identical, but I didn't implement the more "correct" fix that would require canonicalizing the constraints.
* When doing overload resolution and considering potential callees, logic was added so that a non-generic candidate should always be selected over a generic one (generally the Right Thing to do), and also so that a generic candidate with fewer parameters will be selected over one with more (an approximation of the much more complicated rule we'd ideally have).
* The formatting of declarations/overloads for "ambiguous overload" errors was fleshed out a bit to include more context (the "kind" of declaration where appropriate, the return type for function declarations) and to properly space thing when outputting specialization of operator overloads that end with `<` (so that we print `func < <int>(int, int)` instead of just `func <<int,int>(int,int)`).
* The core lookup routines were heavily refactored and reorganized to try to make them bottleneck more effectively so that all paths handle all the nuances of inheritance, extensions, etc.
* Because of the refactoring to lookup logic, the semantic checking logic related to checking if a type conforms to an interface was updated to be driven based on the `Type` that is supposed to be conforming, rather than a `DeclRef` to the type's declaration. This allows it to use the type-based lookup entry point and eliminates one special-case entry point for lookup.
In addition to the various core changes, this change also refactors some of the existing stdlib code to favor writing more things in actual Slang syntax, and less in C++ code that uses `StringBuilder` to construct the Slang syntax. There is a lot more that could be done along those lines, but even pushing this far is showing that the current approach that `slang-generate` takes for how to separate meta-level C++ and Slang code isn't really ideal, so a revamp of the generator code is probably needed before I continue pushing.
One surprising casualty of the refactoring of lookup is that we no longer have the `lookedUpDecls` field in `LookupResult`. That field probably didn't belong there anyway, but the role it served was important. The idea of `lookedUpDecls` was to avoid looking up in the same interface more than once in cases where a type might have a "diamond" inheritance pattern. Removing that field doesn't appear to affect correctness of any of our existing tests, but by adding a specific test for "diamond" inheritance I could see that the refactoring introduced a regression and made looking up a member inherited along multiple paths ambiguous.
Rather than add back `lookedUpDecls` I went for a simpler (but arguably even hackier) solution where when ranking candidates from a `LookupResult` we check for identical `DeclRef`s and arbitrarily favor one over the other. One complication that arises here is that when comparing `DeclRef`s inherited along different paths they might have a `ThisTypeSubstitution` for the same type, but with different subtype witnesses (because different inheritance paths could lead to different transitive subtype witnesses: e.g., `A : B : D` and `A : C : D`).
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* WIP add support for __spirv_version .
* Added IRRequireSPIRVVersionDecoration
* SPIR-V version passed to glslang.
Enable VK wave tests.
Split ExtensionTracker out, so can be cast and used externally to emit.
Added SourceResult.
* Fix warning on Clang.
* Missing hlsl.meta.h
* Refactor communication/parsing of __spirv_version with glslang.
* Fix some debug typos.
Be more precise in handling of substring handling.
* Make glslang forwards and backwards binary compatible.
* Small comment improvements.
* Added slang-spirv-target-info.h/cpp
* Fix for major/minor on gcc.
* Another fix for gcc/clang.
* VS projects include slang-spirv-target-info.h/cpp
* Removed SPIRVTargetInfo
Added SemanticVersion.
Don't bother with passing a target to glslang. Should be separate from 'version'.
* Renamed slang-emit-glsl-extension-tracker.cpp/.h -> slang-glsl-extension-tracker.cpp/.h
Fixed some VS project issues.
* Fix a comment.
* Added slang-semantic-version.cpp/.h
* Added slang-glsl-extension-tracker.cpp/.h
* Added split that can check for input has all been parsed.
* Fix problem on x86 win build.
|
| |
|
|
|
|
| |
* WIP add support for __spirv_version .
* Added IRRequireSPIRVVersionDecoration
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
* Constant time dynamic cast.
* Use getClassInfo virtual function.
Fix problem because of instanciation of specializations was in wrong order for clang.
* Improve comments.
* Improve comment.
* Ensure s_first is defined before kClassInfo, to ensure construction ordering.
|
