ETLCPP / etl

Embedded Template Library
https://www.etlcpp.com
MIT License
2.24k stars 392 forks source link

Setting Error-Handler Callback at Compile-time does not compile #933

Closed RafaelLaya closed 3 months ago

RafaelLaya commented 3 months ago

Hello!

First of all, thanks a lot for maintaining this awesome library

I'm having an issue. According to the documentation https://www.etlcpp.com/error_handler.html we should be able to set the error handlers at compile time. Following this closely, this is what a small snippet would look like:

#define ETL_LOG_ERRORS

#include <iostream>
#include <etl/error_handler.h>

void free_function(const etl::exception& e)
{
  std::cout << "The error was " << e.what() << " in " << e.file_name() << " at " << e.line_number() << "\n";
}
etl::error_handler::set_callback<free_function>();

int main(void)
{
  std::cout << "compiles" << std::endl;
}

However, this does not compile

$ g++ -std=c++20 mytest.cpp -o mytest && ./mytest
mytest.cpp:10:50: error: expected constructor, destructor, or type conversion before ‘;’ token
   10 | etl::error_handler::set_callback<free_function>();

Any idea what's the problem here? In the implementation there's some template magic, but I can't figure out what the problem is. The error message sounds more like a syntax error. I also found tests on the project, but the tests they're only for setting the error callback at runtime, so I suspect it might've worked and then broke at some point during some re-factor

Thanks!

jwellbelove commented 3 months ago

I'll take a look tomorrow.

RafaelLaya commented 3 months ago

Thanks! Definitely appreciate it

In the meanwhile I've been playing with this for the past hour. It seems like the problem is that the compiler thinks this is trying to define a function etl::error_handler::set_callback<free_function>();. I'm guessing because it really isn't producing an output for a global variable to get constructed with (can't just call some functions to "just happen", must be something being initialized or constructed, but probably need to dig into the standard to be sure). Thus I tried the following attempt at a workaround:

#define ETL_LOG_ERRORS

#include <iostream>
#include "error_handler.h"

static void free_function(const etl::exception& e)
{
  std::cout << "GOOD" << std::endl;
}

// return an integer so that we can call set_callback() in global scope
static int set_etl_callback() {
    etl::error_handler::set_callback<free_function>();
    return 0;   
}
// Call and assign to some dummy variable
static int b = set_etl_callback();

int main(void)
{
  std::cout << "compiles" << std::endl;
  auto exc = etl::exception("hello", __FILE__, __LINE__);
  etl::error_handler::error(exc);
}

This seems to work and prints:

compiles
GOOD
jwellbelove commented 3 months ago

As you found, the reason it doesn't work in your first example is that you are trying to call set_callback from outside a function, as if it were a declaration.

You can just set it in main or some other function.

#define ETL_LOG_ERRORS

#include <iostream>
#include <etl/error_handler.h>

void free_function(const etl::exception& e)
{
  std::cout << "Exception: '" << e.what() << "' at line " << e.line_number() << std::endl;
}

int main(void)
{
  etl::error_handler::set_callback<free_function>();

  std::cout << "compiles" << std::endl;
  auto exc = etl::exception("hello", __FILE__, __LINE__);
  etl::error_handler::error(exc);
}
jwellbelove commented 3 months ago

The way that errors are thrown within the ETL is by using the ETL macros.

#define ETL_LOG_ERRORS

#include <iostream>
#include <etl/error_handler.h>

void free_function(const etl::exception& e)
{
  std::cout << "Exception: '" << e.what() << "' at line " << e.line_number() << std::endl;
}

class Exception : public etl::exception
{
public:

  Exception(string_type file_name_, numeric_type line_number_)
    : exception("hello", file_name_, line_number_)
  {
  }
};

int main(void)
{
  etl::error_handler::set_callback<free_function>();

  int i = 3;

  ETL_ASSERT(i != 3, ETL_ERROR(Exception));
}
RafaelLaya commented 3 months ago

Makes sense. Thanks for confirming!

Should we close this issue?

Alsk perhaps we want to update the docs? I think what threw me off is that both the comments above set_callback() in the source code and the examples in the website mention this being a compile time call. Which kind of made me not pay attention to the fundamentals until I realized the problem

RafaelLaya commented 3 months ago

Oh and yeah the etl::exception() call just was the shortest way for me to write a quick snippet of what I was trying to do

jwellbelove commented 3 months ago

I'll check the documentation to try clear up any errors/confusion.