[SAOC 2024] - Fix Segmentation fault when compiling druntime Weekly update #3

[RazvanN]

On Monday, 7 October 2024 at 18:37:31 UTC, Johan wrote:
> On Sunday, 6 October 2024 at 22:25:30 UTC, Dennis wrote:
>> ## Fix Segmentation when Compiling druntime
>>
>> ### Task Accomplished
>> After moving the newScope function (which is a semantic 
>> import) from attrib.d to dsymbolsem.d, and turning it into a 
>> visitor. I encountered an error with the following description:
>> ```make -C druntime
>> make[2]: Entering directory '/home/dencomac/DL/dmd/druntime'
>> ../generated/linux/release/64/dmd -c -conf= -Isrc -Iimport -w 
>> -de -preview=dip1000 -preview=fieldwise -m64 -fPIC 
>> -preview=dtorfields -O -release -inline -version=Shared -fPIC 
>> -version=Shared -fPIC -I. -P=-I. src/core/stdc/errno.c 
>> -of../generated/linux/release/64/errno_c.o
>> make[2]: *** [Makefile:394: 
>> ../generated/linux/release/64/errno_c.o] Segmentation fault
>> make[2]: Leaving directory '/home/dencomac/DL/dmd/druntime'
>> make[1]: *** [Makefile:89: druntime] Error 2
>> make[1]: Leaving directory '/home/dencomac/DL/dmd'
>> make: *** [posix.mak:8: all] Error 2
>> ```
>> It became a challenge to me and to the reviewers.  It was a 
>> runtime error. Although the dmd compiler was built 
>> successfully, when druntime(core runtime library that provides 
>> essential low-level support for the D) was tested, it failed.
>
> Hi Dennis, all,
>
> I'm sorry to say, but I am not feeling very happy about this 
> project.
> It is a very invasive change to the compiler, that I feel 
> should not be undertaken by someone with relatively little 
> experience in D, let alone D compiler frontend development.
> What are the safety checks to prevent _any_ actual change in 
> compiler output?
> The test suite of the compiler (and druntime, and Phobos) is 
> *very* scant and lacks rigorous testing in many places. Which 
> means that you cannot rely on the testsuite to verify that 
> indeed no change happened to the compiler output. There is the 
> project builder (buildkite?) which expands the testsuite, 
> that's nice, but but still.
>
> Are there any extra checks done to see whether binary output of 
> the compiler has changed upon any PR submitted? (for a number 
> of large code bases?)
>
> -Johan

Hi Johan,

If you are arguing that the test suite is not good enough to 
catch breaking changes then that is an argument that can be 
brought up for literally any change that is brought to the 
compiler. In that regard I fail to see how this project is any 
different from any other modification.

With respect to the type of change: I have done literally tens or 
even hundreds of such refactorings in the past 6-7 years. From my 
experience: (1) 0 regressions or bugs were reported linked to 
that sort of changes (apart from some C++ header mismatches - but 
  I don't count those since those are not actually related to the 
compiler behavior); (2) if you break something while refactoring 
code - the test suite will catch it (if the compiler does not 
beat you up to it).

Now, with regards to Dennis's experience: I am his SAoC mentor 
and I'm carefully reviewing his PRs before merging so please rest 
assured that care is being taken with this work. The only reason 
that I've submitted this project to SAoC is because it is 
something trivial enough that most experienced compiler devs 
avoid doing and because it is a good way for someone to get 
accustomed to the compiler codebase.

Regards,
RazvanN