Sundry Sema fixes #4042
Sundry Sema fixes #4042
Conversation
|
@swift-ci Please smoke test |
|
I'm not convinced the second change is an improvement. It's sometimes better than no result, but other times we end up with even stranger diagnostics. I'd hold off on it for now. |
|
Sorry which change? The "invalid pattern" diagnostic change isn't quite done -- I do want to clean up that spew of crap in the existing test before I check it in. |
|
The name-lookup-filtering change. I think it needs a little more consideration to decide if it really makes sense, given the changes in diagnostics. |
|
@jrose-apple I think the diagnostics get better -- instead of "cannot find 'foo'" it says "cannot call type member on instance". Also, having a special case just for methods (and not properties, etc) seemed a little odd. |
Thanks to @lattner for the suggestion.
…stance/static context We do this in a more general way higher up in the constraint solver. Filtering out methods in name lookup only hurts diagnostics. In fact I don't think this behavior was intentional at all, since the code in question was originally written in 2013 before a lot of the more recent member lookup and diagnostic code was added. This does break source compatibility though, but in a minor way. See the change to the CoreGraphics overlay. Again, though, I think this was an accident and not intentional.
Suggest a fix-it for unqualified references to all static members from instance context, not just enum elements. Also, fix a small problem with the fix-it for replacing protocol names with 'Self' inside extension bodies -- we didn't handle nested functions properly.
The problem here is that we would just emit 'invalid pattern' instead of digging deeper, which meant that the fix-it for qualified enum element access wasn't getting inserted for more complex patterns, such as 'case X(let x)'. Unfortunately, in the matching_patterns.swift test, we emit too many diagnostics that are not really useful to figuring out the problem, and the old 'invalid pattern' made more sense. I'll work on some CSDiag tweaks to address this -- I think it makes more sense to dig there than just emit a general 'invalid pattern' diagnostic anyway. Fixes <rdar://problem/27684266>.
slavapestov
force-pushed
the
sundry-sema-fixes
branch
from
4515384 to
e354ecf
Compare
|
@swift-ci Please smoke test and merge |
| @@ -96,7 +96,8 @@ private enum Bar<T> { | |||
|
|
|||
| mutating func value() -> T { | |||
| switch self { | |||
| case E(let x): // expected-error{{invalid pattern}} | |||
| // FIXME: Should diagnose here that '.' needs to be inserted, but E has an ErrorType at this point | |||
| case E(let x): | |||
There was a problem hiding this comment.
We do have to emit some error here, or SILGen will crash.
There was a problem hiding this comment.
Ah, but if it only happens on error cases then we're fine. I see.
|
Slava convinced me in person that the changes for static members are net good, and certainly more consistent. |
No description provided.