[DoesNotReturn] warning is only reported in nullable-enabled context

[cston]

Warning CS8763 should be reported for F1() below:

using System.Diagnostics.CodeAnalysis;

class Program
{
    [DoesNotReturn] static void F1() { } // no warning

#nullable enable
    [DoesNotReturn] static void F2() { } // warning CS8763
}

namespace System.Diagnostics.CodeAnalysis
{
    public class DoesNotReturnAttribute : Attribute { }
}

[RikkiGibson]

I think the behavior in the issue description is expected. DoesNotReturn is not respected by control flow analysis, definite assignment, etc. Is there some design notes/documentation indicating otherwise?

[jcouv]

Moved the issue to csharplang since it's a language design question.

[jcouv]

Added topic to LDM backlog (when convenient)

[333fred]

@jcouv like Rikki said, I don't understand what we're trying to ask here. This is the expected behavior of all nullable flow analysis attributes: what else are you expecting to see?

[jcouv]

@333fred Yes, the current behavior is by current design. The ask is to change this design ;-)

The proposed change and expectation are described in OP. Unlike other nullability attributes, this one doesn't have anything to do with nullability.
If we explore this, there are some other additions we should discuss too (effect on definite assignment). For example, I think this warning would fall out from this proposal:

F1();
int i = 0; // warning CS0162: Unreachable code detected

[RikkiGibson]

This seems about equally sound as definite assignment of out parameters. There's no way to guarantee that a method with an out parameter from metadata never reads the argument before assigning it, or that the method assigns it at all.

void M()
{
    int x;
    SomeLibrary.M(out x);
    x.ToString(); // could still be uninitialized?
}

[333fred]

Unlike other nullability attributes, this one doesn't have anything to do with nullability

It only has to do with nullability. No other part of the language or compiler pays attention to any of these attributes. It does not affect definite assignment in any fashion. I guess I'm not sure what has changed in the past 2 years that we feel we need to revisit this again.

[333fred]

@jcouv did you have a response to my question above?

[jcouv]

@333fred It's a question for Chuck who's raising the issue. Personally, I am sympathetic to the proposal and simply having more experience with the attributes is a sufficient reason to revisit.

[cston]

It only has to do with nullability.

The attribute indicates only that the method does not return which seems independent of nullability, and the warning "CS8763: A method marked [DoesNotReturn] should not return" does not mention nullability, so it's surprising the warning is only reported in a #nullable enable context.

[Youssef1313]

I can see this both ways.

if (x is null)
{
    ADoesNotReturnMethod();
}

_= x.ToString() // no warning. This is the original intent of the attribute.

However, from another perspective, it's just a "DoesNotReturn" where I want the compiler to warn me if it's used while the method does return. So whether or not the compiler is using it for nullability analysis is just one side of it.

[RikkiGibson]

Are we comfortable introducing a new error in implementations when DoesNotReturn is applied, but the method does return? That's what would be necessary to meet "definite assignment" standards, but the shipped behavior is only to warn.

[jcouv]

Discussed in LDM 10/13/2021: in short, keep as-is

[CyrusNajmabadi]

Discussed in LDM 10/13/2021: in short, keep as-is

In long: we are more interested in investing our efforts into a more powerful, generalized, solution like never/bottom types: #538

[333fred]

Notes: https://github.com/dotnet/csharplang/blob/main/meetings/2021/LDM-2021-10-13.md#revisiting-doesnotreturn