sheredom / utest.h

🧪 single header unit testing framework for C and C++
The Unlicense
834 stars 57 forks source link

Handle temporary strings in ASSERT_STREQ etc #127

Closed nickolaym closed 1 year ago

nickolaym commented 1 year ago

Consider such code

#include "utest.h"
#include <string>

UTEST(cpp_lifetime, std_strings) {
  ASSERT_STREQ(std::string{"hello"}.c_str(), "hello");
}

It causes warning (which should be treated as error!)

utest.h/test/lifetime.cpp:5:16: warning: object backing the pointer will be destroyed at the end of the full-expression [-Wdangling-gsl]
  ASSERT_STREQ(std::string("hello").c_str(), "hello");
               ^~~~~~~~~~~~~~~~~~~~
utest.h/test/../utest.h:990:26: note: expanded from macro 'ASSERT_STREQ'
    const char *xEval = (x);                                                   \

But passes.

Moreover,

struct PureString {
  std::string s_;
  PureString(const char* s) : s_(s) {}
  void erase() { for (size_t i = 0, n = s_.size(); i != n; ++i) s_[i] = '#'; }
  ~PureString() { erase(); }
  const char* c_str() const { return s_.c_str(); }
};

UTEST(cpp_lifetime, pure_strings) {
  ASSERT_STREQ(PureString("hello").c_str(), "hello");
}

causes no warnings at all. And it fails.

sheredom commented 1 year ago

The problem is that just including <string> in test framework makes the compile time 20x worse, its that bad a header. I don't want to add any specific support for std::string because of that.

nickolaym commented 1 year ago

The problem is not with std::string only.

Your code works with dangling values.

My example PureString may be rewritten with pure C strings:

struct PureString {
  char* s_;
  PureString(const char* s) : s_(strdup(s)) {}
  PureString(const PureString& ps) : s_(strdup(ps.s_)) {}
  ~PureString() { memset(s_, '#', strlen(s_)); free(s_); }
  const char* c_str() const { return s_; }
};
nickolaym commented 1 year ago

In C++, it is easy to fix it: just use lambda. In pure C, it is easy, too: just write an inline function.

Illustration: https://gcc.godbolt.org/z/zE5oeqWvh