Re: cfe-dev Digest, Vol 156, Issue 88

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view

Re: cfe-dev Digest, Vol 156, Issue 88

Vassil Vassilev via cfe-dev

> Perhaps we should consider the AST dump to be an expert-level feature and
> not try to optimize it for beginner usage, especially if we have better
> output methods. I'm not sure, though; I think telling beginners to look at
> the AST dump and match what they see there has been somewhat effective.
> I think we should assume that we can ask the maintainers of compiler
> explorer to render the output nicely, if we find that's a problem. (They
> already apply some postprocessing to it, to remove pointer values and the
> like.) As an alternative to greying out implicit nodes, we could render
> them inside square brackets or something:
> |-VarDecl 0x638eb60 <col:38, col:43> col:40 a 'A' callinit
> | `-[CXXConstructExpr 0x638eff0 <col:40, col:43> 'A' 'void (int)']
> |   `-IntegerLiteral 0x638ec10 <col:42> 'int' 3
> `-VarDecl 0x638f030 <col:45, col:53> col:47 b 'A' callinit
>  `-CXXConstructExpr 0x638f100 <col:47, col:53> 'A' 'void (int, int)'
>    |-IntegerLiteral 0x638f098 <col:49> 'int' 3
>    `-IntegerLiteral 0x638f0b8 <col:52> 'int' 4
>> Nevertheless, this change in the direction that I consider easier hasn't
>> been 100% popular among other clang developers (though I wonder how much of
>> that is due to being experts?). At this point, if someone wishes to reverse
>> the change of default I don't think I would attempt to oppose it. It's not
>> clear to me whether my vision for making things easier for newcomers (which
>> I set out in EuroLLVM and which included ignoring these nodes) is shared.
>> It might be better at this point for others to decide.

I agree the goal is noble, and more efforts are needed to make things easier for newcomers.  Such efforts are the best and broadest means of increasing "inclusivity," an agreed-upon ideal.  

I don’t use AST matchers and so cannot comment on the specific issues.  But this discussion suggests another good approach would be a newcomer-friendly dump, perhaps call it dump_pedantic().

Putting the implicit nodes in brackets or dimmer text is a start, but better would be to improve the aesthetics still more and, most important of all, add a little explanation to each implicit node in the dump, explaining its semantic role.

E.g. I believe you referenced this example to motivate your AST matchers change awhile back:

struct B { B(int); };
B func1() { return 42; }

Dump of func1:

FunctionDecl 0x55e885ed5eb8 <<source>:7:1, line:9:1> line:7:3 func1 'B ()'
`-CompoundStmt 0x55e885ed6538 <col:11, line:9:1>
  `-ReturnStmt 0x55e885ed6520 <line:8:3, col:10>
    `-ExprWithCleanups 0x55e885ed6508 <col:3, col:10> 'B'
      `-CXXConstructExpr 0x55e885ed64d0 <col:3, col:10> 'B' 'void (B &&) noexcept' elidable
        `-MaterializeTemporaryExpr 0x55e885ed6478 <col:10> 'B' xvalue
          `-ImplicitCastExpr 0x55e885ed6328 <col:10> 'B' <ConstructorConversion>
            `-CXXConstructExpr 0x55e885ed62f0 <col:10> 'B' 'void (int)'
              `-IntegerLiteral 0x55e885ed5f90 <col:10> 'int' 42

That is indeed way too overwhelming, a real turn off to newcomers.

But it wouldn’t be that hard to make it a bit prettier and explanatory; perhaps something like this:

FunctionDecl func1 'B ()' [<<source>:7:1, line:9:1> line:7:3]
      `-ExprWithCleanups 'B' (handles destruction of …)
        CXXConstructExpr 'B' 'void (B &&) noexcept' elidable (moves B object to ...)
        MaterializeTemporaryExpr 'B' xvalue (allocates … )
        ImplicitCastExpr 'B' <ConstructorConversion> (converts ctor’s void return type to B)
        CXXConstructExpr 'B' 'void (int)'
         `-IntegerLiteral 'int' 42

Or something like that.  I myself don’t know why constructors have void return types and why certain of these implicit expressions are needed, but someone who understands these components well could knock out a good "dump_pedantic()" function in a day, and it would greatly help newcomers get up to speed.


cfe-dev mailing list
[hidden email]