8000 Why should we allow leaking temporaries in C++23, since we can delete them with "deducing this&&"? · Issue #2276 · isocpp/CppCoreGuidelines · GitHub
[go: up one dir, main page]

Skip to content
Why should we allow leaking temporaries in C++23, since we can delete them with "deducing this&&"? #2276
@igormcoelho

Description

@igormcoelho

Sorry if this is not the right place to propose some ideas for cpp core guidelines, but I could not find any other place to begin a discussion. This proposal tries to create a new rule that prevents common dangling pointer errors with c++23 "deducing this".

In C++, it is typical to have dangling pointer errors in std::string code using c_str(), and it seems that tooling is able to warn them in some situations, like for std::string with clang Wdangling-gsl:

std::string f1() {
  std::string s{"abc1"};
  return s;
}
// ....
const char* s1 = f1().c_str(); // clang warns with Wdangling-gsl: Object backing the pointer will 
7035
be destroyed at the end of the full-expression

If user ignores the warning of Wdangling-gsl or does not use -fsanitize=address, then it may cause unexpected problems. It also seems to be hardcoded to detect only the case of c_str() from std::string, as it does not issue warnings on similar cases. Consider MyString:

struct MyString {
  std::unique_ptr<char[]> ptr;
  int n;
  explicit MyString(const char* s) {
    this->n = std::strlen(s);
    ptr = std::make_unique<char[]>(n + 1);
    std::strcpy(ptr.get(), s);
  }

  // this is equally dangerous, but will not trigger any warning!
  const char* c_str() { return ptr.get(); }

  int length() const { return n; }
};

MyString f2() {
  MyString s{"abc2"};
  return s;
}
// ...
const char *s2 = f2().c_str();  // no warning here! but problem is the same...

Since c++23 we have "deducing this" (https://wg21.link/P0847R7), I believe we are finally able to delete these dangerous dangling operations, when we are certain they will fail:

  // const char* c_str() { return ptr.get(); }
  const char* c_str(this MyString& self) { return self.ptr.get(); }
  const char* c_str(this MyString&&) = delete;

This way, this flawed code fails to compile:

  const char *s2 = f2().c_str(); // error: Call to deleted member function 'c_str'

I could not find such discussions in the P0847R7 paper itself, and also could not find any guideline here that incentives people to delete this&& methods, specially when resources are leaked.

For example, the length() operation does not seem to be so evil as c_str(), since it only leaks a copy of the size:

std::cout << "s2 len=" << f2().length() << std::endl;  // this is fine! s2 len=4

So, there's no need to delete ALL functions that are rvalued on this, only the ones that leaks internal resources. I found this on cpp core guidelines:

What to do with leaks out of temporaries? : p = (s1 + s2).c_str();

If this is still an open issue, maybe C++ std::string should implement this C++23 strategy for c_str(), and other similar cases, like vector iterators, etc, and while this does not happen, could core guidelines incentive this strategy? Or is there some unknown or serious drawback on enforcing that?

Another similar case that can be solved with this strategy is related to for-range, that also dangles:

class Resource {
public:
  const auto &items() { return data; }
private:
  std::vector<const char *> data{"A", "B", "C"};
};
// ....
  for (auto &x : getResource().items()) // PROBLEM!
    std::cout << x << std::endl;

A recent solution is to do use c++20 range init instead:

  for (auto r = getResource(); auto &x : r.items()) // Fine!
    std::cout << x << std::endl;

However, I believe the real problem here is the items() function that it's inherently flawed, and should disallow leaking internal resources when rvalued:

  const auto &items(this Resource &self) { return self.data; }
  const auto &items(this Resource &&) = delete;
  // const auto &items() { return data; }

This correctly generates an error, even with classic for-range:

  for (auto &x : getResource().items()) // error: Call to deleted member function 'items'
    std::cout << x << std::endl;

Thanks for your time, and I hope this advice can help others, or even improving the c++ STL someday.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

      0