Remove debugreturn.h and machinery around preventing returns

[jkoritzinsky]

Remove last usages of our debug return macros and add a comment in our coding guidelines about returns shouldn't be used.

[Copilot]

Pull Request Overview

The PR removes the legacy debugreturn machinery from VM and interpreter code, deletes the debugreturn.h file, and adds a cautionary note in the exceptions documentation about not returning inside EX_CATCH blocks.

• Remove all #include "debugreturn.h" and related #undef return hacks
• Delete debugreturn.h and purge DEBUG_ASSURE_NO_RETURN_* macros from exception macros
• Add a reminder in docs/design/coreclr/botr/exceptions.md about avoiding returns between EX_CATCH and EX_END_CATCH

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/coreclr/vm/interpexec.cpp	Removed hack around debugreturn.h that broke constexpr usage
src/coreclr/vm/fcall.h	Dropped include of debugreturn.h
src/coreclr/vm/clrex.h	Removed SetupFinally() declaration
src/coreclr/vm/clrex.cpp	Deleted long explanatory comments about unwind behavior
src/coreclr/interpreter/stackmap.cpp	Removed hack undef for return from debugreturn.h
src/coreclr/interpreter/compiler.cpp	Same removal of debugreturn.h hack
src/coreclr/inc/gcinfotypes.h	Removed #undef return hack from header
src/coreclr/inc/ex.h	Cleared out DEBUG_ASSURE_NO_RETURN_* invocations in macros
src/coreclr/inc/debugreturn.h	Deleted entire file
src/coreclr/inc/contract.h	Dropped include of debugreturn.h
src/coreclr/inc/clrhost.h	Dropped include of debugreturn.h
docs/design/coreclr/botr/exceptions.md	Added note cautioning against returns inside EX_CATCH blocks

Comments suppressed due to low confidence (2)

src/coreclr/vm/clrex.h:184

• The declaration of SetupFinally() was removed, but any calls or references to this method in implementation files will now break. Please ensure that its usages are also deleted or updated accordingly to avoid build errors.

void SetupFinally();

docs/design/coreclr/botr/exceptions.md:47

• [nitpick] Consider relocating this guidance into the official coding guidelines document for exception macros, ensuring it’s discoverable alongside related style rules and preventing it from becoming overlooked in implementation-specific docs.

A function should not return from between an `EX_CATCH` usage and an `EX_END_CATCH` usage. Doing so will effectively swallow the exception.

[dotnet-policy-service]

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

[janvorli]

@jkoritzinsky I can see the DEBUG_ASSURE_NO_RETURN_XXX macros used at more places than your PR touches. exceptmacros.h, frames.h and olevariant.cpp.

[jkoritzinsky]

I can't find them (at least in olevariant.cpp with full-text search and a build succeeds on my local machine). Where are you seeing them?

[janvorli]

Oh, I am sorry, I was looking at a local checkout of the repo and haven't realized it was for v9.0.5.

[jkotas]

Doing so will effectively swallow the exception.

Why is it a problem? For example, if I write the following:

bool DoStuffNoThrow()
{
    EX_TRY
    {
        DoStuff();
    }
    EX_CATCH
    {
        return false;
    }
    EX_END_CATCH(SwallowAllExceptions);

     return true;
}

I naturally expect the method to swallow the exception and return false if there is one. SwallowAllExceptions is redundant in this case, but it should not affect correctness.

Or are there other negative side-effects if I do that?

[jkoritzinsky]

My main concern here was I wanted docs to point to in case someone uses a something like RethrowTransientExceptions but has an early return.

[jkotas]

Should we get rid of the argument for EX_END_CATCH macro and make rethrowing a statement to make it more obvious that it is part of the regular flow and returning before that won't execute it?

    EX_CATCH
    {
        return false;
        RethrowTransientExceptions;
    }
    EX_END_CATCH;

Some existing places do it like that already, For example:

runtime/src/coreclr/vm/clsload.cpp

Line 1127 in 15c5141

RethrowTerminalExceptions;

[jkoritzinsky]

That's a good idea. I'll do that.

[jkoritzinsky]

Updated the usage. Looks a lot cleaner to me (and easier to understand in my opinion).

[jkotas]

Thanks

[jkoritzinsky]

/ba-g failures unrelated

[jkoritzinsky]

/ba-g browser wam problem