[Diagnostic] Fix diagnostic when checking conformance for IUO of gene… #32987
Conversation
…ric parameter Prints TypeRepr in diagnostic if possible to throw accurate diagnostic for IUO conformance checking Fixes [rdar://problem/64953106]. Fixes SR-13119
| NULL, | ||
| varDecl->getType(), | ||
| }; | ||
| } |
There was a problem hiding this comment.
Given we're doing this in three places in this function, I think it would be a good idea to extract and reuse:
static TypeLoc getTypeLocForDiagnostics(VarDecl *VD) {
auto varTy = VD->getType();
if (VD->isImplicitlyUnwrappedOptional()) {
return TypeLoc(VD->getTypeReprOrParentPatternTypeRepr(), varTy);
}
return TypeLoc::withoutLoc(varTy);
}
There was a problem hiding this comment.
I don't think this needs to be a utility. Its ideal form is four lines
TypeLoc typeLoc = {
varDecl->getTypeReprOrParentPatternTypeRepr(),
varDecl->getType(),
};
| }; | ||
| } else { | ||
| typeLoc = { | ||
| NULL, |
There was a problem hiding this comment.
You don't need to conditionalize this. getTypeReprOrParentPatternTypeRepr will always get what the user wrote, if they wrote something at all.
There was a problem hiding this comment.
Yes, but this is breaking nested class/struct types. Please see the test regression I have commented in the https://bugs.swift.org/browse/SR-13119. @varungandhi-apple suggested a change which I inferred to be the above.
There was a problem hiding this comment.
Name lookup will see the innermost name in that scope and it should just work out. As long as those fixits compile, this is not a regression and we should update those tests. I very much prefer we preserve the way the user spelled types than fully qualify them all the time.
There was a problem hiding this comment.
Sure, okay. I get it
There was a problem hiding this comment.
I very much prefer we preserve the way the user spelled types than fully qualify them all the time.
I didn't realize there was a disagreement here. Should we write this down somewhere so that newer diagnostics use this style? I thought this was a regression because the original code was printing the full type.
There was a problem hiding this comment.
That's a good point. I recall we have a document that tries to explain good diagnostic hygiene. And this is certainly one of those places where you have to be really careful. Because the user-provided type may not always work if you're pulling something from a different DeclContext, and the fully-qualified type may not even work when you have ambiguities caused by e.g. the module-and-type-sharing-names bug.
Name lookup will see the innermost name anyway and preferred over fully qualified name. Hence the test cases are also updated.
test/decl/protocol/special/coding/struct_codable_failure_diagnostics.swift
Outdated
Show resolved
Hide resolved
This commit includes better comments for easy understanding, formatted the bug fix code with clang-format and fixes wrong variables inadvertently introduced.
|
@swift-ci smoke test |
1 similar comment
|
@swift-ci smoke test |
|
@Nikhil0487 I think we could update those tests that are failing because although they are misleading because |
This commit changes diagnostic type from error type to Int. Although this diagnostic updated is incorrect, this will be resolved when 32371 gets pulled.
|
@swift-ci Please smoke test |
|
@swift-ci Please smoke test macOS platform |
…ric parameter
Prints TypeRepr in diagnostic if possible to throw
accurate diagnostic for IUO conformance checking
Fixes [rdar://problem/64953106]. Fixes SR-13119
The diagnostic emitted when checking conformance for IUO of generic parameter will
print TypeRepr if present to have accurate type information. This prevents
confusion in the diagnostics emitted.
Fixes [rdar://problem/64953106]. Fixes SR-13119(https://bugs.swift.org/browse/SR-13119).