Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add debug break override support #277

Closed
bnason-nf opened this issue Aug 21, 2019 · 6 comments
Closed

Add debug break override support #277

bnason-nf opened this issue Aug 21, 2019 · 6 comments

Comments

@bnason-nf
Copy link
Contributor

Hi @onqtam,

I'm using doctest on a platform that doesn't support the exisiting DOCTEST_BREAK_INTO_DEBUGGER() and isDebuggerActive() implementations. Would you mind adding something like this to support customizing these?

diff --git a/doctest/doctest.h b/doctest/doctest.h
index 3c12557..3a6e247 100644
--- a/doctest/doctest.h
+++ b/doctest/doctest.h
@@ -357,6 +357,7 @@ DOCTEST_MSVC_SUPPRESS_WARNING(26444) // Avoid unnamed objects with custom constr
 #define DOCTEST_GLOBAL_NO_WARNINGS_END() DOCTEST_CLANG_SUPPRESS_WARNING_POP

 // should probably take a look at https://github.com/scottt/debugbreak
+#ifndef DOCTEST_BREAK_INTO_DEBUGGER
 #ifdef DOCTEST_PLATFORM_MAC
 #define DOCTEST_BREAK_INTO_DEBUGGER() __asm__("int $3\n" : :)
 #elif DOCTEST_MSVC
@@ -369,6 +370,7 @@ DOCTEST_GCC_SUPPRESS_WARNING_POP
 #else // linux
 #define DOCTEST_BREAK_INTO_DEBUGGER() ((void)0)
 #endif // linux
+#endif // !DOCTEST_BREAK_INTO_DEBUGGER

 // this is kept here for backwards compatibility since the config option was changed
 #ifdef DOCTEST_CONFIG_USE_IOSFWD
@@ -3844,6 +3846,9 @@ namespace detail {
         return 0;
     }

+#ifdef DOCTEST_IS_DEBUGGER_ACTIVE
+    bool isDebuggerActive() { return DOCTEST_IS_DEBUGGER_ACTIVE(); }
+#else
 #ifdef DOCTEST_PLATFORM_MAC
     // The following function is taken directly from the following technical note:
     // http://developer.apple.com/library/mac/#qa/qa2004/qa1361.html
@@ -3876,6 +3881,7 @@ namespace detail {
 #else
     bool isDebuggerActive() { return false; }
 #endif // Platform
+#endif // !DOCTEST_IS_DEBUGGER_ACTIVE

     void registerExceptionTranslatorImpl(const IExceptionTranslator* et) {
         if(std::find(getExceptionTranslators().begin(), getExceptionTranslators().end(), et) ==

Thanks,
Benbuck

@onqtam
Copy link
Member

onqtam commented Aug 22, 2019

I won't be able to get to this in the near future, and I would much rather have support for the different compilers/platforms than to just have another config option - doctest should provide this out of the box.

Here are 2 things I've saved for inspection for this feature:

If a Pull Request materializes it will get merged! The only requirement is that the doctest_fwd.h part doesn't include any headers.

@bnason-nf
Copy link
Contributor Author

bnason-nf commented Aug 22, 2019

Hi @onqtam,

For the isDebuggerActive() check, the platform I mentioned uses a proprietary API for this that is covered by a non-disclosure agreement, so I'm not sure if supporting all platforms out of the box makes sense for a case like that. What do you think?

Additionally, the debugbreak library you linked to does not support this platform, which uses the __debugbreak() intrinsic but not the MSVC compiler. It's of course trivial to add an ifdef to either doctest or the debugbreak library to support this, but neither would currently work out of the box.

Thanks,
Ben

@onqtam
Copy link
Member

onqtam commented Aug 22, 2019

I just read your patch and that is not a bad idea - I'll add it to the dev branch right now, but I can't promise a release date for an official version.

What's the target platform? (out of curiosity)

@bnason-nf
Copy link
Contributor Author

Hi @onqtam,

I'm specifically working on PlayStation right now, but in general I work on all the game consoles, and except for XBox which uses the Windows tools and APIs, all of the other game consoles do different things for APIs and compilers, so this issue applies concerns basically any non-XBox game console.

Thanks,
Benbuck

@bnason-nf
Copy link
Contributor Author

Oh, and there is no hurry for this, I've patched it locally already.

Thanks,
Benbuck

onqtam pushed a commit that referenced this issue Aug 22, 2019
@onqtam
Copy link
Member

onqtam commented Sep 22, 2019

releasing 2.3.5

@onqtam onqtam closed this as completed Sep 22, 2019
onqtam pushed a commit that referenced this issue Sep 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants