Run-Time Check Failure #2

[hustbaer]

@Swordfish
Naja die Benamsung ... data_size, data_capa - von was solls denn sonst sein als von data? Und unnötige Abkürzungen is auch nicht so meins. => Einfach size und capacity würd's denke ich gut tun.
(BTW: Wenn man es für nötig hält ein sehr kurzes Kommentar neben eine Variable zu schreiben um zu erklären was die bedeutet, dann sollte man sich IMO überlegen ob man nicht einen besseren Namen findet.)

Sternderl beim Variablennamen is auch nicht so meins. Spätestens wenn man so wie du einrückt und dann noch const Variablen hat schaut das dann komisch aus

	size_type   data_size;
	size_type   data_capa;
	std::byte  *data;
	std::byte  *const foo;

oder halt

	size_type          data_size;
	size_type          data_capa;
	std::byte  *       data;
	std::byte  *const  foo;

Das is irgendwie alles nix.

Dann lieber einfach

	size_type data_size;
	size_type data_capa;
	std::byte* data;
	std::byte* const foo;

Die nette Tabellarische Einrückung sieht zwar gut aus, is aber umständlich beizubehalten/wiederherzustellen wenn man Änderungen macht. Wenn man einen Formatter hat der das automatisch so macht OK. Wenn nicht dann verzichte ich lieber darauf - keine Lust gegen den Auto-Formatter zu kämpfen (hab meine IDE so eingestellt dass sie automatisch formatiert nach "paste" bzw. wenn man "Teile" abschliesst (also ";" oder "}" tippt).

Und size_type i{}... echt jetzt? Also 0 kann man IMO schon einfach mal hinschreiben. Der relevante Teil der Sache ist ja nicht dass man einen default-initialisierten size_type haben will sondern dass man will dass 0 drin steht. Dass das zufällig das selbe ist sollte einen mMn. nicht dazu veranlassen die 0 wegzulassen. Und {} für die Initialisierung von Dingen wie Integers, Zeigern, Strings etc. liest sich für mich auch komisch.

[Swordfish]

@hustbaer sagte in Run-Time Check Failure #2:

• Bin mir nicht sicher ob new std::byte[data_capa * sizeof value_type] für Alignment ausreichend ist. Ich glaub schon aber müsste ich nachgucken.
• reinterpret_cast<pointer>(data) + i beim placement-new ist technisch gesehen falsch: du darfst keinen Zeiger machen auf etwas was es (noch) nicht gibt. D.h. du solltest beim placement new eher data + (i * sizeof T) machen. Der Parametertyp ist ja void* wenn ich mich richtig erinnere.

Habe ich nicht mit einem data vom Typ T* das auf byte[N] zeigt dasselbe Problem, daß ich einen Pointer habe auf etwas - ein T - das es nicht gibt?

@hustbaer sagte in Run-Time Check Failure #2:

Exception-safe machen oder asserten (static_assert(std::is_nothrow_copy_constructible...)

vector_t(vector_t<T> const &other)
: data_size {},
  data_capa { other.data_size },
  data      { reinterpret_cast<pointer>(new std::byte[data_capa * sizeof value_type]) }
{
	try {
		for (size_type i{}; i < other.data_size; ++i, ++data_size)
			new (data + i) value_type{ other.data[i] };
	}
	catch (...) {
		for (size_type i{}; i < data_size; ++i)
			data[i].~value_type();  // wer mir einen werfenden dtor unterjubelt ist selbst schuld.
		delete[] reinterpret_cast<std::byte>(data);
		throw;
	}
}

reicht? (typeof data = T*)

[hustbaer]

@Swordfish sagte in Run-Time Check Failure #2:

Habe ich nicht mit einem data vom Typ T* das auf byte[N] zeigt dasselbe Problem, daß ich einen Pointer habe auf etwas - ein T - das es nicht gibt?

Nicht wenn du zuerst alle Ts konstuierst und erst dann den T* mittels des Casts erzeugst.

vector_t(vector_t<T> const &other)
: data_size {},
  data_capa { other.data_size },
  data      { reinterpret_cast<pointer>(new std::byte[data_capa * sizeof value_type]) }
{
	try {
		for (size_type i{}; i < other.data_size; ++i, ++data_size)
			new (data + i) value_type{ other.data[i] };
	}
	catch (...) {
		for (size_type i{}; i < data_size; ++i)
			data[i].~value_type();  // wer mir einen werfenden dtor unterjubelt ist selbst schuld.
		delete[] reinterpret_cast<std::byte>(data);
		throw;
	}
}

reicht? (typeof data = T*)

Nö, reicht nicht. Du zerstörst ja zu viele Items (mindestens eins, denn die letzte Exception die fliegen kann ist beim Konstruieren des letzten Items, d.h. das darf sowieso nie zerstört werden).
Eher so in der Art (draft, ungetestet):

vector_t(size_t n) {
    struct Helper {
        size_t i = 0;
        std::byte* data;

        Helper(size_t n) : data(new std::byte[data_capa * sizeof value_type]) {}

        void construct(size_t n) {
            for (; i < n; i++) {
                new (data + (i * sizeof T)) value_type{};
            }
        }

        ~Helper() {
            while (i > 0) {
                i--;
                reinterpret_cast<T*>(data + (i * sizeof T))->~T();
            }
            delete [] data;
        }
    };

    Helper h{n};
    h.construct(n);
    data = reinterpret_cast<T*>(helper.data);
    data_size = n;
    data_capa = n;
    helper.data = nullptr;
    helper.i = 0;
}

Die weit einfachere Variante ist zu asserten dass die Dinger noexcept konstruierbar/... sind.

[Swordfish]

@hustbaer sagte in Run-Time Check Failure #2:

Nö, reicht nicht. Du zerstörst ja zu viele Items (mindestens eins, denn die letzte Exception die fliegen kann ist beim Konstruieren des letzten Items, d.h. das darf sowieso nie zerstört werden).

Ich stehe gerade auf diversen Leitungen. Nochmal bitte mit mehr Worten.

[hustbaer]

@Swordfish Sorry, hab deinen Code zu schnell überflogen und das ++data_size übersehen.
Wenn du gleich auf die unnötige i Variable verzichtet hättest hätt ichs vermutlich bemerkt

[Swordfish]

Gut. Also doch kosher. Dann muss ich nur noch

@hustbaer sagte in Run-Time Check Failure #2:

Nicht wenn du zuerst alle Ts konstuierst und erst dann den T* mittels des Casts erzeugst.

auf mich wirken lassen

[Swordfish]

@SeppJ Wie siehst Du das mit Object-lifetime und der pointercasterei?

[SeppJ]

Ich hatte mich während des ersten Lesens einen Moment lang gefragt, ob das alles auch 100% mit near- und far-Pointern funktionieren würde. Wahrscheinlich tut es das sogar nicht einmal. Dann sagte ich mir, dass ich auf so etwas einfach keine Rücksicht nehmen würde, und es auch niemandem in einem Review ankreiden würde, das nicht zu tun. Der Sonderfall solcher alter Systeme ist sicher auch der einzige, wo diese super-strikte Standardauslegung relevant wäre, über die ihr gerade diskutiert. Wie gesagt, würde ich darauf keine Rücksicht nehmen. Irgendwann muss man auch mal loslegen und Code schreiben, anstatt über irrelevante Sonderfälle nachzudenken.

[hustbaer]

Ich hab ehrlich gesagt keine Ahnung wie relevant die Regel in diesem Zusammenhang ist
Das müsste man so einen Chandler fragen. Hab aber leider grad keinen zur Hand.

[Swordfish]

@hustbaer sagte in Run-Time Check Failure #2:

Chandler

?

@Columbo ?

der aktuelle Stand (back to byte as underlying):

#include <stdexcept>
#include <string>

template<typename T>
class vector_t
{
public:
	using value_type       = T;
	using pointer          = T*;
	using reference        = T&;
	using const_reference  = T const&;
	using size_type        = std::size_t;

private:
	size_type  data_size;
	size_type  data_capa;  // capacity
	std::byte *data;

public:
	vector_t(std::size_t size = 0, value_type const &value = value_type{})
	: data_size { size },
	  data_capa { size },
	  data      { new std::byte[data_capa * sizeof value_type] }
	{
		for (size_type i{}; i < data_size; ++i)
			new (data + i * sizeof value_type) value_type{ value };
	}

	vector_t(vector_t<T> const &other)
	: data_size {},
	  data_capa { other.data_size },
	  data      { new std::byte[data_capa * sizeof value_type] }
	{
		try {
			for (size_type i{}; i < other.data_size; ++i, ++data_size)
				new (data + i * sizeof value_type) value_type{ *reinterpret_cast<pointer>(other.data + i * sizeof value_type) };
		}
		catch (...) {
			for (size_type i{}; i < data_size; ++i)
				reinterpret_cast<pointer>(data + i * sizeof value_type)->~value_type();
			delete[] data;
			throw;
		}
	}

	friend void swap(vector_t<T> &a, vector_t<T> &b) noexcept
	{
		using std::swap;
		swap(a.data_size, b.data_size);
		swap(a.data_capa, b.data_capa);
		swap(a.data, b.data);
	}

	vector_t& operator=(vector_t<T> other) noexcept
	{
		swap(*this, other);
		return *this;
	}

	virtual ~vector_t()
	{
		for (size_type i{}; i < data_size; ++i)
			reinterpret_cast<pointer>(data + i * sizeof value_type)->~value_type();
		delete[] data;
	}

	size_type size()     const noexcept { return data_size; }
	size_type capacity() const noexcept { return data_capa; }
	
	      reference operator[](size_type index)       { return *reinterpret_cast<pointer>(data + index * sizeof value_type); }
	const_reference operator[](size_type index) const { return *reinterpret_cast<pointer>(data + index * sizeof value_type); }

private:
	reference checked_access(size_type index)
	{
		if (data_size <= index)
			throw std::out_of_range{ "vector_t::at(" + std::to_string(index) + ")" };
		return this->operator[](index);
	}

public:
	      reference at(size_type index)       { return checked_access(index); }
	const_reference at(size_type index) const { return const_cast<vector_t*>(this)->checked_access(index); }
};

@hustbaer Die dinger heißen data_size und data_capa um eine kollision mit size() und capacity() zu vermeiden. Ich bin kein Fan von prefixes oder postfixes.

[hustbaer]

@Swordfish sagte in Run-Time Check Failure #2:

Chandler

?

https://research.google/people/ChandlerCarruth/
Musst du mal auf YouTube suchen, der macht extrem gute Vorträge. Also nicht nur technisch interessant sondern auch sehr unterhaltsam weil sehr gut vorgetragen.

Die dinger heißen data_size und data_capa um eine kollision mit size() und capacity() zu vermeiden. Ich bin kein Fan von prefixes oder postfixes.

Ah, verstehe. Naja, ich bin ein grosser Fan von m_. Weil's so praktisch ist. Für viele Dinge. Wie man z.B. hier sieht

[Swordfish]

@hustbaer Geh' p0593r6 lesen und verklicker mir das so daß ich es verstehe und lass' uns nicht über sinnlosigkeiten wie data_ vs. m_ diskutieren. Deal? ^^

[hustbaer]

@Swordfish
Im Prinzip heisst das (u.A.) dass du die in 3.2. angeführten Operationen (malloc, ::operator new, new std::byte[N], new char[N] etc.) verwenden kannst, und den so gewonnenen Speicher dann einfach verwenden als wäre da ein Array von T-Objekten. (Bzw. auch ein einziges T Objekt, ganz wie es das Programm eben benötigt.)
Allerdings ohne dass der Konstruktor eines T-Objekts aufgerufen würde.

D.h. du darfst z.B. Zeiger auf das Objekt bzw. Array formen und Zeigerarithmetik damit machen. Also z.B.

char* mem = new char[2 * sizeof T]; // Aktiviert die magische auto-Objektifizierung
// hier entsteht automagisch das T[] das nötig ist um dem Programm definiertes Verhalten zu geben,
// allerdings ohne die Ts zu konstruieren
T* arr = reinterpret_cast<T*>(mem); // OK
new (&arr[0]) T(args...); // OK
new (&arr[1]) T(args...); // OK
arr[0].useT(); // OK
arr[1].useT(); // OK
arr[1].~T(); // OK
arr[0].~T(); // OK

Wie man die ganze Gaudi (also den Speicher selbst) dann wieder freigeben soll wenn man new char[] verwenden hat, hab ich da drin allerdings nicht gefunden. Denn das ursprüngliche char Array existiert ja nun nicht mehr (?) nachdem man den Speicher als T[] verwendet hat. D.h. ein char Array an der Adresse freizugeben wäre UB. Aber ich schätze mal vermutlich per std::bless. Also:

std::bless(arr, 2 * sizeof T); // Die magische auto-Objektifizierung wieder aktivieren
// hier entsteht automagisch das char[] das nötig ist um dem Programm definiertes Verhalten zu geben
delete [] reinterpret_cast<char*>(arr); // OK

Bzw. je nachdem was das Standardkomitee entscheidet gibt's kein std::bless sondern man muss statt dessen placement-new eines byte-Arrays verwenden:

new(arr) std::byte[2 * sizeof T]; // Ändert keine Bytes weil ja kein Initializer, reaktiviert aber die magische auto-Objektifizierung
// hier entsteht automagisch das char[] das nötig ist um dem Programm definiertes Verhalten zu geben
delete [] reinterpret_cast<char*>(arr); // OK

[hustbaer]

Von daher wäre also vermutlich gut einen T* als Member zu verwenden statt eines std::byte*.

[Columbo]

@hustbaer Ich gehe davon aus, dass fuer char dieselben Regeln gelten (werden) wie fuer unsigned char, naemlich dass es Speicher fuer Objekte bereitstellt, und deshalb nicht zerstoert wird.

@Swordfish Ich wuerde es eher so angehen (das ist jetzt wirklich auf die Schnelle abgetippt, kann das spaeter rigoroser machen):

#include <memory>
#include <cstddef>

template<typename T>
class vector_t
{
public:
	using value_type       = T;
	using pointer          = T*;
	using const_pointer    = const T*;
	using reference        = T&;
	using const_reference  = T const&;
	using size_type        = std::size_t;

private:
	size_type  data_size;
	size_type  data_capa;  // capacity
	std::unique_ptr<std::byte[]> storage;

public:

    pointer data() noexcept {return std::launder(reinterpret_cast<pointer>(storage.get()));}
    const_pointer data() const noexcept {return std::launder(reinterpret_cast<const_pointer>(storage.get()));}

	vector_t(std::size_t size = 0, value_type const &value = value_type{})
	: data_size(size), data_capa(size),
	  storage(std::make_unique<std::byte[]>(data_capa * sizeof(value_type))) {
	    std::uninitialized_fill_n(data(), data_size, value);
	}

	vector_t(vector_t<T> const &other) : vector_t(other.begin(), other.end()) {}

	vector_t(std::initializer_list<value_type> ilist) : vector_t(ilist.begin(), ilist.end()) {}

    template <typename ForwardIt>
	vector_t(ForwardIt first, ForwardIt last)
	: data_size(std::distance(first, last)), data_capa(data_size),
	  storage(std::make_unique<std::byte[]>(data_capa * sizeof(value_type))) {
		std::uninitialized_copy_n(first, data_size, data());
	}

	friend void swap(vector_t<T> &a, vector_t<T> &b) noexcept {
		using std::swap;
		swap(a.data_size, b.data_size);
		swap(a.data_capa, b.data_capa);
		swap(a.data, b.data);
	}

	vector_t& operator=(vector_t<T> other) noexcept {
		swap(*this, other);
		return *this;
	}

	~vector_t() {
	    std::destroy_n(data(), size());
	}
	
          pointer begin()       noexcept {return data();}
          pointer end()         noexcept {return data() + size();}
	const_pointer begin() const noexcept {return data();}
	const_pointer end()   const noexcept {return data() + size();}

	size_type size()     const noexcept { return data_size; }
	size_type capacity() const noexcept { return data_capa; }
	
	      reference operator[](size_type index)       { return data()[index]; }
	const_reference operator[](size_type index) const { return data()[index]; }

private:
	reference checked_access(size_type index) {
		if (data_size <= index)
			throw std::out_of_range{ "vector_t::at(" + std::to_string(index) + ")" };
		return operator[](index);
	}

public:
	      reference at(size_type index)       { return checked_access(index); }
	const_reference at(size_type index) const { return const_cast<vector_t*>(this)->checked_access(index); }
};

#include <string>
#include <iostream>
int main() {
    vector_t<std::string> vec{"asdf", "Swordfish", "blub"};
    vector_t vec2 = vec;
    for (auto& s : vec2) std::cout << s << std::endl;
    
}

[*john 0]

@hustbaer sagte in Run-Time Check Failure #2:

T* arr = reinterpret_cast<T*>(mem); // OK

Nicht ok, an dieser Stelle ist ein static_cast ausreichend, da nur ein Zeiger in einen anderen Zeiger konvertiert wird! Also T* arr = static_cast<T*> (mem); so wäre es besser.

Wie man die ganze Gaudi (also den Speicher selbst) dann wieder freigeben soll wenn man new char[] verwenden hat, hab ich da drin allerdings nicht gefunden.

So wie Du es angefordert hast. D.h. bei new char[]mit delete[]auf ein char* Zeiger. Also

char * pc = new char[sizeof(T)*n];
T* pT = static_cast<T*>(pc);
// use it
pc = static_cast<char*>(pT); // pc hat noch immer den gleichen Wert, aber so geht es auch, wenn pc nicht mehr da wäre
delete[] pc;
`
Analog dazu mit 'malloc' und 'free' usw.

[Ein ehemaliger Benutzer]

Und warum würde man jetzt einen Pointer auf std::byte als unterliegenden Typen benutzen statt den Template-Parametern?

[Columbo]

@john-0 sagte in Run-Time Check Failure #2:

@hustbaer sagte in Run-Time Check Failure #2:

T* arr = reinterpret_cast<T*>(mem); // OK

Nicht ok, an dieser Stelle ist ein static_cast ausreichend, da nur ein Zeiger in einen anderen Zeiger konvertiert wird! Also T* arr = static_cast<T*> (mem); so wäre es besser.

Hoer auf so einen Schwachsinn zu faseln. Das ist offensichtlich nicht ausreichend.

@eigenartig sagte in Run-Time Check Failure #2:

Und warum würde man jetzt einen Pointer auf std::byte als unterliegenden Typen benutzen statt den Template-Parametern?

Weil das der Typ des Arrays ist, welches den Speicher bereitstellt?

[Ein ehemaliger Benutzer]

@Columbo sagte in Run-Time Check Failure #2:

Weil das der Typ des Arrays ist, welches den Speicher bereitstellt?

Ja gut. Warum denn nicht T[N] statt std::byte[N * sizeof(T)], damit spart man sich doch die ganze Zeigerarithmetik?

[Swordfish]

@eigenartig byte oder char als underlying type weil man nicht ständig Ts konstruieren und zerstören will nur weil sich der zu Verfügung stehende Speicher ändert (reserve(), shrink_to_fit(), ...).

---

@Columbo sagte in Run-Time Check Failure #2:

kann das spaeter rigoroser machen

Danke vielmals, reicht aber schon. Ich male dann später weiter wenn ich Zeit und Muße habe.