Group scattered AST visitors into a visitors package

[wilzbach]

I started off by moving the four visitors (parsetime, permissive, strict, transitive)
to their own package.
Then visitor.d and visitors looked a bit strange, so I moved visitor.d to visitors/package.d

As there is also astXXXX, dmd.ast.visitors would work too.

CC @RazvanN7

[dlang-bot]

Thanks for your pull request, @wilzbach!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

[ibuclaw]

Can this just be kept in toplevel? I'd be ok with a rename to visitors.h

[wilzbach]

Sure, I wasn't fully sure about this anyways (that's why I kept this change as a separate commit).

[WalterBright]

A little exposition on how this improves encapsulation would be nice. It's hard to tell from the diffs.

[wilzbach]

A little exposition on how this improves encapsulation would be nice. It's hard to tell from the diffs.

This group all the various visitors in the codebase neatly together, s.t. they are easy to find for an reader looking to learn more about DMD's visitors:

• the dmd.visitors module package is now the go-to place on the documentation and can e.g. contain a class hierarchy diagram (see below) and further explanations
• it highlights that dmd.visitors.strict is currently unused

Hierarchy in dmd.visitors

extern (C++) class ParseTimeVisitor(AST)

• extern (C++) class Visitor : ParseTimeVisitor!ASTCodegen
  • extern (C++) class StoppableVisitor : Visitor
• extern (C++) class SemanticTimePermissiveVisitor : Visitor
  • extern (C++) class SemanticTimeTransitiveVisitor : SemanticTimePermissiveVisitor
• class PermissiveVisitor(AST): ParseTimeVisitor!AST
  • ParseTimeTransitiveVisitor(AST) : PermissiveVisitor!AST
    • StrictVisitor(AST) : ParseTimeVisitor!AST

(simplified)

There's probably a lot more on @RazvanN7's mind.

[PetarKirov]

Add "src/dmd/visitors/package.d"

[PetarKirov]

And also parsetime.

[wilzbach]

The reason why it didn't break, is because this package isn't used anymore. There's frontend now...

[PetarKirov]

BTW, the convention for naming modules composed of two or more words is to separate the parts with an underscore. So how about renaming parsetime to parse_time?

[wilzbach]

(rebased)

Andrei: There seems to be merit in putting the entire visitation hierarchy in a separate package.

@andralex @WalterBright so can we move forward here? It looks like the PR is blocked on your approval.

[WalterBright]

This PR is just the start of a cleanup series :)

Oh gawd, please no. No packages, no glorious shuffling everything around.

[WalterBright]

Let's close this.

I've pointed out several times a list of things that will substantively improve DMD's encapsulation, readability, maintainability, etc. I will prepare a better list and post it to the n.g. That's what we should be spending time on.

[wilzbach]

I've pointed out several times a list of things that will substantively improve DMD's encapsulation, readability, maintainability, etc. I will prepare a better list and post it to the n.g. That's what we should be spending time on.

Fair enough. Looking forward to the list!