Re-factor the handling of *empty* Name
-instances (PR 13612 follow-up)
#13738
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.
When working on PR #13612, I mostly prioritized a simple solution that didn't require touching a lot of code. However, while working on PR #13735 I started to realize that the static
Name.empty
construction really wasn't a good idea.In particular, having a special
Name
-instance where thename
-property isn't actually a String is confusing (to put it mildly) and can easily lead to issues elsewhere. The only reason for not simply allowing thename
-property to be an empty string, in PR #13612, was to avoid having to touch a lot of existing code. However, it turns out that this is only limited to a few methods in thePartialEvaluator
and a few of theBaseLocalCache
-implementations, all of which can be easily re-factored to handle emptyName
-instances.All-in-all, I think that this patch is even an overall improvement since we're now validating (what should always be)
Name
-data better in thePartialEvaluator
.This is what I ought to have done from the start, sorry about the code churn here!