Run-Time Check Failure #2

[SeppJ]

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

@SeppJ würdest Du bitte meinen Code oben mal zerreißen?

Nicht mehr heute Abend Erinner' mich morgen, falls ich es nicht selber tu!

[Swordfish]

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

Nicht mehr heute Abend Erinner' mich morgen, falls ich es nicht selber tu!

¡Si Señor!

nicht ankreiden daß alles was einen dynamischen vector ausmacht (resize, reserve, ...) fehlt ... erstmal grundgerüst. Noob muss noch üben.

[Schlangenmensch]

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

Ich glaube, das kommt tatsächlich daher, dass das in C oder C++ tatsächlich eine Herausforderung darstellen kann. Gerade wegen der komplizierteren Speicherverwaltung, Exception Safety, Rule of 3/5/0 usw.
In Java ist es total langweilig, eine LinkedList zu schreiben. Was soll man dabei lernen?

Ich musste trotzdem im Studium eine in Java schreiben

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

Wenn man's strikt idiomatisch macht, ist die LinkedList auch in C++trivial, weil man dann zur Verbindung Smartpointer nimmt, wodurch Speicherverwaltung entfällt und es - wie in Java - letztlich nur darauf ankäme, die Punkte richtig zu verbinden.

Wobei man grade bei Listen und Bäumen da etwas aufpassen muss, wegen rekursivem Destruktor Aufruf, was zu einem stack overflow führen kann.

[SeppJ]

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

@SeppJ würdest Du bitte meinen Code oben mal zerreißen?

Sieht schick aus. Stilistisch würde mich interessieren, wie du die const-Accessors implementieren würdest: Copy&Paste? Nutzung der non-const Member? Anders? (Das ist echtes Interesse, denn ich weiß nicht, welcher Stil dafür momentan "in" ist)

[Swordfish]

@SeppJ ich würde (werde) es so machen:

#include <cstddef>
#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:
	explicit
	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 (reinterpret_cast<pointer>(data) + i) value_type{ value };
	}

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

	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].~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)       noexcept { return reinterpret_cast<pointer>(data)[index]; }
	const_reference operator[](size_type index) const noexcept { return reinterpret_cast<pointer>(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 reinterpret_cast<pointer>(data)[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); }
};

operator[]() copy 'n paste weil ... es ist nur ein einzeiler und wirklich bis auf zweimal const dasselbe.
at() über eine Hilfsfunktion weil genug Zeilen um bei copy 'n paste messy zu werden.
Hat jemand andere Ideen?

[Swordfish]

Der

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

vector_t(std::size_t size = 0, value_type const &value = value_type{})

kann als conversion constructor missbraucht werden oder? Sollte der explicit sein?

[hustbaer]

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

...
kann als conversion constructor missbraucht werden oder? Sollte der explicit sein?

Ganz entschieden: ja!

[Swordfish]

@hustbaer ok, geändert. Hast Du sonst noch etwas?

[hustbaer]

@Swordfish

Hab grad nicht viel Zeit, was mir schnell aufgefallen ist:

• 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.
• Es kann sein dass du bei den anderen Stellen wo du reinterpret_cast<pointer>(data) + i machst noch std::launder brauchst. Vielleicht kann @Columbo uns sagen ob es nötig ist oder nicht
  Ansonsten liesse sich das vermeiden indem du das Member zu einem T* machst. Wenn erstmal alles fertig gekonstruiert ist, also nach der placement-new Schleife, ist es ja ein Zeiger auf hintereinanderliegende T*. Müsste also mMn. OK sein.
• Exception-safe machen oder asserten (static_assert(std::is_nothrow_copy_constructible...)

Die ganzen Wiederholungen von reinterpret_cast<pointer>(data) + i finde ich auch nicht schön. Bloss ist da wieder die const Sache doof, man bräuchte dann gleich wieder zwei Hilfsfunktionen
(Fiele aber auch alles weg wenn du nen T* Member machst.)

Ansonsten finde ich die Benamsung und den Stil stellenweise komisch, aber das is dann wieder ein anderes Thema

[Swordfish]

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

Ansonsten finde ich die Benamsung und den Stil stellenweise komisch, aber das is dann wieder ein anderes Thema

mach ruhig weiter ... gibs mir ^^

[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.