vcg-ast by UplinkCoder · Pull Request #6556 · dlang/dmd

UplinkCoder · 2017-02-22T04:05:22Z

In order to debug the inliner I found a need to print the write-out AST after all semantic steps have been done (akin to gcc -E).

This is a developer tool and is therefore not covered by the test-suite.

MartinNowak · 2017-02-24T12:01:40Z

src/mars.d

  -transition=?    list all language changes
  -unittest        compile in unit tests
  -v               verbose
+  -vcg-ast         generates .d.cg file with the ast right before obj-file generation


Please remove from the help menu, it's an internal tool and I wouldn't want to commit to it too much.

MartinNowak · 2017-02-24T12:02:42Z

src/mars.d

+    if(global.params.vcg_ast)
+    {
+        import ddmd.hdrgen;
+        foreach(mod;modules)


Style, space after if and for

MartinNowak · 2017-02-24T12:03:58Z

src/mars.d

+            auto modFilenameLength = strlen(modFilename);
+            auto cgFilename = cast(char*)allocmemory(modFilenameLength + 4);
+            strcpy(cgFilename, modFilename);
+            cgFilename[modFilenameLength .. modFilenameLength + 4] = ".cg\0";


There are path tools in root, look at other path manipulating code in mars.d.

this is a few lines above:

assert(*ext == '.'); newname = cast(char*)mem.xmalloc((ext - p) + 1); memcpy(newname, p, ext - p); newname[ext - p] = 0; // strip extension name = newname;

Some comment indicating what you're trying to do to the file name would be most helpful.

I'm not sure what that code is doing to the filename, but in root.filename there are forceExt, defaultExt, etc.

cgFilename = filename ~ ".cg";

MartinNowak · 2017-02-24T12:05:51Z

src/hdrgen.d

+    bool hdrgen;        /// true if generating header file
+    bool ddoc;          /// true if generating Ddoc file
+    bool fullDump;     /// true if generarting a full ast_dump
+


Typo, alignment, space...

Why are so many specializations needed?

because fullDump has to behave differently then hdrgen and so does ddoc,

MartinNowak · 2017-02-24T12:08:49Z

src/hdrgen.d

                buf.level++;
            s.elsebody.accept(this);
-            if (!s.elsebody.isScopeStatement())
+            if (!s.elsebody.isScopeStatement() && !s.elsebody.isIfStatement)


What does this change actually do?

otherwise

if (a) {} else if (b) {} else if (c) {}

will be printed as

if (a) {} else if (b) {} else if (c) {}

MartinNowak · 2017-02-24T12:09:47Z

src/hdrgen.d

+            if (ti.aliasdecl)
+            {
+                ti.aliasdecl.accept(this);
+            }


And this? Commit message is not helpful here.

it writes out the instanced body of the template.

jacob-carlborg · 2017-02-24T13:20:21Z

Could we get a sample of the output?

UplinkCoder · 2017-02-24T17:11:33Z

@jacob-carlborg you don't want to see this :)

MartinNowak · 2017-02-24T17:35:39Z

alias vec = __vector(int[4]);

vec binop(vec a)
{
    vec b = a;
    return b;
}

import object;
alias vec = __vector(int[4]);
__vector(int[4]) op(__vector(int[4]) a)
{
	return a;
}
__vector(int[4]) binop(__vector(int[4]) a)
{
	return (__vector(int[4]) a = a;) , a;
}

It's really only useful to debug dmd internal issues.

UplinkCoder · 2017-02-24T18:45:46Z

It does have the utility of showing templates and mix-ins expanded.
That is why I made it a user-visible switch.

jacob-carlborg · 2017-02-25T13:36:49Z

Cool. Perhaps a nitpick, but this isn't really the AST itself that is printed, rather the result after lowering/semantic pass. I think having ast in the name of the flag is a bit misleading (although I don't know that vcg stands for).

I've been working on a visitor that prints the data in the AST itself, which I initially thought this was.

UplinkCoder · 2017-02-25T19:03:37Z

-vcg-ast stands for View CodeGen-AST
@jacob-carlborg why would you implement another visitor to do it ?
The PrettyPrintVisitor isn hdrgen does it for you. You just have to evoke it before the semantic-passes.

jacob-carlborg · 2017-02-25T19:24:26Z

jacob-carlborg why would you implement another visitor to do it ?
The PrettyPrintVisitor isn hdrgen does it for you. You just have to evoke it before the semantic-passes.

As far as I can see PrettyPrintVisitor will print the source code that resulted in a given AST. My visitor prints the actual structure of the AST:

NewExp
{
  Expression
  {
    type: TypeClass
    {
      Type
      {
        ty: 7,
        deco: C4core3ast10expression18FunctionExpression
      },
      sym: ClassDeclaration
      {
        AggregateDeclaration
        {
          ScopeDsymbol
          {
            Dsymbol
            {
              ident: Identifier(FunctionExpression)
            },

          },

        }
      }
    },
    op: TOKnew
  },
  thisexp: Expression(null),
  newargs: null,
  newtype: TypeClass
}

It's a good tool to learn the AST and understand the compiler.

UplinkCoder · 2017-02-25T19:37:26Z

@jacob-carlborg where is that work ?

jacob-carlborg · 2017-02-25T19:39:02Z

jacob-carlborg where is that work ?

In my local branch 😃. I can extract and make it publicly available if it's of any interest. It's not complete, I've added types and fields when needed.

UplinkCoder · 2017-02-25T19:42:02Z

@jacob-carlborg please push it. I'd like to have a look.

jacob-carlborg · 2017-02-25T19:47:40Z

I just extracted it from my other branch without compiling it. The file is not added to the makefiles or anything.

https://github.com/jacob-carlborg/dmd/blob/ast-printer/src/ast_printer.d

veelo · 2017-03-05T20:01:13Z

Not a comment on the code, but maybe a useful tip: for inspecting large ASTs, it can be practical if nodes in the tree can be collapsed and expanded while you are looking at it. This can be done by writing the tree out as HTML5 using the <detail> tag (not supported by IE though). Here is an example. The code that generated this is in https://github.com/PhilippeSigaud/Pegged/blob/master/pegged/tohtml.d.

jacob-carlborg · 2017-03-05T20:04:22Z

The output is actually some kind of JavaScript format. I paste that in my editor, enable JavaScript as the language and then code folding works. Both human readable and collapsing works.

WalterBright · 2017-03-07T10:49:08Z

General comment: there are zero comments in the code. For example, it seems like "print template instances" ought to be a comment somewhere in the commit entitled "print template instances". Little nudges like these can be very helpful.

WalterBright · 2017-03-07T10:54:09Z

src/mars.d

+            auto modFilename = mod.srcfile.toChars();
+            auto modFilenameLength = strlen(modFilename);
+            auto cgFilename = cast(char*)allocmemory(modFilenameLength + 4);
+            strcpy(cgFilename, modFilename);


One reason I dislike ad-hoc C string manipulation is the use of strcpy here. It should be memcpy(cgFilename, modFilename, modFilenameLength) because the length is already computed, and strcpy will compute it again, and strcpy will also add a 0 which will get promptly overwritten.

…-fullDump output for enums

MartinNowak · 2017-03-11T18:31:13Z

Doesn't seem like any blocker for a debug tool.

MartinNowak · 2017-03-11T18:31:27Z

Auto-merge toggled on

schveiguy · 2017-08-09T13:32:34Z

What version of the compiler did this make it into? Can it be put into the changelog? Every time I want to use this feature, I have to search for Stefan's posts to the forums to figure out the name of the switch. It's not listed in dmd --help, and not on the changelog.

It's a super-handy feature, especially when trying to figure out which templates are instantiated, and how are they implemented.

UplinkCoder · 2017-08-09T14:12:42Z

@schveiguy it should be in 2.073 ?
The reason this is not documented is that it is not an official feature.
It's a tool for compiler-devs and not intended to be user-facing.
see: #6556 (comment)

schveiguy · 2017-08-09T14:23:40Z

Just today, I used it to determine the exact implementation of formattedRead when called with certain parameters, to see if it allocates using the GC. Tagging a formattedRead call as @nogc didn't help because it uses enforce.

Having a tool to understand exactly how the compiler is going to interpret your templates and other code is sometimes indispensable, especially for a language like D where so much meta-code is happening.

Note, if you didn't want it to be public facing, you shouldn't have posted about it on the newsgroup :)

UplinkCoder · 2017-08-09T14:30:15Z

@schveiguy I want users to know about it.
Though if it were a publicly documented switch people might relay on the quality and format of the output. Which I cannot guarantee.

schveiguy · 2017-08-09T14:32:11Z

people might relay on the quality and format of the output

There has to be a way that it can be documented somewhere, but not "official". Again, I just want to be able to remember the name of the switch (it's not intuitive to me).

UplinkCoder · 2017-08-09T14:33:18Z

@schveiguy "view code gen - a s t"

schveiguy · 2017-08-09T14:44:10Z

I get that the name stands for something, but I just can't remember it :)

Haven't you ever misremembered an option of a tool, looked for it using --help, and if not there, tried man, or online docs? The issue is that there isn't any existing doc that shows it.

UplinkCoder · 2017-08-09T15:08:29Z

@schveiguy I put it in the wiki
https://wiki.dlang.org/DMD_Source_Guide#DMD_Hacking_Tips_.26_Tricks

Apparently AST for ldc is an abuse of terminology, as it doesn't produces anything resembling a syntax tree: dlang/dmd#6556 (comment) . It is potentially meaningful only to ldc developers. Anyway the `generateAST` result type is fixed, along with some other small stuff around.

UplinkCoder changed the title ~~[WIP] first steps to vcg-ast~~ vcg-ast Feb 22, 2017

UplinkCoder force-pushed the cg_ast_ouput branch 5 times, most recently from 0e5cd6f to 21154e2 Compare February 23, 2017 15:32

MartinNowak requested changes Feb 24, 2017

View reviewed changes

WalterBright reviewed Mar 7, 2017

View reviewed changes

UplinkCoder added 6 commits March 10, 2017 14:35

implement vcg-ast

973bfe4

remove raising identation-levle of if-else chains and beautify hdrgen…

ea62869

…-fullDump output for enums

print template instances

f75a0d4

print the newline only in fullAst mode to avoid having to change tests

20e0d8a

address changes

c55a1d7

add explainatory comments

06fb773

UplinkCoder force-pushed the cg_ast_ouput branch from c02be1a to 06fb773 Compare March 10, 2017 13:36

MartinNowak approved these changes Mar 11, 2017

View reviewed changes

MartinNowak merged commit 9d0dba9 into dlang:master Mar 11, 2017

ibuclaw mentioned this pull request Aug 5, 2017

More fixing up missing fields and virtual functions in C++ headers #7062

Merged

OfekShilon mentioned this pull request Nov 13, 2023

Fix #4654 compiler-explorer/compiler-explorer#5749

Merged

Uh oh!

Conversation

UplinkCoder commented Feb 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

UplinkCoder Feb 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

UplinkCoder Mar 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

UplinkCoder Feb 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jacob-carlborg commented Feb 24, 2017

Uh oh!

UplinkCoder commented Feb 24, 2017

Uh oh!

MartinNowak commented Feb 24, 2017

Uh oh!

UplinkCoder commented Feb 24, 2017

Uh oh!

jacob-carlborg commented Feb 25, 2017

Uh oh!

UplinkCoder commented Feb 25, 2017

Uh oh!

jacob-carlborg commented Feb 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

UplinkCoder commented Feb 25, 2017

Uh oh!

jacob-carlborg commented Feb 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

UplinkCoder commented Feb 25, 2017

Uh oh!

jacob-carlborg commented Feb 25, 2017

Uh oh!

veelo commented Mar 5, 2017

Uh oh!

jacob-carlborg commented Mar 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WalterBright commented Mar 7, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MartinNowak commented Mar 11, 2017

Uh oh!

MartinNowak commented Mar 11, 2017

Uh oh!

schveiguy commented Aug 9, 2017

Uh oh!

UplinkCoder commented Aug 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

UplinkCoder commented Feb 22, 2017 •

edited

Loading

UplinkCoder Feb 24, 2017 •

edited

Loading

UplinkCoder Mar 7, 2017 •

edited

Loading

UplinkCoder Feb 24, 2017 •

edited

Loading

jacob-carlborg commented Feb 25, 2017 •

edited

Loading

jacob-carlborg commented Feb 25, 2017 •

edited

Loading

jacob-carlborg commented Mar 5, 2017 •

edited

Loading

UplinkCoder commented Aug 9, 2017 •

edited

Loading

UplinkCoder commented Aug 9, 2017 •

edited

Loading

UplinkCoder commented Aug 9, 2017 •

edited

Loading