Fix crash when creating predicate with generic root type #1483
+28
−5
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
KeyPath
s used in predicates with a computed component can contain two types of arguments:Predicate
can support generic arguments, but not concrete values passed to subscripts (the subscript predicate expression should be used instead). When originally writing this code, I had conflated the two concepts as one and introduced therestrictArguments
flag to work around cases where code accidentally asserted eagerly for only generic arguments. However, I also accidentally defaulted the argument tofalse
so the assertion never ran. As part of #1476 I set the default value totrue
correcting this mistake. Unfortunately, since the assertion conflates the two types of arguments, the assertion trips incorrectly in some existing client code (Predicate
s created where the root type is generic and therefore property access provides a generic argument in the key path). For now, to fix this regression, this removes therestrictArguments
parameter and removes the assertion altogether. In the long term, we should followup with a fix that adds this assertion back with improved validation logic not based on arestrictArguments
parameter but rather based on other contents in the keypath to distinguish between the two types of arguments (tracked by #1482 - I think this is something we should do as a followup since its implementation requires more testing and either requires a new C shim or some even more hacky workarounds).rdar://159210502