diff options
| author | Tim Foley <tfoleyNV@users.noreply.github.com> | 2018-03-26 17:44:50 -0700 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2018-03-26 17:44:50 -0700 |
| commit | 17b66ef006cd15f822284b7f8fe4a45d77e75944 (patch) | |
| tree | a8d3a2fed4790715cb8db5f9fc97bc6a651b29bb /source/slang/check.cpp | |
| parent | 6d400b6944c56dbe0d5507288a2944e2f1710821 (diff) | |
Unify all generic parameters, even if some mismatch (#454)
* Fix decl-ref printing to handling NULL pointers
If the underlying decl, or its name is NULL, then use an empty string for the declaration name.
This issue was found when debugging, but could bite non-debug cases too, if we ever try to print something like a generic type constraint, which has no name.
* Unify all generic parameters, even if some mismatch
Fixes #449
The front end tries to infer the right generic arguments to use at a call site using a sloppily implemented "unification" approach. The basic idea is that if you pass a `vector<float,3>` into a function that operates ona `vector<T,N>` where `T` and `N` are generic paameters, then the unification will try to unify `vector<float,3>` with `vector<T,N>` which will lead to it recursively unifying `float` with `T` and `3` with `N`, at which point we have viable values to substitute in for those parameters.
Where the existing approach is maybe not quite right is in how it handles obvious unification failures. So if we ask the code to unify, say, `float` with `uint`, it will bail out immediately because those can't be unified. This sounds right superficially, but in some cases with might be calling a function that takes a `vector<float,N>` and passing a `vector<uint,3>` and we'd like to at least get far enough along with unification to see that `N` should be `3` so that the front end can maybe decide to call the function anyway, with some amount of implicit conversion.
Over time I've had to modify a lot of the "unification" logic so that it doesn't treat the obvious failures as a hard stop, and instead just returns the failure as a boolean status, but keeps on trying to unify things even after such a failure. When doing unification as part of inference for generic arguments, there will usually be subsequent steps (e.g., type conversions for function aguments) that will catch the type errors that arise.
This specific change is to make is so that when unifying the substitutions for a generic decl-ref, we try to unify all the pair-wise arguments, and don't bail out on the first mismatch (so that the `float`-vs-`uint` failure above doesn't lead to us skipping the `3` and `N` pairing).
The one case we need to watch out for in all of this is when unification is used to check if an `extension` declaration (which might be generic) is actually application to a concrete type. In that case we obviously don't want an extension for `vector<float,N>` to apply to `vector<uint,3>`, so it is important that the extension case check the return status from the unification logic (*or* in the future, it could just confirm that the substituted type is equivalent to the original as a post-process...).
I've added a test case that reproduces the original failure that surfaced the bug.
* fixup: add expected test output
Diffstat (limited to 'source/slang/check.cpp')
| -rw-r--r-- | source/slang/check.cpp | 11 |
1 files changed, 8 insertions, 3 deletions
diff --git a/source/slang/check.cpp b/source/slang/check.cpp index dbd9b37b7..2b7f8f2fc 100644 --- a/source/slang/check.cpp +++ b/source/slang/check.cpp @@ -5419,17 +5419,22 @@ namespace Slang // Their arguments must unify SLANG_RELEASE_ASSERT(fstGen->args.Count() == sndGen->args.Count()); UInt argCount = fstGen->args.Count(); + bool okay = true; for (UInt aa = 0; aa < argCount; ++aa) { if (!TryUnifyVals(constraints, fstGen->args[aa], sndGen->args[aa])) - return false; + { + okay = false; + } } // Their "base" specializations must unify if (!TryUnifySubstitutions(constraints, fstGen->outer, sndGen->outer)) - return false; + { + okay = false; + } - return true; + return okay; } bool TryUnifyTypeParam( |
