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

Mixin Template Intermediary Storage #237

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

MrcSnm
Copy link

@MrcSnm MrcSnm commented Dec 20, 2022

Want some feedback if something is a bit hazy or help if you can make it clearer

Want some feedback if something is a bit hazy or help if you can make it clearer
@pbackus
Copy link
Contributor

pbackus commented Dec 20, 2022

File needs a .md extension to make Github render it correctly.

@dkorpel
Copy link
Contributor

dkorpel commented Dec 20, 2022

It took me a while to understand what the DIP was trying to solve, but I now I understand: it's about how mixin templates by nature don't have lexical scoping, so you can't declare temporaries into them since those would get mixed in along with the rest. The abstract talking about "without making it get included in the code generation" made me think it was about the backend / those symbols not appearing in the resulting binary.

I think making it a visibility attribute (mixin private) makes more sense. Also, the delete keyword has been deprecated, so this DIP would revive it.

Some links to real code using workarounds for this problem would be helpful both for motivation and understanding of the DIP.

@MrcSnm
Copy link
Author

MrcSnm commented Dec 20, 2022

It took me a while to understand what the DIP was trying to solve, but I now I understand: it's about how mixin templates by nature don't have lexical scoping, so you can't declare temporaries into them since those would get mixed in along with the rest. The abstract talking about "without making it get included in the code generation" made me think it was about the backend / those symbols not appearing in the resulting binary.

I think making it a visibility attribute (mixin private) makes more sense. Also, the delete keyword has been deprecated, so this DIP would revive it.

Some links to real code using workarounds for this problem would be helpful both for motivation and understanding of the DIP.

Yea, I thought about the delete keyword because they would not appear in the resulting binary. But using mixin private seems really nice! I'm going to edit it.

Real code examples are a bit verbose, so I thought they would not fit in a DIP.

I have some like that, notice that I'm doing mixin(mem) 3 times. That seems wasteful in my vision. Plus, I done that because it is less verbose than __traits(getMember, mod, mem)

mixin template HipExportDFunctions(alias mod)
{
	import std.traits:getSymbolsByUDA, ParameterIdentifierTuple;
    pragma(msg, "Exporting ", mod.stringof);
	static foreach(mem; __traits(allMembers, mod))
	{
        //Currently only supported on classes and structs
		static if( (is(mixin(mem) == class) || is(mixin(mem) == struct) ))
		{
            mixin HipExportDFunctionsImpl!(mem, mixin(mem));
		}
	}
}

Change from mixin delete to mixin private
@ghost
Copy link

ghost commented Mar 22, 2023

I think that the goal of the DIP is not well defined.

It is about adding a new storage class to prevent clashes in static foreaches that are not declared in function bodies, it's not related to mixin templates at all. I dont know how to call that "static foreach local invariants" maybe ?

This is why it is a bit hard to understand for now.

A better introductionary example would be

before:

    static foreach(mem; __traits(allMembers, A))
    {
        // this fails for now, because the symbol already exists for b
        alias member = __traits(getMember, A, mem);
    }

    struct A
    {
        int b;
        int c;
    }

after:

    static foreach(mem; __traits(allMembers, A))
    {
        // works now:
        // "mixin private" scope creates a symbol whose
        // lifetime & scope are limted to an iteration
        mixin private alias member = __traits(getMember, A, mem);
    }

    struct A
    {
        int b;
        int c;
    } 

That get rid of the confusion created by the mixin templates. Alos since it's clear now that it's not about mixins,maybe that another syntax would make more sense.

@munael
Copy link

munael commented Dec 18, 2023

@SixthDot

While redefinition within a static foreach is one example of the issue, it's not the root cause, if I'm reading this correctly.

The DIP would make any definitions exposed via a mixin private block scoped to the current mixin's interpretation context, so they're not mixed in after the mixin template is fully interpreted and ready for inclusion in its parent/calling context.

If we were designing the language today, one could even argue that mixin private should be the default behavior for all code in a mixin, while exposed code would need its own qualifier, like export mixin <code> or a $(<code>).

That would allow for such things as using foreach instead of static foreach since the exposed/exported code is explicitly marked on its own. Although other uses of static foreach wouldn't be as clean as now.

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

Successfully merging this pull request may close these issues.

4 participants