GDScript: "while true:" block with no "break" statements inside and at least one "return" should count as a return for the purpose of "code paths return a value"

[BenLubar]

Describe the project you are working on

Porting a card game to Godot, but the specific type of game is irrelevant for this proposal.

Describe the problem or limitation you are having in your project

Best way I can explain this is a screenshot of what currently happens with a slightly contrived code example:

func count_number_of_lines(file: FileAccess) -> int:
    var lines := 0
    while true:
        lines += 1

        if file.eof_reached():
            return lines

        file.get_line()

An actual example of a function in my project this would affect is 83 lines long.

```gdscript static func parse(fh: FormatHelper, version: CardDef.EncodingFormat) -> CardFilter: match version: CardDef.EncodingFormat.CARD_0, CardDef.EncodingFormat.CARD_1: push_error("card format version %d: filters not supported for this version" % [version]) return null CardDef.EncodingFormat.CARD_2, CardDef.EncodingFormat.CARD_4: var rank_tribe := fh.read_uint8() var rank := (rank_tribe >> 4) as RankDef.Rank var tribe := (rank_tribe & 15) as TribeDef.Tribe var legacy_name := "" if tribe == TribeDef.Tribe.LEGACY_CUSTOM: legacy_name = fh.read_stringvar() return make_and([make_rank(rank), make_tribe(tribe, legacy_name)]) CardDef.EncodingFormat.CARD_5: var first := true var tag := fh.read_uint8() var tribe_filters: Array[CardFilter] = [] var negated_tribe_filters: Array[CardFilter] = [] var rank_filters: Array[CardFilter] = [] var tp_filters: Array[CardFilter] = [] while true: match tag >> 4: 0b0000: match tag & 15: 0b0000: for filter in negated_tribe_filters: filter.negate = true var all_tribes := make_and(tribe_filters + negated_tribe_filters) var any_rank := make_or(rank_filters) var any_tp := make_or(tp_filters) return make_and([all_tribes, any_rank, any_tp]) 0b0001: if not first: push_error("card format version %d: unhandled filter tag %d" % [version, tag]) return null return make_card(fh.read_uvarint()) _: push_error("card format version %d: unhandled filter tag %d" % [version, tag]) return null 0b0001: var tribe := (tag & 15) as TribeDef.Tribe var legacy_name := "" if tribe == TribeDef.Tribe.LEGACY_CUSTOM: legacy_name = fh.read_stringvar() tribe_filters.append(make_tribe(tribe, legacy_name)) 0b0010: var tribe := (tag & 15) as TribeDef.Tribe var legacy_name := "" if tribe == TribeDef.Tribe.LEGACY_CUSTOM: legacy_name = fh.read_stringvar() negated_tribe_filters.append(make_tribe(tribe, legacy_name)) 0b0011: var rank := (tag & 15) as RankDef.Rank assert(rank <= RankDef.Rank.TOKEN) rank_filters.append(make_rank(rank)) 0b0100: match tag & 15: 0b0000: var tp := fh.read_svarint() tp_filters.append(make_tp(tp)) 0b0001: if not first: push_error("card format version %d: unhandled filter tag %d" % [version, tag]) return null var filter := CardFilter.new() filter.type = Type.LEGACY_TARGETED_CARD return filter _: push_error("card format version %d: unhandled filter tag %d" % [version, tag]) return null first = false tag = fh.read_uint8() # unreachable return null CardDef.EncodingFormat.CARD_6: breakpoint # TODO return null _: push_error("card format version %d: unhandled filter version" % [version]) return null ```

Describe the feature / enhancement and how it helps to overcome the problem or limitation

This proposal would remove the need for a dummy return statement at the end of the function with a note that it's actually unreachable.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

In the exact situation where:

1. A code path within a function contains a while loop with a condition of the boolean literal true.
2. The while block does not contain any break statements. (breaks in nested loops are ignored for this, but breaks in other nested blocks like if are counted)
3. There is at least one return statement within the loop (including in nested loops).

The code validator would treat the while true: block as an arbitrary return statement from within it for the purpose of determining whether all code paths return a value.

If this enhancement will not be used often, can it be worked around with a few lines of script?

Yes, the point of this proposal is to avoid having to end a function with unreachable code. The workaround (which is the current situation in its entirety) is to add an unreachable return statement of the correct type and optionally annotate it with a comment, like so:

# unreachable
return null

Is there a reason why this should be core and not an add-on in the asset library?

This is a change to the GDScript code validator in the editor.

[AThousandShips]

This isn't guaranteed to ever return, see the "halting problem", and I'd say it's bad practice generally to write functions like this

But it comes down to the level of analysis and how complex we want to make it, and how much it's worth it

Almost all of the time this kind of function is not correct, the so unless we also add analysis for potential dangerous loops I don't think we need to treat this case special, and it's a good indicator to the user that the code is something you should pay attention to

Tl;Dr; Either, IMO, we should have a static analysis feature that earns about potentially unsafe loops as well, or this edge case should be treated no different than other halting problem cases

[BenLubar]

Solving the halting problem is not the purpose of the "not all code paths return a value" warning. For example, this function has no warnings despite the fact that code execution will not resume after it runs:

func exit_the_game() -> String:
    get_tree().quit()
    return "unreachable"

The purpose of the warning is to make sure that all paths where the function can return a value are returning an explicitly specified value and not the null that reaching the end of a function without an explicit return statement would return.

In this specific case, it is absolutely possible to determine that the function cannot return a value without hitting an explicit return statement, using the rules I specified.

It's quite common to have a loop with an exit condition that needs to be checked between the start and the end of an iteration.

[AThousandShips]

I would consider an infinite loop that isn't guaranteed to exit would be considered a code path that doesn't return, but the solution wouldn't be to add a return statement after the loop, but the code in question isn't guaranteed to ever return, and especially static analysis can't check that, so I think the lines are a bit blurry

What's the difference between your code and:

var do_exit = false

func count_number_of_lines(file: FileAccess) -> int:
    var lines := 0
    while true:
        lines += 1

        if do_exit:
            return lines

        file.get_line()

But I can agree that the loop could be treated as a complete path, but I'd say it should be done carefully as loop analysis is a messy question (though granted static loop analysis in c++ does handle this safely)

See this thread for some context:

• https://github.com/godotengine/godot/issues/86521#issuecomment-1869547074

[BenLubar]

This function should also not result in the warning being discussed because there is no path through the function even assuming every branch is reachable where the function reaches the end without an explicit return statement.

func count_number_of_lines(_file: FileAccess) -> int:
    while true:
        pass

These equivalent functions in other languages also result in no warnings. Making the condition of the loop a variable does result in a warning or compile error:

// Go 1.23
func foo() int {
    for {}
}

// gcc 14.2 -Wall -Wextra -Werror -pedantic
int foo() {
    while (true);
}

// Rust 1.81.0
fn foo() -> i32 {
    loop {}
}

[Mickeon]

CC @dalexeev

A bit neutral on this. If it's an easy fix I welcome it, but if it requires re-engineering the existing parser, it's not worth it in my opinion.

[dalexeev]

This is possible to solve, but requires more advanced control flow static analysis. See also:

• godotengine/godot#71763
• godotengine/godot#86521
• godotengine/godot#92341

• 8994

The error "Not all code paths return a value" should only be produced if there is a block with no outgoing edges and no return statement. In this example, the control flow graph looks like this:

The only block with no outgoing edges (block 3) has a return statement.

The question also arises whether we should ignore unreachable/dead code without return statement.

[Xtarsia]

func count_number_of_lines(file: FileAccess) -> int:
    var lines := 0
    while true:
        lines += 1

        if file.eof_reached():
            return lines

        file.get_line()
    return ERROR

regardless of coding practice, wouldn't adding this return resolve the problem for OP?

I wouldnt expect flow analysis to check the conditions of the while loop.