Skip to content

fix Issue 11922 - [REG2.065a] ICE on nonexistent identifier in templated... #3094

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.

Sign up for GitHub

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

Merged
9rnsr merged 1 commit into from
Jan 16, 2014
Merged

fix Issue 11922 - [REG2.065a] ICE on nonexistent identifier in templated... #3094

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/expression.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8775,7 +8775,7 @@ Expression *CallExp::semantic(Scope *sc)

// Do overload resolution
f = resolveFuncCall(loc, sc, s, tiargs, ue1 ? ue1->type : NULL, arguments);
if (!f)
if (!f || f->type->ty == Terror)
Copy link
Contributor

@9rnsr 9rnsr Jan 15, 2014

Choose a reason for hiding this comment

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

Your approach will continue to pollute compiler code!

Copy link
Contributor

@dnadlinger dnadlinger Jan 15, 2014

Choose a reason for hiding this comment

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

I agree with Kenji here in that this approach seems to be much more error-prone. But then, I usually only work on the semantic part for the occasional bug fix, so I'm really not the one to complain.

Copy link
Member Author

@WalterBright WalterBright Jan 15, 2014

Choose a reason for hiding this comment

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

I don't agree, for reasons explained in #3034

Copy link
Contributor

@9rnsr 9rnsr Jan 15, 2014

Choose a reason for hiding this comment

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

@klickverbot I think that Walter's approach is essentially fragile, and it will need continued maintenance, like this PR. So I really don't like this change.

Copy link
Contributor

@9rnsr 9rnsr Jan 15, 2014

Choose a reason for hiding this comment

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

@klickverbot If you have a time, could you please read our discussion in #3034?

Copy link
Contributor

@dnadlinger dnadlinger Jan 15, 2014

Choose a reason for hiding this comment

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

I've read the discussion there, otherwise I wouldn't have commented on it in the first place based on this one change. I agree with your reasoning, but as I said, I wouldn't put too much weight on my own opinion here.

Copy link
Contributor

@9rnsr 9rnsr Jan 15, 2014

Choose a reason for hiding this comment

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

@klickverbot I understand. Thank you.

return new ErrorExp();

if (f->needThis())
Expand Down
18 changes: 18 additions & 0 deletions test/fail_compilation/ice11922.d
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
TEST_OUTPUT:
---
fail_compilation/ice11922.d(11): Error: undefined identifier a
fail_compilation/ice11922.d(17): Error: template instance ice11922.S.f!int error instantiating
---
*/

struct S
{
auto f(B)(B) { return a; }
}

void main()
{
S s;
s.f(5);
}