fix Issue 23586 - DMD forgets a variable was just declared

[ibuclaw]

The grammar/relationship of labelled statements to case statements are:

LabeledStatement:
    Identifier :
    Identifier : Statement

Statement:
    EmptyStatement
    NonEmptyStatement
    ScopeBlockStatement

NonEmptyStatement:
    NonEmptyStatementNoCaseNoDefault
    CaseStatement
    CaseRangeStatement
    DefaultStatement

CaseStatement:
    case ArgumentList : ScopeStatementList(opt)

ScopeStatementList:
    StatementListNoCaseNoDefault

StatementListNoCaseNoDefault:
    StatementNoCaseNoDefault
    StatementNoCaseNoDefault StatementListNoCaseNoDefault

StatementNoCaseNoDefault:
    EmptyStatement
    NonEmptyStatementNoCaseNoDefault
    ScopeBlockStatement

So the code:

Bar:
case 0:
    auto y = 7;
    return y;

Should result in a tree that looks like (as per the grammar rules):

LabeledStatement (
    Identifier(Bar) : Statement (
        CaseStatement (
            ArgumentList(0) : ScopeStatementList(
                DeclarationStatement(auto y = 7),
                ReturnStatement(y),
            )
        )
    )
)

However, instead it's currently being parsed as:

LabeledStatement (
    Identifier(Bar) : Statement (
        CaseStatement (
            ArgumentList(0) : ScopeStatementList(
                DeclarationStatement(auto y = 7)
            )
        )
    )
)
ReturnStatement(y)

Because both case and default case statements check whether ParseStatementFlags.curlyScope is in effect to determine whether to only parse one statement, or a list of statements. This flag is removed when a labelled statement is being parsed - forward the flag on so that it will be correctly picked up.

[dlang-bot]

Thanks for your pull request, @ibuclaw!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	23586	major	DMD forgets a variable was just declared.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#14747"

[ibuclaw]

Initial failures are due to Statement.hasCode returning true when it sees a LabelStatement.

However...

dmd/compiler/src/dmd/statement.d

Lines 290 to 326 in c5d192d

	extern (C++) final class HasCode : StoppableVisitor
	{
	alias visit = typeof(super).visit;
	public:
	override void visit(Statement s)
	{
	stop = true;
	}
	override void visit(ExpStatement s)
	{
	if (s.exp !is null)
	{
	stop = s.exp.hasCode();
	}
	}
	override void visit(CompoundStatement s)
	{
	}
	override void visit(ScopeStatement s)
	{
	}
	override void visit(ImportStatement s)
	{
	}
	override void visit(CaseStatement s)
	{
	}
	override void visit(DefaultStatement s)
	{
	}
	}

If default and case statements are assumed to have no executable code, it seems odd that labelled statements aren't treated the same way - in both case and label statements, it's the enclosed statement that has code, not the case/label itself.

[thewilsonator]

Should this go to stable?

[ibuclaw]

Should this go to stable?

On the one hand, I'd prefer if this had a little more battle testing done, and it's not actually fixing any regression. On the other, it does seem to be a small localized fix.

[thewilsonator]

Well your choice. Merge when you'r made your decision.

[ibuclaw]

The next beta is scheduled to be released on the 1st anyway.