On 5/23/2021 7:25 PM, Nicholas Wilson wrote:
> Directories.

The #1 problem isn't directories, it's "every module imports every other module" that leaves one with nowhere to start.

We currently have:

  dmd
  dmd/root
  dmd/backend

I regularly fend off attempts to have dmd/root import files from dmd, and dmd/backend import files from dmd. I recently had to talk someone out of having dmd/backend import files from dmd/root.

In other words, a failure of encapsulation.

Let's look at one example, picked more or less because I've looked at it recently, dmd/target.d. The reason for its existence is to abstract target information. It's imports are:

  import dmd.argtypes_x86;
  import dmd.argtypes_sysv_x64;
  import core.stdc.string : strlen;
  import dmd.cond;
  import dmd.cppmangle;
  import dmd.cppmanglewin;
  import dmd.dclass;
  import dmd.declaration;
  import dmd.dscope;
  import dmd.dstruct;
  import dmd.dsymbol;
  import dmd.expression;
  import dmd.func;
  import dmd.globals;
  import dmd.id;
  import dmd.identifier;
  import dmd.mtype;
  import dmd.statement;
  import dmd.typesem;
  import dmd.tokens : TOK;
  import dmd.root.ctfloat;
  import dmd.root.outbuffer;
  import dmd.root.string : toDString;

If I want to understand the code, I have to understand half of the rest of the compiler. On a more abstract level, why on earth would a target abstraction need to know about AST nodes? At least half of these imports shouldn't be here, and if they are, the code needs to be redesigned.

Recently I needed some target information in the ImportC lexer, and it would have been so easy to just import dmd.target. But then that drags along all the imports that I've really tried to avoid importing into the lexer.

Iain came up with a clever solution to use a template parameter.

Note that Phobos suffers terribly from this disease (everything ultimately imports everything else), which makes it very hard to understand and debug.

Fixing this is not easy, it requires a lot of hard thinking about what a module *really* needs to do. But each success at eliminating an import makes it more understandable.

Creating a false hierarchy (an implied relationship that is instantly defeated by the imports) of files won't fix it.

A good rule of thumb is:

    *** Never import a file from an uplevel directory ***

Import sideways and down, never up.

On Monday, 24 May 2021 at 02:56:14 UTC, Walter Bright wrote:
> On 5/23/2021 7:25 PM, Nicholas Wilson wrote:
>> Directories.
>
> The #1 problem isn't directories, it's "every module imports every other module" that leaves one with nowhere to start.
>
> We currently have:
>
>   dmd
>   dmd/root
>   dmd/backend
>
> I regularly fend off attempts to have dmd/root import files from dmd, and dmd/backend import files from dmd. I recently had to talk someone out of having dmd/backend import files from dmd/root.
>
> In other words, a failure of encapsulation.

This is a _completely_ orthogonal problem.

The symptoms are completely orthogonal, although easily confused: failure of encapsulation makes _reasoning_ about the _interconnectedness_ of code difficult, failure to package makes _exploration_ and _enumeration_ of code (files, functions, classes, data structures) more difficult.
The solutions, however are cross enabling: we can implement and _enforce_ policies like say "AST node implementing modules should not import semantic analysis modules" with reasonable confidence iff we have all the AST modules in one place and all the semantic analysis modules in one place.

The symptoms of failure of encapsulation I'm going to assume you are well aware of.
The symptoms of failure to use packages are as follows:
 * the sheer number of filed in src/dmd make it impossible to remember what each file is for.  This problem is compounded by the fact that many files have names that do not describe well what they do _especially_ to newcomers. Principle offending example `ob`. Compare with names like `filecache`.
* it is impossible to determine at a glance what files are related to each other:
is `foreachvar.d` an AST node?, what about `dcast.d`? (No and No)
Whats the difference between `glue.d` and `gluelayer.d`?
is `visitor.d`, `transitivevisitor.d`, `strictvisitor.d` `parsetimevisitor.d` and `permissivevisitor.d` a complete list of the module public visitor modules? (No)
Which of `cond.d` and `staticcond.d` is the AST node for a static condition? What does the other file do? (`cond.d`, semantic analysis)
What files do semantic analysis? Which files declare AST nodes? Which files interface with the backend (and subsequently are not part of LDC or GDC)?
Where is DMD's entry point?


> snip example
> Fixing this is not easy, it requires a lot of hard thinking about what a module *really* needs to do. But each success at eliminating an import makes it more understandable.

Fixing the lack of directory issue requires only to think about what a module _is_ i.e. what package it belongs to: driver/frontend (mars, errors etc) , lexer group (lex, parse, tokens etc), ast, semantic analysis, backend interfacing, backend, root.

> Creating a false hierarchy (an implied relationship that is instantly defeated by the imports)

You cannot seriously tell me with a straight face that e.g. AST, is not a hierarchy and should not be grouped together.

> of files won't fix [failure to encapsulate].

Indeed is fixes a different problem, but it makes fixing failure to encapsulate much easier.

> A good rule of thumb is:
>
>     *** Never import a file from an uplevel directory ***
>
> Import sideways and down, never up.

Indeed. However you can't to much of that with just

>   dmd
>   dmd/root
>   dmd/backend

On 5/23/21 10:56 PM, Walter Bright wrote:
> I recently had to talk someone out of having dmd/backend import files from dmd/root.

One problem with that is code duplication. There are two types OutBuffer in frontend and Outbuffer in backend that are 95% identical, yet duplicated. Recent improvements (two distinct) will need to be duplicated to the other, which is clearly not a good way to go.

How to address this problem?

I think all of us looking to improve dmd's architecture would be well served by reading this book:

https://amazon.com/gp/product/0135974445/

Really close, cover to cover. A lot of the principles in that book are either applied with good results (sadly not as often as one would hope), or not, with the expected poor outcome, in dmd's codebase. For example, this:

> A good rule of thumb is:
> 
>     *** Never import a file from an uplevel directory ***
> 
> Import sideways and down, never up. 

is an approximate formulation of a subset of Dependency Inversion Principle:

https://en.wikipedia.org/wiki/Dependency_inversion_principle

On 5/24/21 1:15 AM, Nicholas Wilson wrote:
> Indeed is fixes a different problem, but it makes fixing failure to encapsulate much easier.

I think the best first step is to add `private` to the codebase. This is cheap to get into and informs any future refactoring. I find it confusing that people push for massive reorganization for years, but won't bother to create 50 line PRs that add `private` appropriately.

On 5/23/2021 10:15 PM, Nicholas Wilson wrote:
> This is a _completely_ orthogonal problem.

It's the same problem.

D's support for modules and packages is literally designed around matching the hierarchy of the source files.

Shuffling files around accomplishes nothing when every module imports every other module.

On Monday, 24 May 2021 at 06:58:48 UTC, Walter Bright wrote:
> On 5/23/2021 10:15 PM, Nicholas Wilson wrote:
>> This is a _completely_ orthogonal problem.
>
> It's the same problem.
>
> Shuffling files around accomplishes nothing when every module imports every other module.

Did you read _literally nothing else_ that I wrote?

Let me quote myself again so that you don't miss it:

> The symptoms are completely orthogonal, although easily confused: failure of encapsulation makes _reasoning_ about the _interconnectedness_ of code difficult, failure to package makes _exploration_ and _enumeration_ of code (files, functions, classes, data structures) more difficult.

Putting the modules into packages fixes EXACTLY the problem of horrible experience with exploration and enumeration. It explicitly does not fix failure of encapsulation because it is a  _completely_ orthogonal set of symptoms.

> D's support for modules and packages is literally designed around matching the hierarchy of the source files.

Yes, and?

On Monday, 24 May 2021 at 02:56:14 UTC, Walter Bright wrote:
> On 5/23/2021 7:25 PM, Nicholas Wilson wrote:
>> Directories.
>
> The #1 problem isn't directories, it's "every module imports every other module" that leaves one with nowhere to start.
>
> We currently have:
>
>   dmd
>   dmd/root
>   dmd/backend
>
> I regularly fend off attempts to have dmd/root import files from dmd, and dmd/backend import files from dmd. I recently had to talk someone out of having dmd/backend import files from dmd/root.
>
> In other words, a failure of encapsulation.
>
> Let's look at one example, picked more or less because I've looked at it recently, dmd/target.d. The reason for its existence is to abstract target information. It's imports are:
>
>   import dmd.argtypes_x86;
>   import dmd.argtypes_sysv_x64;
>   import core.stdc.string : strlen;
>   import dmd.cond;
>   import dmd.cppmangle;
>   import dmd.cppmanglewin;
>   import dmd.dclass;
>   import dmd.declaration;
>   import dmd.dscope;
>   import dmd.dstruct;
>   import dmd.dsymbol;
>   import dmd.expression;
>   import dmd.func;
>   import dmd.globals;
>   import dmd.id;
>   import dmd.identifier;
>   import dmd.mtype;
>   import dmd.statement;
>   import dmd.typesem;
>   import dmd.tokens : TOK;
>   import dmd.root.ctfloat;
>   import dmd.root.outbuffer;
>   import dmd.root.string : toDString;
>
> If I want to understand the code, I have to understand half of the rest of the compiler. On a more abstract level, why on earth would a target abstraction need to know about AST nodes? At least half of these imports shouldn't be here, and if they are, the code needs to be redesigned.
>
> Recently I needed some target information in the ImportC lexer, and it would have been so easy to just import dmd.target. But then that drags along all the imports that I've really tried to avoid importing into the lexer.
>
> Iain came up with a clever solution to use a template parameter.
>
> Note that Phobos suffers terribly from this disease (everything ultimately imports everything else), which makes it very hard to understand and debug.
>
> Fixing this is not easy, it requires a lot of hard thinking about what a module *really* needs to do. But each success at eliminating an import makes it more understandable.
>
> Creating a false hierarchy (an implied relationship that is instantly defeated by the imports) of files won't fix it.
>
> A good rule of thumb is:
>
>     *** Never import a file from an uplevel directory ***
>
> Import sideways and down, never up.

A good enhancement to the language would be adding some sort of module declaration that just states the admitted import packages or modules. I know that could be done by an external tool, but I feel that this one is a common problem.