https://issues.dlang.org/show_bug.cgi?id=22365

RazvanN <razvan.nitu1305@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |razvan.nitu1305@gmail.com

--- Comment #1 from RazvanN <razvan.nitu1305@gmail.com> ---
The chances of this bug getting fixed without a minimal reproducible  test case are small. Maybe you can use DustMite [1] to reduce it?

[1] https://dlang.org/blog/2020/04/13/dustmite-the-general-purpose-data-reduction-tool/

--

https://issues.dlang.org/show_bug.cgi?id=22365

--- Comment #2 from evilrat666@gmail.com ---
(In reply to RazvanN from comment #1)
> The chances of this bug getting fixed without a minimal reproducible  test case are small. Maybe you can use DustMite [1] to reduce it?
> 
> [1] https://dlang.org/blog/2020/04/13/dustmite-the-general-purpose-data- reduction-tool/

nope, unfortunately I wasn't able to reduce it yet, this is a complex library with tons of dependencies and it is happily reduces it down to 'import ;'.

I'll try again later, meanwhile I traced down the issue and ready to submit possible fix (I hope that's a real fix and not just symptoms mending) PR after I double check everything.

Here is an overview
-----------------------------------------
dmd issue 22365

calls:

  [1] semantic3.d:1369  Semantic3Visitor.visit(CtorDeclaration) at 1446
TryCatchStatemt generation

  [2] semantic3.d:208  Semantic3Visitor.visit(FuncDeclaration) at 595
statementSemantic(funcdecl.fbody, sc2) call

  [3] statementsem.d:148  statementSemantic(Statement, Scope*) at 145
StatementSemanticVisitor.accept()

  [4] statement.d:1580  TryCatchStatemt.accept (just in case)

  [5] statementsem.d:3936  StatementSemanticVisitor.visit(TryCatchStatemt) at
3957 semanticScope() <--- here, returns wrong scope

    [6] statementsem.d:4486  semanticScope(Statement, Scope*, Statement,
Statement, Statement) <--- push & pop the same scope?

    [7] statementsem.d:4473  semanticNoScope(Statement, Scope*) at 4468
statementSemantic() returns null which triggers assert above

        [8] statementsem.d:236  StatementSemanticVisitor.visit(CompoundStatent)
at 257-264 flatten() and then statementSemantic()

          [9] statementsem.d:4631  flatten(Statement, Scope*)  probably at 4720
if condition


specifically look:

[5] https://github.com/dlang/dmd/blob/96b959e27c690f5e3abf411f8032c60f4728c1c1/src/dmd/statementsem.d#L3936

[6] https://github.com/dlang/dmd/blob/96b959e27c690f5e3abf411f8032c60f4728c1c1/src/dmd/statementsem.d#L4497

so, at step 6 when using debbuger after semanticNoScope() restore s (null at
this moment) back to original s (from function argument), now it works as
expected

        > s = s.semanticNoScope(scd); // <- null
        > /* restore s */
    > scd.pop();
    > return s;

tracing it further...

[8] https://github.com/dlang/dmd/blob/96b959e27c690f5e3abf411f8032c60f4728c1c1/src/dmd/statementsem.d#L255

calling flatten() produces Statements[] array with single null element which is then returned as valid result here around line 420

    > if (cs.statements.length == 1)
    > {
    >   result = (*cs.statements)[0];
    >   return;
    > }

[9] https://github.com/dlang/dmd/blob/96b959e27c690f5e3abf411f8032c60f4728c1c1/src/dmd/statementsem.d#L4726

debug condition doesn't get picked up and returns that Statements[] array with single null element afterwards

    > if (cs.condition.include(sc))

can be fixed with adding else if check for null and else return 0 here https://github.com/dlang/dmd/blob/96b959e27c690f5e3abf411f8032c60f4728c1c1/src/dmd/statementsem.d#L4737



this doesn't happens when adding debug else condition to original problem source, or replacing debug with version (all/none) or anything else, so there is a logic issue related to debug ConditionStatement somewhere

--

https://issues.dlang.org/show_bug.cgi?id=22365

moonlightsentinel@disroot.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |moonlightsentinel@disroot.o
                   |                            |rg
           Hardware|x86_64                      |All
                 OS|Windows                     |All

--- Comment #3 from moonlightsentinel@disroot.org ---
Reduction:


alias DrawableRef = Ref!int;

class DrawableCache {
    DrawableRef _nullDrawable;

    this() {
        debug Log;
    }
}


struct Ref(T) {

    ~this() {
    }
}

Requires -preview=dtorfields with older compilers.

--

https://issues.dlang.org/show_bug.cgi?id=22365

--- Comment #4 from moonlightsentinel@disroot.org ---
Even smaller:


class DrawableCache {
    Ref _nullDrawable;

    this() {
        debug Log;
    }
}


struct Ref {

    ~this() {}
}

--

https://issues.dlang.org/show_bug.cgi?id=22365

Dlang Bot <dlang-bot@dlang.rocks> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |pull

--- Comment #5 from Dlang Bot <dlang-bot@dlang.rocks> ---
@MoonlightSentinel updated dlang/dmd pull request #13516 "Don't crash for conditionally empty body of TryStatement" fixing this issue:

- Fix 22365 - Don't crash for conditionally empty body of a TryStatement

  The body may end up empty / null after `statementSemantic` if it only
  contains conditionally compiled statements (`version`, `debug`, ...)
  whose conditions are not met.

  This only triggered for the `-preview=dtorfields` rewrite because it
  didn't wrap the empty body into an additional `ScopeStatement`. But it
  should be sufficient to check during `TryStatement` semantic instead
  of unconditionally allocating an additional node for this rare case.

https://github.com/dlang/dmd/pull/13516

--

https://issues.dlang.org/show_bug.cgi?id=22365

Dlang Bot <dlang-bot@dlang.rocks> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #6 from Dlang Bot <dlang-bot@dlang.rocks> ---
dlang/dmd pull request #13516 "Fix 22365 - Don't crash for conditionally empty body of a TryStatement" was merged into stable:

- 4b2b2af5c276f64334a8eb9daf0c00db83cb93d2 by MoonlightSentinel:
  Fix 22365 - Don't crash for conditionally empty body of a TryStatement

  The body may end up empty / null after `statementSemantic` if it only
  contains conditionally compiled statements (`version`, `debug`, ...)
  whose conditions are not met.

  This only triggered for the `-preview=dtorfields` rewrite because it
  didn't wrap the empty body into an additional `ScopeStatement`. But it
  should be sufficient to check during `TryStatement` semantic instead
  of unconditionally allocating an additional node for this rare case.

https://github.com/dlang/dmd/pull/13516

--