Pull to refresh
4
0
Nikolai Panov @code_panik

User

Send message

Польза выводов мне кажется немного не очевидной. Выражение s = -u для знакового s и беззнакового u состоит из двух: унарного минуса и присваивания.

Мы разобрались, что результат выражения -u для любого беззнакового u определён. По ссылке

8.5.2.1.8 ... The negative of an unsigned quantity is computed by subtracting its value from 2^n, where n is the number of bitsin the promoted operand. The type of the result is the type of the promoted operand.

Беззнаковый тип по определению не может переполниться, потому что вычисления выполняются по модулю наибольшего представимого + 1 для данного типа. Поведение унарного минуса естественно соответствует этой логике - бежим по циферблату назад.

Вся проблема в попытке присвоить целому знакового типа значение, которое не представимо данным типом. Как указано в статье, поведение определяется реализацией.

7.8.3 If the destination type is signed, the value is unchanged if it can be represented in the destination type;otherwise, the value is implementation-defined.

То есть, как обычно, следует обращать внимание именно на совместимость данных разных типов.

Примечательно то, что на данный момент в плюсах нет способа устранить этот копипаст. 

"Можно ли реализовать обобщенную арифметику для enum?" - можно https://godbolt.org/z/Yee376o9Y

"Нужно ли это авторам libc++?" - нет.

Бросилось в глаза:

  • повторение inline в теле класса излишне, потому что метод, определенный в теле класса, по умолчанию inline;

  • лучше в GGarbageCollector* Get() возвращать (возможно const) ссылку, чтобы убрать ненужное разыменовывание указателя. Чтобы избежать неявных глобальных изменений данных, следует использовать глобальные const переменные. Синглтон - глобальный объект, поэтому лучше возвращать const ссылку и объявлять public методы const;

  • если служебный метод класса (напр. конструктор или деструктор) имеет поведение по умолчанию (пустое тело в коде), для наглядности лучше сделать его = default;

  • повторять несколько раз спецификаторы доступа к членам класса необязательно. Общий подход к размещению членов класса; https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rl-order

  • аргумент IsNew конструктора TGCPointer(T* InObject, bool IsNew) не имеет прямого соотвествия полю класса, а участвует в скрытой логике реализации, вытаскивая её на поверхность. Наверное, следует пересмотреть дизайн класса;

  • get-методы, например TGCPointer::GetChecked, по названию, не должны менять состояния класса. GetChecked, вероятно, не get-метод, так как в теле выполняется несколько не связанных с получением данных действий. Следует переименовать, лучше разбить (или удалить, если не окажется полезным);

  • TGCPointer::operator=(T* Ptr) для произвольного адреса выглядит небезопасным;

  • в MakeGCPointer вместо аргументов выписаны типы аргументов, должно быть вроде new T{forward(args)...};

  • о замене size_t на uintptr_t и использовании reinterpret_cast вместо C-стиля; https://stackoverflow.com/questions/153065/converting-a-pointer-into-an-integer

  • delete допускает nullptr аргумент, проверка не нужна;

  • BitsPackSize и BitsPackMask следует связать через промежуточные вычисления, чтобы не вносить правки по отдельности руками;

  • функции, возвращающие FAddrHandler*, могут возвращать ссылку (const), чтобы не добавлять лишнего разыменовывания;

  • для получения данных, на которые указывает iterator в STL следует использовать разыменовывание; https://en.cppreference.com/w/cpp/named_req/Iterator

  • подход к реализации структуры данных с forward_list выглядит переусложненным. Можно было заменить внутренности реализации FAnyPtrMap на unordered_map;

Пока в целом код выглядит непродуманным - чересчур много ручной работы и частных случаев (напр. SyncRegisterExactlyNew, SyncRegister). В таком виде код не поддерживаемый и не расширяемый. После первых правок захочется всё выкинуть и написать заново.
Основная проблема - преждевременная оптимизация. Сначала следовало выделить интерфейс и выписать простую реализацию. Потом оценивать целевые показатели по времени (памяти) и оптимизировать по необходимости.

Выше не упоминаются конкретные детали реализации, поэтому можно подобрать такие, чтобы обойтись без ub. Речь может идти о placement new с ручным вызовом деструкторов. Для простоты можем говорить о методах create/destroy (возможно, pimpl класса). Граф ссылок можно обойти поиском в глубину, вызывая destroy на каждом шаге назад.

В Qt нет gc. Есть объекты, владеющие ресурсами. Сборка мусора и владение ресурсом - разные коцепции.

Подозрения по поводу UB больше нет, оно там есть, так как ссылка привязывается к переменной l локальной для конструктора. Та же проблема, что и при возврате ссылки из функции - данных нет, ссылка осталась.

Ещё раз аккуратно перечитал. Это замечание - не проблема примера. В примере не используются copy/move-конструкторы X. Тогда пусть будет дополнением. Спасибо, что обратили внимание.

Думаю, есть несколько проблем в реализации класса X с шаблонным конструктором.

По-видимому, считается, что такой конструктор заменяет конструктор по умолчанию. Нет, см. напр. в Vandevoorde - C++ Templates. То есть может использоваться move-конструктор по умолчанию, напр. если не гарантировано создание объекта in-place (как в C++17). https://cppinsights.io/s/997cc3fb

Я очень подозреваю UB с dangling reference в специализации X::X<int>(int &&), потому что в списке инициализации X(int &&l) : val(l) член класса val (lvalue ссылка) ссылается на локальную переменную l. Когда собирал с -O0, то получал val != 0. В случае с -O(>0) val == 0 на разных компиляторах в compiler explorer. Кажется странным, что там же pvs studio ругается на https://godbolt.org/z/Esc813ssd и не ругается на https://godbolt.org/z/rvjaezbbT, хотя результат зависит от оптимизации.

Information

Rating
Does not participate
Registered
Activity