Run-Time Check Failure #2



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



  • @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.



  • @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 🙂



  • @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? ^^



  • @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
    


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


  • Mod

    @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;
        
    }
    


  • @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.


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


  • Mod

    @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?



  • @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?



  • @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.



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

    @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(), ...).

    Stimmt, ja.



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

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

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

    Der Schwachsinn kommt von dir! Genau aus diesem Grund darf man an dieser Stelle reinterpret_cast nicht nutzen, weil es eben fundamentale Fehler überdeckt. Zeiger beliebigen Typs lassen sich mit static_cast konvertieren (Ergänzung: notfalls über zwei casts von T1* auf void* auf T2*), wenn das scheitert, dann wird versucht etwas anderes als einen Zeiger zu konvertieren, und dann läuft etwas ganz falsch.

    Beispiel warum man kein reinterpret_cast oder C-Casts nehmen sollte.

    class Foo {
    public:
            double x, y;
    };
    
    int main () {
            constexpr size_t size = 1000;
            size_t s = 0x0F00000000000000;
    
            // C Casts sind also sinnvoll?
            Foo* t = (Foo*)s;
    
            // man vergleiche vor allem diese beiden Zeilen miteinander
            Foo* p = reinterpret_cast<Foo*>(s);
            Foo* q = static_cast<Foo*>(s);
    
            //  man muss den Umweg über void* nehmen, aber es funktioniert
            char* sc = static_cast<char*>(malloc(sizeof(Foo)*size));
            void* pv = static_cast<void*>(sc);
            Foo* pF = static_cast<Foo*>(pv);
    }
    


  • @john-0 Für gewöhnlich deuten solche Dreieckscasts eher auf einen Hack hin.

    struct T {};
    struct U : T {};
    struct V : T {};
    
    U u;
    auto tPtr = static_cast<T*>(&u);
    auto vPtr = static_cast<V*>(tPtr); 
    

    Würdest du ja wohl auch kaum als valide bezeichnen, oder?

    Da wäre ein reinterpret_cast an der Stelle schon mal ein deutliches Warnsignal, dass eben einen ungewöhnlicher und potentiell gefährlichen cast vorliegt. Mit einem 3-way-static_cast kaschierst du das Wesentliche nur.



  • @DNKpp sagte in Run-Time Check Failure #2:

    Würdest du ja wohl auch kaum als valide bezeichnen, oder?

    Das Problem hier ist, dass Columbo ein Design gewählt hat, was den Konventionen von Container nicht entspricht. Üblicherweise verwenden Container Allokatoren um Speicher anzufordern, und eben keine HighLevel Funktionen. Die Arbeit das korrekt umzusetzen wollte sich Columbo wohl nicht machen, und es wäre für einen Anfänger etwas viel. Das Stichwort ist hier Rule of Five und die notwendigen Fallunterscheidungen, die man bezüglich der alloc_traits::propagate_on_container_copy_assigment, … beachten muss.

    Allokatoren allozieren bereits typsicher den Speicher und liefern Zeiger auf T zurück. D.h. im Container selbst muss nie ein Cast verwendet werden. Das Problem liegt dann in den Allokatoren, und diese fordern üblicherweise per Low Level Routinen Speicher an und diese liefern i.d.R. void* zurück. D.h. auch hier ist ein meistens ein static_castausreichend. I.d.R. deshalb, weil ich hier nicht irgend welche obskuren Lösungen ausschließen will, weil sonst wieder jemand Heureka ruft "John-0" hat was Falsches geschrieben.

    Da wäre ein reinterpret_cast an der Stelle schon mal ein deutliches Warnsignal, dass eben einen ungewöhnlicher und potentiell gefährlichen cast vorliegt.

    Columbo wählte unter diesen Rahmenbedingungen eine Lösung die reinterpret_cast erzwingt. Hätte er saubere Arbeit abgeliefert, wäre gar kein Cast notwendig gewesen. Hätte er LowLevel Funktionen gewählt, wäre es mit einem static_cast getan gewesen. Er gehört aber zu denjenigen hier im Forum ab, die beständig schreiben, dass man ja keine LowLevel Speicherverwaltung verwenden darf, weil das ja fehlerträchtig sei. Nur deshalb ist ein reinterpret_cast hier notwendig. Ergo, es ist eine Frage des Designs.

    Darüber hätte man sich sachlich austauschen können, nur das ist wohl nicht Columbos Weg.


  • Mod

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

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

    Würdest du ja wohl auch kaum als valide bezeichnen, oder?

    Das Problem hier ist, dass Columbo ein Design gewählt hat, was den Konventionen von Container nicht entspricht. [..blabla]

    @Swordfish Ich glaub er versucht Dein Design zu trashtalken.



  • @Columbo Ich glaub das ist mir piiiep.



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

    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()));}
    

    std::launder wurde doch u.a. gemacht damit man dem Compiler sagen kann dass an der Adresse jetzt ein neues T Objekt zu finden ist -- oder hab ich das falsch verstanden? Wäre das dann nicht eine ziemliche Optimierungs-Bremse?



  • @hustbaer Da wird gewaschen um dem Compiler zu verklickern daß er alles was er sich über die constness des Objekts zusammenreimen könnte vergessen muss da die Möglichkeit besteht daß sich das Objekt in der Zwischenzeit über einen non-const pointer geändert hat.


Anmelden zum Antworten