Run-Time Check Failure #2


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



  • @Swordfish Und wieso sollte das nötig sein? Verstehe ich nicht. Das wäre dann ja genau so eine Optimierungs-Barriere die man in einem vector sicher nicht haben will.

    Also ich bin immer noch der Meinung dass es optimal wäre hier einen T* als Member zu verwenden. Weil man dann den ganzen launder Käse nicht braucht. Und so wie ich p0593r6 verstanden habe will man das auch erlauben - ob's jetzt im aktuellen Draft so steht oder nicht.



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

    @Swordfish Und wieso sollte das nötig sein?

    Du holst einen T const* mit data() const, danach holst Du einen T* mit data() und veränderst lustig die Elemente, dann holst Du Dir nochmal einen T const* mit data() const. Der Compiler dürfte ohne launder() davon ausgehen daß sich die Elemente hinter dem T const* nicht verändert haben.



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

    Der Compiler dürfte ohne launder() davon ausgehen daß sich die Elemente hinter dem T const* nicht verändert haben.

    Nein. Er darf nur davon ausgehen dass es immer noch das selbe Element ist. Aber da das Element selbst nicht const ist, darf er nicht davon ausgehen dass es sich nicht geändert haben kann.

    Ein Problem ergibt sich nur dann wenn Objekte erzeugt und zerstört und wieder erzeugt werden -- an der selben Adresse. Und genau da geht mir jetzt auch gerade ein Licht auf, denn genau das kann beim vector ja passieren.

    Also das Problem wäre nur hier:

    vector<T> vec;
    vec.push_back(1);
    int a = vec[0].constMember;
    vec.pop_back();
    vec.push_back(2);
    int b = vec[0].constMember;
    

    Hier ist vec[0] nicht mehr das selbe Ding wie vorher. Es hat den selben Typ und steht an der selben Stelle, aber es ist ein neues Objekt. Daher dürfen sich const Member geändert haben. Und genau das, also dass hier etwas zerstört und ein neues Etwas an der selben Stelle wieder erzeugt wurde muss man daher dem Compiler mitteilen.



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

    Der Compiler dürfte ohne launder() davon ausgehen daß sich die Elemente hinter dem T const* nicht verändert haben.

    Davon steht hier nichts. Allerdings ist es in dem hier verwendeten Kontext laut dieser Referenz sinnvoll, weil so man anzeigen kann, dass über diesen Zeiger nicht mehr auf nicht initialisierten Speicher zugreift, sondern die Objekte mit placement new konstruiert worden sind.



  • So kann man das ganz ohne Casts nutzen, wenn man schon Delegation nutzt.

    #include <memory>
    #include <cstddef>
    #include <iterator>
    
    template <typename T, class Allocator = std::allocator<T>>
    class SimpleStorage {
    public:
            using allocator_type    = Allocator;
            using alloc_traits              = typename std::allocator_traits<allocator_type>;
            using size_type                 = typename alloc_traits::size_type;
    
            allocator_type  alloc;
            size_type       c;
            T* p;
    
            ~SimpleStorage () {alloc.deallocate(p,c);}
            SimpleStorage (const size_type capacity, allocator_type& a) : c(capacity), p(alloc.allocate(c)) {}
            SimpleStorage (const size_type capacity) : alloc({}), c(capacity), p(alloc.allocate(c)) {}
            SimpleStorage (const SimpleStorage&) = delete;
            SimpleStorage (SimpleStorage&&) = delete;
            SimpleStorage& operator= (const SimpleStorage&) = delete;
            SimpleStorage& operator= (SimpleStorage&&) = delete;
    };
    
    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;
            std::unique_ptr<SimpleStorage<T>> storage;
    public:
        pointer data() noexcept {return std::launder(storage->p);}
        const_pointer data() const noexcept {return std::launder(storage->p);}
    
            explicit vector_t(std::size_t size = 0, value_type const &value = value_type{})
                    : data_size(size), storage(std::make_unique<SimpleStorage<T>>(data_size))
            {
                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)), storage(std::make_unique<SimpleStorage<T>>(data_size))
            {
                    std::uninitialized_copy_n(first, data_size, storage->p);
            }
    
            friend void swap(vector_t<T> &a, vector_t<T> &b) noexcept {
                    using std::swap;
                    swap(a.data_size, b.data_size);
                    swap(a.storage, b.storage);
            }
    
            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 storage->c; }
    
                  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;
    
    }
    
    

Anmelden zum Antworten