[WIP] Check the containing context when checking access for operators #18164
Conversation
...to push people towards getFormalAccessScope. The one use case that isn't covered by that is checking whether a declaration behaves as 'open' in the current file; I've added ValueDecl::hasOpenAccess to handle that specific case. No intended functionality change.
This gets adjustAccessLevelForProtocolExtension, a hack of sorts to begin with, out of ValueDecl's general API, and down to a helper for isAccessibleFrom and isSetterAccessibleFrom. (The only reason these two don't go through access scopes is as an optimization.)
This function (actually checkAccess) was relying on some implicit assumptions that aren't actually valid in all cases. When they're not, just fall back to a slower but more correct implementation; when they are, assert that the two implementations get the same answer. This allows us to get rid of adjustAccessLevelForProtocolExtension (see previous commit), though unfortunately not all of the associated hack. The diff is bigger than I'd like because it includes moving functions from NameLookup.cpp into Decl.cpp, but most of those didn't change. - checkAccess only changed in the one if branch for protocols - ValueDecl::isAccessibleFrom just added the assertion - AbstractStorageDecl::isSetterAccessibleFrom did not change No expected functionality change.
|
@swift-ci Please test compiler performance |
Build comment file:Compilation-performance test failed |
This might help eliminate operators from overload sets sooner, if they weren't supposed to be accessed from the current context at all. Or it might not make any noticeable difference. No functionality change; the tests added did not change behavior.
|
@swift-ci Please test compiler performance |
|
@swift-ci Please test |
|
Build failed |
|
Build failed |
|
!!! Couldn't read commit file !!! |
|
@swift-ci Please test compiler performance |
Build comment file:Compilation-performance test failed |
@aciidb0mb3r, @shahmishal, do we need to clean out the build directory here? |
|
hmmm, I think we need to make sure llbuild rebuilds the Swift bindings when Swift compiler binary changes. |
|
I'll try to fix that today. Cleaning should work in the meantime. |
|
@jrose-apple workspace cleaned, please start testing again. |
|
Well, the patch does have other bugs anyway, so I'll hold off here. But hopefully it's fixed things for others. |
|
@jrose-apple seems like the refactoring parts have been accomplished in other PRs. Is there a bug fix here that we should take as well, or should this be closed? |
|
Indeed, I'm not sure what's salvageable from here. It's been a long time. |
This might help eliminate operators from overload sets sooner, if they weren't supposed to be accessed from the current context at all. Or it might not make any noticeable difference. Builds on #18048.
No functionality change; the tests added did not change behavior.