| Commit message (Collapse) | Author | Age |
|---|
| ... | |
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
framework for passes to process them. (#2297)
* Added a decorator to mark functions for forward-mode differentiation
* Fill out support for calls to non-decl values
The existing compiler logic has a few places (semantic checking plus AST-to-IR lowering) where it assumes that function calls (`InvokeExpr`) are only ever made to expressions that resolve to a specific `Decl` (`DeclRefExpr`). This assumption allows semantic checking and lowering code to inspect things like the parameter list of an actual declaration, rather than just the type signature of the callee, and that infrastructure is used to support various features (e.g., default argument values on parameters).
The AST and IR representations themselves have no matching requirement, and the places where the more general case of call expressions would need to be supported were relatively clear in the code. This change attempts to add suitable logic into each of those places.
Note that this change does *not* surface any valid way to form input code that would cause these new code paths to be executed, so it is entirely possible that there are bugs in the logic as written here. The primary goal of this change is simply to get a sketch of the correct code checked in so that we have something to build on once we have language features that will require this support.
* fixup: warnings-as-errors
* Added parser logic for '__jvp(<fn-name>)' operator
* Fixed issue with missing overload candidate item and added basic parsing test for the __jvp syntax
* Added a blank JVP Auto-diff pass and a pass that replaces 'JVPDerivativeOf' calls with the differentiated function
* Added a couple comments
* Added parameter handling for the JVP pass
Co-authored-by: Theresa Foley <tfoley@nvidia.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Major language server features.
* Include slangd in binary release.
* Fix compiler issues.
* Fix compiler error.
* Completion resolve.
* Various improvements.
* Update diagnostic test expected output.
* Bug fix for source locations.
* Adjust diagnostic update frequency.
* Update github actions to store artifacts.
* Fix infinite parser loop.
* Fix parser recovery.
* Fix parser recovery.
* Update test.
* Fix test.
* Disable IR gen for language server.
* Allow commit characters in auto completion.
* Fix lookup for invoke exprs.
* More parser robustness fixes.
* update solution file
Co-authored-by: Yong He <yhe@nvidia.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
An earlier refactoring pass over the compiler codebase split the
type that had been called `CompileRequest` into three distinct
pieces:
* `FrontEndCompileRequest` which was supposed to own state and
options related to running the compiler front end and producing
IR + reflection (e.g., what translation units and source
files/strings are included).
* `BackEndCompileRequest` which was supposed to own state and options
related to running the compiler back end to translate the IR
for a `ComponentType` (program) into output code. (Note that the
`BackEndCompileRequest` was conceived of as orthogonal to the
`TargetRequest`s, which store per-target and target-specific
options.)
* `EndToEndCompileRequest` which was an umbrella object that owns
separate front-end and back-end requests, plus any state that is
only relevant when doing a true end-to-end compile (such as the
kinds of compiles initiated with `slangc`). As originally conceived,
the only state that this type was supposed to own was stuff related
to "pass-through" compilation, as well as state related to writing
of generated code to output files.
That refactoring work was very useful at the time, because it allowed
us to "scrub" the back end compilation steps to remove all
dependencies on front-end and AST state (this was important for our
goals of enabling linking and codegen from serialized Slang IR).
At this point, however, it is clear that the hierarchy that was built
up serves very little purpose:
* The `BackEndCompileRequest` type is only used in two places:
* As part of an `EndToEndCompileRequest`, where the settings on
the `BackEndCompileRequest` can be configured, but only through
the `EndToEndCompileRequest`
* As part of on-demand code generation through the `IComponentType`
APIs. In this case, the settings stored on the
`BackEndCompileRequest` are not accessible to the application
at all, and will always use their default values, so that
instantiating a "request" object doesn't really make any sense.
* The `FrontEndCompileRequest` type has a similar situation:
* Front-end compilation as part of an `EndToEndCompileRequest`
supports user configuration of `FrontEndCompileRequest` settings,
but only through the `EndToEndCompileRequest`
* Front-end compilation triggered by an `import` or a `loadModule()`
call does not support user configuration of settings at all. It
will always derive all relevant settings from thsoe on the
session ("linkage").
In addition, subsequent changes have been made to the compiler that
show a bit of a "code smell" and/or forward-looking worries for this
decomposition:
* In some cases we've had to add the same setting to multiple types
in the breakdown (front-end, back-end, end-to-end, linkage, target,
etc.) which makes it harder for us to validate that all the possible
mixtures of state work correctly.
* Related to the above, in some cases we have manual logic that copies
state from one of the objects in the breakdown to another, in order
to ensure that the user's intention is actually followed.
* As a forward-looking concern, it seems that developers have sometimes
added new configuration options and state to places that don't really
make sense according to the rationale of the original decomposition
(e.g., we probably don't want to have a lot of state that is only
available via end-to-end requests, given that the API structure is
meant to push users *away* from end-to-end compiles).
As a result of all of the above, I've been planning a large refactor
with the following big-picture goals:
* Eliminate `BackEndCompileRequest`
* Move all relevant state/options from the back-end request to
the end-to-end request, since that is the only place they could
be set anyway.
* Introduce a transient "context" type to be used for the duration
of code generation that serves the main functions that back-end
requests really served in the codebase
* Make `EndToEndCompileRequest` be a subclass of
`FrontEndCompileRequest`
* Consider addding a transient "context" type for front-end
compiles that can be used in `import`-like cases rather than
needing a full front-end request object. If this works, then
eliminate `FrontEndCompileRequest` and be back to world with
just a single `CompileRequest` type
* Move *all* compiler configuration options to a distinct type (named
something like `CompilerConfig` or `CompilerOptions` or whatever)
which stores setting as key-value pairs, and has a notion of
"inheritance" such that one configuration can extend or build on top
of another. Make all the relevant types use this catch-all structure
instead of redundantly storing flags in many places.
This change deals with the first of those bullets: removeal of
`BackEndCompileRequest`. The addition of the `CodeGenContext` type is
perhaps an unncessary additional step, but making that change helps
clean up a bunch of the code related to per-target code generation,
so I think it is the right choice.
Co-authored-by: Yong He <yonghe@outlook.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.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* #include an absolute path didn't work - because paths were taken to always be relative.
* Split out AST 'printing'.
* Replace listener with List<Section>
* Section -> Part.
* Kind -> Type Flags -> Kind for ASTPrinter::Part
* Improve comments around ASTPrinter.
* toString -> toText on Val derived types. toText appends to a StringBuilder.
* Added toSlice free function.
Added operator<< for Val derived types.
Use << where appropriate in doing toText.
* More work at mark down output.
* Fill in sourceloc for enum case.
Add more sophisticated location determination for EnumCase.
Refactored documentation output into DocMarkdownWriter.
* Improvements for sig output.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* 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
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Another fix for overriding property decls
The central problem we keep running into with `property` decls in `interface`s comes down to two choices:
1. When a member lookup `obj.someName` or a simple lookup for `someName` produces an overloaded result, we make no attempt to resolve the overloading right away, and instead postpone disambiguation until the point where that expression gets *used*, in case the context where it gets used can help in disambiguation (a notable case being when there is a call expression `obj.someName(...)` or `someName(...)`).
2. When looking up members in a the scope of a type (either for `obj.someName` or `someName` in the context of a method), we include all results from base types in the set of overloads returned, even in cases where the type has a direct member that "overrides" the inherited one.
The combination of these factors means that when a `struct` type implements a `property` to satisfy a requirement of an inherited `interface`, then references to `obj.someProp` end up being ambiguous between the property in the concrete `struct` type and the property it inherits through the `interface`.
There is no quick fix possible for issue (2). It might seem that we could just skip over members inherited through `interface`s when doing lookup in a type, but that solution wouldn't apply to inheritance from another `struct` type, or any future scenario where we support default implementations of methods in interfaces. The simple idea of saying that a derived-type member named `M` hides all inherited members named `M` is possible, but would lead to a bad user interface when a type wants to support both a core "bottleneck" method and a bunch of convenience overloads with the same name.
That leaves us with issue (1), and trying to find a reasonable fix for it. The common case is that any expression `e` eventually gets used in a context where it will be be subject to disambiguation:
* If we form a call expression `e(...)`, then the overload resolution logic will (obviously) work to disambiguate which `e` was meant.
* If `e` is used as an argument to another call (`f(... e ...)` or `... + e`), then `e` will be coerced to the expected parameter type for its argument position, and that coercion will disambiguate it (this is the bit that was fixed in #1501)
* If `e` is used in another context where a type is expected/known, it will also be coerced: `if(e)`, `int v = e`, etc.
The problem case that is left behind is any scenario where `e` is not subject to one of the above resolution cases, which mostly amounts to cases where an expression is never coerced to a single fixed type. There are a few important cases where this occurs today:
* When the expression is used as the left-hand side of an assignement (`e = ...`).
* When an expression is used to initialize a variable with an implicit type (`let v = e`).
* When inferring generic arguments from the value arguments at a call site (`f(e)` where `f` is defined as `f<T>(T v)`)
The key connecting thread in each of these cases is that the front-end needs to determine the type of `e` to make progress.
Our semantic checking logic already has functions that try to draw a distinction between the two cases:
* The `CheckTerm()` operation is supposed to be used when we expect that we will eventually coerce or otherwise diambiguate the term, and also in cases where we don't yet know if a term should name a type or a value
* The `CheckExpr()` operation is supposed to be used when we do not expect that we will apply coercion/disambiguation to a term, and need to have assurances that it has been coerced into a non-overloaded expression with a reasonable type
The simple part of the fix made here is to make `CheckExpr()` actually do part of what it is suppsoed to (attempt to disambiguate overloaded terms), and then audit all the call sites to `CheckExpr()` to make sure they are actually ones that intend to opt into that logic.
The messier part of the fix is dealing with generic argument inference, because we need to extract the type of the disambiguated expression for the purposes of inference, but we don't want to disturb the actual argument list at a call site (because type coercion of the arguments is supposed to handle the disambiguation). This part is done with a bit of special-casing in the overload-resolution context, by adding a method that gets the type or an argument after disambiguation (when possible).
* fixup
Co-authored-by: Yong He <yonghe@outlook.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
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>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Our current lookup process always finds *all* members of a type, which can include both an inherited member (e.g., from an `interface`) and one that logically overrides/implements it. If something downstream doesn't filter this result down and favor the derived member, then an ambiguity error will result.
To date, this has mostly been a non-issue because we haven't emphasized inheritance, and the main case we did support (`struct` types implemented `interface` methods) gets disambiguated as part of overload resolution for function calls.
Recent changes to support `property` declarations to `interface`s add the possibility for ambiguity between a "base" and "derived" declaration that can't rely on overload resolution for disambiguation.
The approach in this PR is to add disambiguation logic to the other main place where the results of lookup get used. If a lookup result is being assigned to a variable, passed to a function, or otherwise used in a case where a value of a specific type is needed, it will be "coerced" to the desired type. This change makes it so that the first step in the coercion logic is to try to disambiguate the expression that is being coerced.
In order to ensure that an overloaded expression can be detected and resolved even when just checking if coercion is possible, I needed to update the `canCoerce*()` functions to also take the expression that is being tested for coercibility, and not just its type. There is only one case (that I saw) where coercion checks were being made without an expression value available, and that case didn't actually need/want to handle overloading.
In order to test the fixes here, I added logic to the `property`-in-`interface` test to make sure that the critical cases work as expected (references to a derived member using "dot syntax" and "implicit `this`" syntax).
Alternatives Considered
-----------------------
The first attempt at this fix took a simpler approach: I added the disambiguation logic as a post-process on member lookup. That is, given `obj.foo` I would take the `LookupResult` for `foo` and immediately try to filter it to include only the most-derived members. This approach has the major benefit of catching even more use cases of values (and thus helping to ensure that we don't spend forever chasing down more of these ambiguity errors), but it also has two critical problems:
1. If we only trigger disambiguation when looking up `obj.foo`, then we can't do anything to help when `foo` is looked up as an ordinary identifier, but is actually equivalent to `this.foo`. A full fix would require doing this disambiguation on *every* name lookup, which leads to the second issue:
2. It is important that for a method call like `obj.m(...)` we do *not* disambiguate when looking up `obj.m`, and instead let the overload resolution for the call resolve things. That choice is what makes it possible to call an inherited `m` declaration even when there is a derived `m` with a different signature.
Issue (1) is covered by the test case that was added here, but we should probably have a test case for (2) to make sure we don't break that use case.
Caveats
-------
An important case that we don't solve in this PR is when the result of a lookup is captured in a variable without an explicit type:
let f = obj.foo;
That case also needs disambiguation, and should be addressed in a later change.
A secondary issue is that our approach to prioritizing declarations during lookup is still quite naive. We really need a way for lookup to attach information about nesting of scopes to results (to be clear that results from inner scopes should be preferred over those from outer scopes), as well as have a robust mechanism for comparing the priority of members based on the inheritance graph of a type. This change doesn't do anything to make the situation better or worse.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
There are two main bug fixes here:
* We were failing to diagnose when code calls a `[mutating]` method on a value that doesn't support mutation (that is an r-value instead of an l-value).
* We had a bug in the synthesis logic for interface requirements where we used the *result* type of the requirement in place of each of the *parameter* types.
The second bug made synthesis often produce incorrect signatures with `void` parameters.
The first bug meant that even though a `[mutating]` method should not be able to satisfy a non-`[mutating]` method (and we had code to enforce this for the "exact match" case), when we go on to try and synthesize a non-`[mutating]` method that satisfies the requirement by delegating to the user-written one, it would end up succeeding, because nothing was stopping a non-`[mutating]` method from calling a `[mutating]` one.
In each case this code adds a fix and a test case to confirm it.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* 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.
|
| |
|
|
|
|
|
|
|
| |
* 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
* * Make hash code types explicit
* Use HashCode as return type of GetHashCode
* Added conversion from double to int64_t
* Split Stable from other hash functions
* toHash32/64 to convert a HashCode to the other styles.
GetHashCode32/64 -> getHashCode32/64
GetStableHashCode32/64 -> getStableHashCode32/64
* Other Get/Stable/HashCode32/64 fixes
* GetHashCode -> getHashCode
* Equals -> equals
* CreateCanonicalType -> createCanonicalType
* Catches of polymorphic types should be through references otherwise slicing can occur.
* Fixes for newer verison of gcc.
Fix hashing problem on gcc for Dictionary.
* Another fix for GetHashPos
* Fix signed issue around GetHashPos
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* 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
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
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.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
If somebody defines two `struct` types with the same name:
```hlsl
struct A {}
// ...
struct A {}
```
and then tries to use that name when specializing a generic function:
```hlsl
void doThing<T>() { ... }
// ...
doThing<A>();
```
then the Slang front-end currently crashes, which leads to it not diagnosing the original problem (the conflicting declarations of `A`).
This change fixes up the checking of generic arguments so that it properly fills in dummy "error" arguments in place of missing or incorrect arguments, and thus guarantees that the generic substitution it creates will at least be usable for the next steps of checking (rather than leaving null pointers in the substitution).
This change also fixes up the error message for the case where a generic application like `F<A>` is formed where `F` is not a generic. We already had a more refined diagnostic defined for that case, but for some reason the site in the code where we ought to use it was still issuing an internal compiler error around an unimplemented feature.
This chagne includes a diagnostic test case to cover both of the above fixes.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
* * 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
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
These are steps towards a fix for the problem of not being able to call a static method as follows:
SomeType::someMethod();
One problem in the above is that the parser gets confused and parses an (anonynmous!) function declaration. This change doesn't address that problem, but *does* fix the problem that when checking fails to coerce `SomeType::someMethod` into a type it was triggering an unimplemented-feature exception rather than a real error message.
Another problem was that if the above is re-written to try to avoid the parser bug:
(SomeType::someMethod)();
we end up with a call where the base expression (the callee) is a `ParenExpr` and the code for handling calls wasn't expecting that. Instead, it sent the overload resolution logic into an unimplemented case that was bailing by throwing an arbitrary C++ exception instead of emitting a diagnostic.
This latter issue was fixed in two ways. First, the code path that failed to emit a diagnostic now emits a reasonable one for the unimplemented feature (this still ends up being a fatal compiler error). Second, we properly handle the case of trying to call a `ParenExpr` by unwrapping it and using the base expression instead, so that `(<func>)(<args>)` is always treated the same as `<func>(<args>)`.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* 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`).
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
This arose when a user tried to specialize the DXR 1.1 `RayQuery` type to a local variable:
```hlsl
RAY_FLAG rayFlags = RAY_FLAG_CULL_FRONT_FACING_TRIANGLES | RAY_FLAG_CULL_NON_OPAQUE;
RayQuery<rayFlags> query;
```
In this case, we issued an error around `rayFlags` not being a constant as expected, but then we also crashes later on in checking because the `DeclRef` that was being used for the type had a null pointer for the generic argument corresponding to `rayFlags`.
The main fix here was thus to add an `ErrorIntVal` case that can be used to represent something that should be an `IntVal` but where there was some kind of error in the input code so that the actual value isn't known to the compiler.
A secondary fix here is that we were issuing error messages about expecting a constant for a parameter like `rayFlags` there *twice*, and one of those times was during the `JustChecking` part of overload resolution (when we are not supposed to emit any diagnostics). I fixed that up by allowing the `DiagnosticSink` to be used to be passed down explicitly (and allowing it to be null), while also leaving behind overloaded functions with the old signatures so that all the existing logic can continue to work unmodified.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* * Improved fastRemoveAt
* Fixed off by one bug
* Fixed const safeness with List<>
* Made List begin and end const safe.
* Revert to previous RefPtr usage.
* Fix bug with casting.
* Tabs -> spaces.
Small fixes/improvements to List.
* Improve comment on List.
* hasContent -> isNonEmpty
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Support conversion from int/uint to enum types
The basic feature here is tiny, and is summarized in the code added to the stdlib:
```
extension __EnumType
{
__init(int val);
__init(uint val);
}
```
The front-end already makes all `enum` types implicitly conform to `__EnumType` behind the scenes, and this `extension` makes it so that all such types inherit some initializers (`__init` declarations, aka. "constructors") that take `int` and `uint`.
(Note: right now all `__init` declarations in Slang are assumed to be implemented as intrinsics using `kIROp_Construct`. This obviously needs to change some day, especially so that we can support user-defined initializers.)
Actually making this *work* required a bit of fleshing out pieces of the compiler that had previously been a bit ad hoc to be a bit more "correct." Most of the rest of this description is focused on those details, since the main feature is not itself very exciting.
When overload resolution sees an attempt to "call" a type (e.g., `MyType(3.0)`) it needs to add appropriate overload candidates for the initializers in that type, which may take different numbers and types of parameters. The existing code for handling this case was using an ad hoc approach to try to enumerate the initializer declarations to consider, which might be found via inheritance, `extension` declarations, etc.
In practice, the ad hoc logic for looking up initializers was just doing a subset of the work that already goes into doing member lookup. Changing the code so that it effectively does lookup for `MyType.__init` allows us to look up initializers in a way that is consistent with any other case of member lookup. Generalizing this lookup step brings us one step closer to being able to go from an `enum` type `E` to an initializer defined on an `extension` of an `interface` that `E` conforms to.
One casualty of using the ordinary lookup logic for initializers is that we used to pass the type being constructed down into the logic that enumerated the initializers, which made it easier to short-circuit the part of overload resolution that usually asks "what type does this candidate return."
It might seem "obvious" that an initializer/constructor on type `Foo` should return a value of type `Foo`, but that isn't necessarily true.
Consider the `__BuiltinFloatingPointType` interface, which requires all the built-in floating-point types (`float`, `double`, `half`) to have an initializer that can take a `float`.
If we call that interface in a generic context for `T : __BuiltinFloatingPointType`, then we want to treat that initializer as returning `T` and not `__BuiltinFloatingPointType`.
Without the ad hoc logic in initializer overload resolution, this is the exact problem that surfaced for the stdlib definition of `clamp`.
The solution to the "what type does an initializer return" problem was to introduce a notion of a `ThisType`, which refers to the type of `this` in the body of an interface.
More generally, we will eventually want to have the keyword `This` be the type-level equivalent of `this`, and be usable inside any type.
The `calcThisType` function introduced here computes a reasonable `Type` to represent the value of `This` within a given declaration.
Inside of concrete type it refers to the type itself, while in an `interface` it will always be a `ThisType`.
The existing `ThisTypeSubstitution`s, previously only applied to associated types, now apply to `ThisType`s as well, in the same situations.
The next roadblock for making the simple declarations for `__EnumType` work was that the lookup logic was only doing lookup through inheritance relationships when the type being looked up in was an `interface`.
The logic in play was reasonable: if you are doing lookup in a type `T` that inherits from `IFoo`, then why bother looking for `IFoo::bar` when there must be a `T::bar` if `T` actually implements the interface?
The catch in this case is that `IFoo::bar` might not be a requirement of `IFoo`, but rather a concrete method added via an `extension`, in which case `T` need not have its own concrete `bar`.
The simple/obvious fix here was to make the lookup logic always include inherited members, even when looking up through a concrete type.
Of course, if we allow lookup to see `IFoo::bar` when looking up on `T`, then we have the problem that both `T::bar` and `IFoo::bar` show up in the lookup results, and potentially lead to an "ambiguous overload" error.
This problem arises for any interface rquirement (so both methods and associated types right now).
In order to get around it, I added a somewhat grungy check for comparing overload candidates (during overload resolution) or `LookupResultItem`s (during resolution of simple overloaded identifiers) that considers a member of a concrete type as automatically "better" than a member of an interface.
The Right Way to solve this problem in the long run requires some more subtlety, but for now this check should Just Work.
One final wrinkle is that due to our IR lowering pass being a bit overzealous, we currently end up trying to emit IR for those new `__init` declarations, which ends up causing us to try and emit IR for a `ThisType`.
That is a case that will require some subtlty to handle correctly down the line, for for now we do the expedient thing and emit the `ThisType` for `IFoo` as `IFoo` itself, which is not especially correct, but doesn't matter since the concrete initializer won't ever be called.
* testing: add more debug output to Unix process launch function
* testing: increase timeout when running command-line tests
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Split apart `SemanticsVisitor`
The existing `SemanticsVisitor` type was the visitor for expressions, statements, and declarations, and its monolithic nature made it hard to introduce distinct visitors for different phases of checking (despite the fact that we had, de facto, multiple phases of declaration checking).
This change splits up `SemanticsVisitor` as follows:
* There is nosw a `SharedSemanticsContext` type which holds the shared state that all semantics visiting logic needs. This includes state that gets mutated during the course of semantic checking.
* The `SemanticsVisitor` type is now a base class that holds a pointer to a `SharedSemanticsContext`. Most of the non-visitor functions are still defined here, just to keep the code as simple as possible. The `SemanticsVisitor` type is no longer a "visitor" in any meaningful way, but retaining the old name minimizes the diffs to client code.
* There are distinct `Semantics{Expr|Stmt|Decl}Visitor` types that have the actual `visit*` methods for an appropriate subset of the AST hierarchy. These all inherit from `SemanticsVisitor` primarily so that they can have easy access to all the helper methods it defines (which used to be accessible because these were all the same object).
Any client code that was constructing a `SemanticsVisitor` now needs to construct a `SharedSemanticsContext` and then use that to initialize a `SemanticsVisitor`. Similarly, any code that was using `dispatch()` to invoke the visitor on an AST node needs to construct the appropriate sub-class and then invoke `dispatch()` on it instead.
This is a pure refactoring change, so no effort has been made to move state or logic onto the visitor sub-types even when it is logical. Similarly, no attempt has been made to hoist any code out of the common headers to avoid duplication between `.h` and `.cpp` files. Those cleanups will follow.
The one cleanup I allowed myself while doing this was getting rid of the `typeResult` member in `SemanticsVisitor` that appears to be a do-nothing field that got written to in a few places (for unclear reasons) but never read.
* Remove some statefulness around statement checking
Some of the state from the old `SemanticsVisitor` was used in a mutable way during semantic checking:
* The `function` field would be set and the restored when checking the body of a function so that things like `return` statements could find the outer function.
* The `outerStmts` list was used like a stack to track lexically surrounding statements to resolve things like `break` and `continue` targets.
Both of these meant that semantic checking code was doing fine-grained mutations on the shared semantic checking state even though the statefullness wasn't needed.
This change moves the relevant state down to `SemanticsStmtVisitor`, which is a type we create on-the-fly to check each statement, so that we now only need to establish the state once at creation time.
The list of outer statements is handled as a linked list threaded up through the stack (a recurring idiom through the codebase).
There was one place where the `function` field was being used that wasn't strictly inside statement checking: it appears that we were using it to detect whether a variable declaration represents a local, so I added an `_isLocalVar` function to serve the same basic purpose.
With this change, the only stateful part of `SharedSemanticsContext` is the information to track imported modules, which seems like a necessary thing (since deduplication requires statefullness).
* Refactor declaration checking to avoid recursion
The flexiblity of the Slang language makes enforcing ordering on semantic checking difficult. In particular, generics (including some of the built-in standard library types) can take value arguments, so that type expressions can include value expressions. This means that being able to determine the type of a function parameter may require checking expressions, which may in turn require resolving calls to an overloaded function, which in turn requires knowing the types of the parameters of candidate callees.
Up to this point there have been two dueling approaches to handling the ordering problem in the semantic checking logic:
1. There was the `EnsureDecl` operation, supported by the `DeclCheckState` type. Every declaration would track "how checked" it is, and `EnsureDecl(d, s)` would try to perform whatever checks are needed to bring declaration `d` up to state `s`.
2. There was top-down orchestration logic in `visitModuleDecl()` that tried to perform checking of declarations in a set of fixed phases that ensure things like all function declarations being checked before any function bodies.
Each of these options had problems:
1. The `EnsureDecl()` approach wasn't implemented completely or consistently. It only understood two basic levels of checking: the "header" of a declaration was checked, and then the "body," and it relied on a single `visit*()` routine to try and handle both cases. Things ended up being checked twice, or in a circular fashion.
2. Rather than fix the problems with `EnsureDecl()` we layered on the top-down orchestration logic, but doing so ignores the fact that no fixed set of phases can work for our language. The orchestration logic was also done in a relatively ad hoc fashion that relied on using a single visitor to implement all phases of checking, but it added a second metric of "checked-ness" that worked alongside `DeclCheckState`.
This change strives to unify the two worlds and make them consistent. One of the key changes is that instead of doing everything through a single visitor type, we now have distinct visitors for distinct phases of semantic checking, and those phases are one-to-one aligned with the values of the `DeclCheckState` type.
More detailed notes:
* Existing sites that used to call `checkDecl` to directly invoke semantic checking recursively now use `ensureDecl` instead. This makes sure that `ensureDecl` is the one bottleneck that everything passes through, so that it can guarantee that each phase of checking gets applied to each declaration at most once.
* The existing `visitModuleDecl` was revamped into a `checkModule` routine that does the global orchestration, but now it is just a driver routine that makes sure `ensureDecl` gets called on everything in an order that represents an idealized "default schedule" for checking, while not ruling out cases where `ensureDecl()` will change the ordering to handle cases where the global order is insufficient.
* Because `checkModule` handles much of the recursion over the declaration hierarchy, many cases where a declaration `visit*()` would recurse on its members have been eliminated. The only case where a declaration should recursively `ensureDecl()` its members is when its validity for a certain phase depends on those members being checked (e.g., determining the type of a function declaration depends on its parameters having been checked).
* All cases where a `visit*()` routine was manually checking the state/phase of checking have been eliminated. It is now the responsibility of `ensureDecl` to make sure that checking logic doesn't get invoked twice or in an inappropriate order.
* Most cases where a `visit*()` routine was manually *setting* the `DeclCheckState` of a declaration have been eliminated. The common case is now handled by `ensureDecl()` directly, and `visit*()` methods only need to override that logic when special cases arise. E.g., when a variable is declared without a type `(e.g., `let foo = ...;`) then we need to check its initial-value expression to determine its type, so that we must check it further than was initially expected/required.
* This change goes to some lengths to try and keep semantic checking logic at the same location in the `slang-check-decl.cpp` file, so each of the per-phase visitor types is forward declared at the top of the file, and then the actual `visit*()` routines are interleaved throughout the rest of the file. A future change could do pure code movement (no semantic changes) to arrive at a more logical organization, but for now I tried to stick with what would minimize the diffs (although the resulting diffs can still be messy at times).
* One important change to the semantic checking logic was that the test for use of a local variable ahead of its declaration (or as part of its own initial-value expression) was moved around, since its old location in the middle of the `ensureDecl` logic made the overall flow and intention of that function less clear. There is still a need to fix this check to be more robust in the future.
* Add some design documentation on semantic checking
The main thing this tries to lay out is the strategy for declaration checking and the rules/constraints on programmers that follow from it.
* fixup: typos found during review
|
|
|
The semantic checking logic was all inside `slang-check.cpp` and as a result this was a monster file that was extremely hard to follow. This change splits `slang-check.cpp` into several smaller files, although some of the resulting files are still quite large.
This change attempts to be a copy-paste job as much as possible and does *not* perform any cleanup on naming, structure, duplication, etc. in the code it deal with. No function bodies or signatures have been touched.
|
