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 16689 - Errors in instantiated mixin templates should show instantiation point #12463

Merged
merged 1 commit into from
May 26, 2021

Conversation

ibuclaw
Copy link
Member

@ibuclaw ibuclaw commented Apr 23, 2021

Template mixins are a kind of enclosing template instance that the Scope was instantiated from.

Setting it in semantic2, same as TemplateInstance, allows static asserts to follow the instantiation context from mixins.

Give this PR a little while to allow time to decide whether this is correct, given that template mixins have a hybrid status between isTemplateInstance() == true and isInstantiated() == false

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @ibuclaw!

Bugzilla references

Auto-close Bugzilla Severity Description
16689 enhancement Errors in instantiated mixin templates should show instantiation point

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#12463"

Copy link
Contributor

@RazvanN7 RazvanN7 left a comment

Choose a reason for hiding this comment

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

Looks good. Ping me when you think that this is good to go.

@ibuclaw
Copy link
Member Author

ibuclaw commented Apr 28, 2021

I would prefer to systematically go through all code that makes use of sc.tinst and write a test triggering each with a template mixin (as before, only a template instance would have enabled those code paths).

ping @WalterBright.

@ibuclaw
Copy link
Member Author

ibuclaw commented Apr 28, 2021

Though as this is limited to semantic2 only, there is not going to be much in the way of collateral damage. So I wouldn't be surprised if I find nothing.

@RazvanN7
Copy link
Contributor

RazvanN7 commented May 7, 2021

@ibuclaw any progress on this?

@RazvanN7
Copy link
Contributor

@ibuclaw OK to merge?

@RazvanN7 RazvanN7 closed this May 26, 2021
@RazvanN7 RazvanN7 reopened this May 26, 2021
@RazvanN7 RazvanN7 merged commit 79f0462 into dlang:master May 26, 2021
andralex pushed a commit to andralex/dmd that referenced this pull request May 27, 2021
UplinkCoder pushed a commit to UplinkCoder/dmd that referenced this pull request Aug 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants