Contributing to DMD

Mike Franklin slavo5150 at yahoo.com
Thu May 24 01:00:18 UTC 2018


On Wednesday, 23 May 2018 at 14:25:44 UTC, Steven Schveighoffer 
wrote:

> Common questions I have when looking at dmd code:
>
> Where is this in the source?
> How do I know I'm in a function?
> How do I know I'm in a template?
> What module am I in?
> How do I know what has been run?
> What is the proper way to generate lowered code?

I hear you, I think most of us have been there.  Razvan also 
mentioned this in the beginning of his DConf talk.  The only 
documentation that I know of is 
https://wiki.dlang.org/DMD_Source_Guide.  I believe its very 
outdated, but probably still helpful.  You get to it from the 
from page of the wiki by going to "Compilers & Tools" --> "DMD".

I'll share how I got started, as I think it might serve as a path 
for others.
1.  I filed this issue about `static alias this`:  
https://issues.dlang.org/show_bug.cgi?id=17684
2.  Vladimir identified it as a regression, and posted the PR 
that broke it.
3.  The PR that broke it was super simple, so I figured if it was 
simple to break, it would be simple to fix.
4.  Yep, after a lot of grepping, it was pretty simple to fix: 
https://github.com/dlang/dmd/pull/7055
5.  It got a little euphoric about fixing this and that motivated 
me to do more.  And it just took off from there.

I recognize not everyone will have a simple fix like this to get 
them past the barrier to entry.  And fewer will get their PR 
reviewed and merged like mine was (Thanks Petar).

I actually learned a lot by studying other people's PRs, trying 
to learn what they were doing any why.  If you have a bug you'd 
like to fix, search through the open and closed PRs and see if 
you can find something similar.  Study it and see if you can 
extrapolate any understanding from it.

I also received a LOT of help from Iain when I attempted this 
ambitions fix:  https://github.com/dlang/dmd/pull/7079.  Again, 
few people will receive such valuable help, so I am very 
fortunate, but it may be a strategy to just try, knowing your fix 
is wrong, and hope in the review process you'll get the 
information you need to make it correct.  But you could also 
irritate a lot of people if such a PR shows little effort on your 
part.

A few questions I had when I got started:
Q.  How do I build the compiler?
A:  `make -f posix.mak -j4`

Q:  How do I run the test suite locally?
A:  `make -C test/ -f Makefile -j4`

Finally, if anyone needs help, please ask questions on the forum 
(General or Internals for compiler devs), slack, IRC, etc.  If 
you don't get an answer, it doesn't mean you're being ignored; 
not many people know how to answer such questions.

That all being said, here's the kicker which both Steven and 
Rikki alluded to:  We have a review bottleneck, not a 
contribution bottleneck.  If we could attract more talent with 
the domain expertise needed to certify a PR, then the 
contributions would increase organically.  That's a terrible 
reality, but I fear that if anyone heeds any of what I said and 
starts making PRs, their PRs will just rot in the PR queue 
waiting for a review.  Finding minuscule problems with a PR is 
easy, being able to say "yes, this is the right fix" is 
hard...very hard, and comes with consequences if you're wrong. 
Unfortunately, those with the expertise to certify difficult PRs 
tend to be very part-time reviewers, and sorry, I'm not one of 
them.

There are a few characteristics of a PR that will increase 
likelihood of review:
   * Has tests and passes the test suite
   * Is noncontroversial
   * Benefit is obvious (i.e. Fixes a bug, especially a 
regression, and is not an enhancement).  This is preventing me 
from certifying Rikki's PR.
   * Has clear explanation of the problem and the fix so anyone 
can understand it.  Not everyone has an intimate understanding of 
the problem like you, help them out.  This is what is preventing 
me from reviewing and certifying Steven's and Rikki's PRs.
   * It's simple.  The fewer lines of change, the better.

There are few characteristics of a PR that will decrease 
likelihood of a review:
   * No tests and/or test suite shows a red X
   * Is controversial or benefit is not obvious
   * Reviewer has to spend a lot of time studying the code to 
figure out why and what
   * Has a lot of refactoring changes that aren't needed for the 
actual fix

I was thinking about making a set of videos about how to get 
started contributing to the compiler, but I decided it wouldn't 
be a good idea until we can do something about our PR bottleneck; 
it'd probably just fill the PR queue making matters worse.

Anyway, that's my two-cents.  Take everything I've said with a 
grain of salt (as they say); I'm often wrong.

Mike


More information about the Digitalmars-d mailing list