Discussion: Conditional non-void methods

[alrz]

If a method accepts a single parameter with of the return-type, you should be able apply [Conditinal].

[Conditional("DEBUG")]
public static T AssertNotNull<T>(this T value) where T : class? { .. }

Such method would not be required to be static and allowed be an extension.

Currently we have to make a local, plus at some places it woudn't be possible to get out of expression context.

[canton7]

What would it return if DEBUG isn't set?

It sounds like what you want is something like this:

public static T AssertNotNull<T>(this T value) where T : class?
{
#if DEBUG
    if (value == null)
    {
        throw new ArgumentNullException();
    }
#endif
    return value;
}

That's definitely small enough to get inlined by the JIT when DEBUG isn't set (and probably even when it is).

[huoyaoyuan]

What would it return if DEBUG isn't set?

He should mean that the conditional expression should return it's parameter as-is when DEBUG isn't set.

It sounds like what you want is something like this:

The method will depend callee's project configuration, not caller's. So it must be shared by source.

I think the usage is not strong enough for a language feature.

[SamPruden]

This feels a bit too magic to me. Why would returning the input parameter be a sensible default for all methods that match that shape? I can see that it works for assertions, but I'm not sure that's always going to be a sensible assumption.

If you want something like this, it might make more sense to add more configuration to [Conditional]. You could have things like [Conditional("DEBUG", orReturnParameter: "value")] and [Conditional("DEBUG", orReturnsDefault: true)].

[PathogenDavid]

I feel like something like conditional call replacement might be easier to comprehend and be more flexible than trying to cover this one use case. IE:

[ConditionalReplacement("DEBUG", "AssertNotNull_DEBUG")]
public static T AssertNotNull<T>(this T value)
    where T : class?
    => value;

public static T AssertNotNull_DEBUG<T>(this T value)
    where T : class?
{
    Assert(value != null);
    return value;
}

---

All that being said, you're probably better off trusting the JIT's ability to remove unused branches. If the JIT can prove a condition is never true, it'll eliminate the branch. This is true for both static readonly fields and properties which return constants.

So you're probably better off with something like this:

static readonly bool AssertsAreEnabled = CheckAssertsEnabled();

private static bool CheckAssertsEnabled()
    // Look if the entry assembly is marked as having asserts enabled, or look for a tagged method or something.
    => Assembly.GetEntryAssembly().GetCustomAttribute<EnableAssertsAttribute>() != null;

public static T AssertNotNull<T>(this T value)
    where T : class?
{
    if (AssertsAreEnabled)
    { Assert(value != null); }

    return value;
}

Although like I said, this does require having some trust in the JIT. (And wouldn't work in AOT scenarios.) However, it's a solution that works today!

The other solution is just shipping separate debug and release versions of your library and using Canton's approach, which is slightly annoying but isn't so bad once you get it set up.