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 gen #426

Closed
wants to merge 2 commits into from
Closed

Mixin gen #426

wants to merge 2 commits into from

Conversation

MartinNowak
Copy link
Member

This pull adds a new command line option -M (-Mf, -Md).
When set string mixins are written to a separate file, which by default
carry a .mixin extension.
The logic for the files paths matches that of header generation.

The main benefits of adding this option are the simpler visibility of the
code vs. using pragma(msg) and fixing debug line information
which made it previously almost impossible to debug mixins.

I had to fix File::appendv and add File::append for POSIX.

@Trass3r
Copy link
Contributor

Trass3r commented Oct 4, 2011

Hmm what about template mixins?

@MartinNowak
Copy link
Member Author

Mixin templates are merely general templates with different instantiation scope
and have their code nicely defined in the sources.
Therefor it is no issue to debug/see the actual code.

@9rnsr
Copy link
Contributor

9rnsr commented Oct 8, 2011

I think we can use pragma(msg) for code printing.
If you want source position for printing, you can write a small utility like follows:

template CodePrinter(string code)
{
    @property string CodePrinter(string fn = __FILE__, size_t ln = __LINE__)()
    {
        pragma(msg, "mixin at ", fn, "(", cast(int)ln, ")");
        pragma(msg, "----\n", code, "\n----");
        return code;
    }
}
string gen()
{
    return "1 + 2";
}
struct S
{
    enum n = mixin(CodePrinter!(gen())());
}

output (in compilet time):

mixin at test.d(16)
----
1 + 2
----

What is the advantage of this proposal than a library way?

@MartinNowak
Copy link
Member Author

The advantage is that the debug output gets proper file and line information.
So when debugging a program you don't hit the macro wall.
Because it will not really be possible for any IDE tool to expand string mixins I don't
see any solution to this but to generate a file. Some recent C/C++ tools do expand macros
which is a huge help/feature in certain situations.

@Trass3r
Copy link
Contributor

Trass3r commented Jan 6, 2012

Does this also fix error message line numbers?

@MartinNowak
Copy link
Member Author

It could if the files were written before parsing and not afterwards.
Think I should change that.

@mihails-strasuns
Copy link

vote up, useful feature to switch on/off often, much more suited for compiler than library

@PhilippeSigaud
Copy link

So, no love for this, then?

@dnadlinger
Copy link
Member

Imho integrating this with the compiler makes a lot of sense, as it allows us to generate correct debug info for metaprogramming/string mixin-heavy code (this is a huge problem in practice). Only nitpick: The command line option description should state more clearly that this is just a debugging aid – as the average DMD user, I would wonder what a »mixin file« is…

@MartinNowak
Copy link
Member Author

The command line option description should state more clearly that this is just a debugging aid

Do you have any idea?

Does this also fix error message line numbers?

It does by now. The mixin code is appended before parsing and the AST has line number information pointing to the mixin file.

@mrmonday
Copy link
Contributor

How about "generate a file with all string mixins expanded (primarily useful for debugging)"?

@andralex
Copy link
Member

(background: I'm doing a review of pull requests < 1000 as they tend to be controversial)

I think this has great promise, but we should rethink it. Large string mixins are arbitrarily difficult to compute (think parser generators, regexen, and such). What we really want is not to dump mixins in a file for future human examination, but develop a simple and coherent system for saving generated mixins across compilations - a sort of "precompiled mixins" if you wish. That way the compiler seeing ctRegex!"[0-9A-Z-a-z][0-9A-Z-a-z_-]*" will dump the generated string once and then reuse it unless it detect it must re-generate it.

That is a nontrivial task but, I think, one well worth thinking about if we want to scale up generative programming in D.

braddr pushed a commit to braddr/dmd that referenced this pull request Oct 22, 2012
Issue 7476 - Write(ln) functions no longer accept retro range
@MartinNowak
Copy link
Member Author

What we really want is not to dump mixins in a file for future human examination.

This pull is only about human examination. I made this because it fixes error messages and debug information. Both of which were already unvaluable tools for developing a lexer generator (https://github.com/dawgfoto/lexer).
I've experimented with way more complex DFA/LALR lexer/parser generators some month ago and doing that at compile time wouldn't have worked out without this.

unless it detect it must re-generate it

That sounds like Make or rdmd, doesn't it?

a sort of "precompiled mixins" if you wish

We could look into saving compressed, serialized parse trees at some point. This would benefit normal compilation too.
Otherwise I think we should rather focus on speeding up the compiler even though caching is never a bad idea.

9rnsr and others added 2 commits December 20, 2012 09:06
Issue 9068 - fix compiler error when breaking from some labelled loops.
 - this greatly simplifies to debug mixins
 - the usual -Mf -Md variants are implemented as well
 - needed to fix File::appendv and add File::append for
   POSIX
@MartinNowak
Copy link
Member Author

The problem remains that debug info can't reference generated source code.

@andralex
Copy link
Member

I'll close this for now pending further insights.

@ghost
Copy link

ghost commented Nov 19, 2013

I think we should consider reopening this. As dawgfoto said, this pull is only about human examination (debugging), not about compilation caching.

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.

8 participants