Issue 9631 - Error message not using fully qualified name#7405
Issue 9631 - Error message not using fully qualified name#7405dlang-bot merged 3 commits intodlang:masterfrom
Conversation
Fix "cannot implicitly convert expression" ambiguous errors. Add Type.toAutoQualChars().
|
Thanks for your pull request, @ntrel! We are looking forward to reviewing it, and you should be hearing from a maintainer soon. Some tips to help speed things up:
Bear in mind that large or tricky changes may require multiple rounds of review and revision. Please see CONTRIBUTING.md for more information. Bugzilla references
|
src/ddmd/mtype.d
Outdated
| return buf.extractString(); | ||
| } | ||
|
|
||
| static const(char*)[2] toAutoQualChars(Type t1, Type t2) |
There was a problem hiding this comment.
Please add DDoc comment header.
There was a problem hiding this comment.
Also, why the word "Auto"? I would have expected something like toFullyQualChars.
There was a problem hiding this comment.
Will do. Auto is because it only qualifies the types if they would otherwise collide. Suggestions welcome.
There was a problem hiding this comment.
Ok, I see. No need to change it in my opinion. I was just curious.
WalterBright
left a comment
WalterBright
left a comment
There was a problem hiding this comment.
Needs Ddoc header for toAutoQualChars() function. Also, please make it a non-member function instead of static.
| * For printing two types with qualification when necessary. | ||
| * Disambiguates the case where toChars() on each type match. | ||
| */ | ||
| const(char*)[2] toAutoQualChars(Type t1, Type t2) |
There was a problem hiding this comment.
Still needs "Returns" and "Params". See https://dlang.org/spec/ddoc.html
There was a problem hiding this comment.
Added Returns. I don't know anything meaningful to put for Params.
| * Disambiguates the case where toChars() on each type match. | ||
| * Returns: Names of types. | ||
| */ | ||
| const(char*)[2] toAutoQualChars(Type t1, Type t2) |
There was a problem hiding this comment.
How about this?
/**
* For printing two types with qualification when necessary.
* Params:
* t1 = The first type to receive the type name for
* t2 = The second type to receive the type name for
* Returns:
* The fully-qualified names of both types if the two type names are not the same,
* or the unqualified names of both types if the two type names are the same.
*/
ntrel
force-pushed
the
qualify-symbol
branch
from
eecb944 to
92cd084
Compare
|
BTW I have follow up fixes for 3 other kinds of error message where the same fix applies, will submit soon. |
|
|
||
| void main() | ||
| { | ||
| T2!().F x = T1!().F(); |
There was a problem hiding this comment.
Sorry @ntrel but this isn't passing coverage. You'll have to add a test that tests the case when both I'm not sure what's missing, but it needs additional tests to increase coverage.t1 and t2 are the same different (or so I assume).
There was a problem hiding this comment.
Nevermind, The coverage report looks weird, and the project coverage looks good, so let's merge this.
4 participants
Fix cannot implicitly convert expression of type T to T ambiguous errors.
Add
Type.toAutoQualChars().There are other related errors that could use this fix, this only addresses the first test case in bugzilla.