Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue 15309 again, without breaking the fix for regression 15550 #5390

Merged
merged 3 commits into from Feb 1, 2016

Conversation

9rnsr
Copy link
Contributor

@9rnsr 9rnsr commented Feb 1, 2016

Issue 15309 has fixed in #5263, but it had introduced a regression issue 15550.

By @MartinNowak, 15550 has been fixed in #5368, but it has also reopened 15309.

Fix 15309 again.

{
if (ti.needsTypeInference(sc))
{
// Bugzilla 15550: ti is a partial instantiattion form fun!tiargs.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be instantiation.

And would it be helpful to put some of this comment in the error message? Maybe expression (%s) is a partially instantiated template and has no type?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed typo,

The suggested message would be better, but diagnostic improvement is beyond the purpose of this PR.

I have a plan to move the "has no type" message into ScopeExp.checkValue().

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. It would be nice to improve these errors as they can be unhelpful.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By #5400, the error message for typeof(func!tiargs) will be improved.

Recognize partial instantiation form inside typeof()
@yebblies
Copy link
Member

yebblies commented Feb 1, 2016

Auto-merge toggled on

@9rnsr
Copy link
Contributor Author

9rnsr commented Feb 1, 2016

Thanks!

yebblies added a commit that referenced this pull request Feb 1, 2016
Fix issue 15309 again, without breaking the fix for regression 15550
@yebblies yebblies merged commit ef5cb0d into dlang:master Feb 1, 2016
@9rnsr 9rnsr deleted the fix15309 branch February 1, 2016 13:33
@@ -5332,6 +5332,9 @@ public:
return e.semantic(sc);
}
}
// ti is an instance which requires IFTI.
sds = ti;
type = Type.tvoid;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This remains still problematic, b/c whenever we don't handle ScopeExp correctly, a wrong type (void) might slip through.
How about using Type.terror instead? It should always be an error to use the type of an template instance that might need inference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants