⚠ This page is served via a proxy. Original site: https://github.com
This service does not collect credentials or authentication data.
Skip to content

Conversation

@guneshsn1
Copy link
Contributor

Description

I have updated the Assertion.h and Assertion.cpp files which is located in OvDebug

Related Issue(s)

Fixes #627

Review Guidance

I have changed the old assert type with the std::source_location type which is a new feature that only works in only C++20

Screenshots/GIFs

I don't have any.

Checklist

  • My code follows the project's code style guidelines
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated the documentation accordingly
  • My changes don't generate new warnings or errors

Copy link
Member

@adriengivry adriengivry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for submitting this PR!

For this issue, I was thinking of something much simpler.
Overload targets C++20, so having a non-C++20 version isn't necessary here.

I was actually thinking of something much simpler, like:

Assertion.h

/**
* @project: Overload
* @author: Overload Tech.
* @licence: MIT
*/

#pragma once

#include <source_location>
#include <string>

#ifdef NDEBUG
#define OVASSERT(condition, message) static_cast<void>(0)
#else
#define OVASSERT(condition, message) \
	static_cast<bool>(condition) ? \
	static_cast<void>(0) : \
	OvDebug::_Assert(#condition, message, std::source_location::current());
#endif

namespace OvDebug
{
	void _Assert(
		const char* p_expression,
		const std::string_view p_message,
		const std::source_location& p_location
	);
}

Assertion.cpp

/**
* @project: Overload
* @author: Overload Tech.
* @licence: MIT
*/

#include <OvDebug/Assertion.h>
#include <OvDebug/Logger.h>

#include <cassert>
#include <format>

#if defined(_MSC_VER)
#   define OV_DEBUG_BREAK() __debugbreak()
#elif defined(__clang__) || defined(__GNUC__)
#   define OV_DEBUG_BREAK() __builtin_trap()
#else
#   define OV_DEBUG_BREAK() std::abort()
#endif

void OvDebug::_Assert(const char* p_expression, const std::string_view p_message, const std::source_location& p_location)
{
	OVLOG_ERROR(std::format(
		"Assertion failed: {}\n"
		"  Expression: {}\n"
		"  Function: {}\n"
		"  Location: {}:{}", 
		p_message, p_expression, p_location.function_name(), 
		p_location.file_name(), p_location.line()));
	OV_DEBUG_BREAK();
}

Which in practice would produce this sort of output:

[ERROR] 2025-12-08_18-42-56 Assertion failed: x must be positive
  Expression: x > 0
  Function: void OvEditor::Panels::MenuBar::InitializeSettingsMenu()
  Location: src/OvEditor/Panels/MenuBar.cpp:107

with a use case like that:

int x = -1;
OVASSERT(x > 0, "x must be positive");

@guneshsn1
Copy link
Contributor Author

Okey I am taking care of that now.

@guneshsn1 guneshsn1 closed this Dec 9, 2025
@adriengivry
Copy link
Member

adriengivry commented Dec 9, 2025

You don't need to close the PR, you can just push your changes to your updated-assert branch and it will show up here

@guneshsn1 guneshsn1 reopened this Dec 9, 2025
@guneshsn1
Copy link
Contributor Author

Oh, okay thanks. I didn't know that

@guneshsn1
Copy link
Contributor Author

I think it is done now

Copy link
Member

@adriengivry adriengivry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Let's wait for the build machine to validate that it compiles properly on all platform.

@adriengivry adriengivry changed the title Updated assert Updated assert to be more descriptive Dec 9, 2025
@adriengivry adriengivry added the Refactoring Something that needs a refactoring label Dec 9, 2025
@guneshsn1
Copy link
Contributor Author

Looks good to me! Let's wait for the build machine to validate that it compiles properly on all platform.

Thanks for all the help tho. I will try to contribute this project as much as i can.

@adriengivry
Copy link
Member

@guneshsn1 all checked passed ✅
Merging now, thanks for your contribution!

@adriengivry adriengivry merged commit 12fc181 into Overload-Technologies:main Dec 9, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Refactoring Something that needs a refactoring

Development

Successfully merging this pull request may close these issues.

Better assertions with std::source_location

2 participants