Будь проклят тот день #3 [С++ template hell]

щас налетит «это у меня руки кривые» https://github.com/kripken/box2d.js/issues/90 офф иссусле

помогите разобраться непойму

тебе не дано, не вижу смысла распинаться.

чет слабо доджишь

тесты emscripten-а уже заоджили окей

а шо по box2d

утечка памяи только в clang

кто нуб?

утечка памяи только в clang

Это не обязательно означает ошибку в компиляторе. Особенно в clang, выпоняющем довольно агрессивные оптимизации

P. S. Нихрена себе бомбит. Это мешает думать, успокойся

void* b2BlockAllocator::Allocate(int32 size) {
...
	if (size > b2_maxBlockSize)
	{
		return b2Alloc(size);
        }
...
    return m_freeLists[some_index];
...

Вы там из C# что ли пришли? Кто владеет объектом, определитесь уж. Стандартная ситуация с выращенными в тепличных условиях GC кодерами.

Там же у b2BlockAllocator::Free два параметра — указать и size. По size затем в Free определяют, кто именно должен освобождать память.

Странный подход, конечно, но выглядит вполне себе рабочим.

По size затем в Free определяют, кто именно должен освобождать память.

Ну да. Но это же рукопашная. Кто-то передал (гипотетически) во Free не тот size - вот утечка. В этом разобраться - надо реально засесть надолго, вникнуть. Но «пахнущий» код дает направление, куда копать.

Может быть еще проще. Ручной вызов Free в каком-то месте стоит рядышком с UB в коде. clang этот UB оптимизирует так, что Free выбрасывается, тогда как gcc или msvc могут не выбрасывать.

Да рабочая гипотеза. Как уже было сказано тут, отсутствие статического анализа и кросслатформенных сборок с -Werror не приблизит разрешения проблемы. Так же, как и нежелание участников проекта это понимать.

-Werror то не нужен

С чего бы это? Это потому, что ваша студия из своих заголовочников сыплет ворнингами? Ну так, наводит на подозрения

(Не стоит упоминять, что множество ворнингов и строгость реакции на них - тюнится. Но оставлять это на «и так сойдет» не прокатывает, что и подтверждает тред.)

ваша студия

Лолшто ?-)

множество ворнингов и строгость реакции на них - тюнится

Лол. Ну затюнь варнинг на сравнение знакового и беззнакового так, чтобы вот таких false-positive не было:

std::size_t max(...);
auto begin(...), end(...);
...
while (std::distance(begin,end) <= max) { ... }

unsigned i(0);
...
if (i == -1) { ... }

Предупреждения должны быть для обоих этих случаев.

-Werror вызывает боль в двух случаях:

- когда используется внешняя либа, над исходниками который ты не властен (например, при компиляции Asio clang-ом выдается очень много предупреждений);

- когда переходишь на новую версию компилятора, в которой появились новые диагностики и новая версия компилятора стала ругаться на то, что старые версии пропускали.

Для собственного же кода -Werror больше полезен, чем вреден.

std::distance(begin,end) <= max

тут типа есть разница между size_t и ptrdiff_t

Предупреждения должны быть для обоих этих случаев

Нет. std::distance() всегда выдаёт неотрицательный результат, т. к. в другом случае всё равно UB:

The behavior is undefined if end is not reachable from begin by (possibly repeatedly) incrementing begin.

при компиляции Asio clang-ом выдается очень много предупреждений

Кстати, да(-Wzero-as-null-pointer под gcc - не то чтобы много, но образованию простыни способствует)

Да, но значение std::ptrdiff_t, возвращаемое std::distance-ом всегда положительно

значение std::ptrdiff_t, возвращаемое std::distance-ом всегда положительно

Где это сказано?

The value may be negative if random-access iterators are used and first is reachable from last (since C++11)

Нет. std::distance() всегда выдаёт неотрицательный результат, т. к. в другом случае всё равно UB:

The behavior is undefined if end is not reachable from begin by (possibly repeatedly) incrementing begin.

Вроде как в C++11 и последующих условие другое:

If InputIt is not RandomAccessIterator, the behavior is undefined if last is not reachable from first by (possibly repeatedly) incrementing first. If InputIt is RandomAccessIterator, the behavior is undefined if last is not reachable from first and first is not reachable from last.

https://en.cppreference.com/w/cpp/iterator/distance

std::distance():
The behavior is undefined if end is not reachable from begin by (possibly repeatedly) incrementing begin.

В других случаях UB наступает ещё раньше, там уж не до сравнения

включи std=c++98. начиная с C++11 тут нет UB

Да, но warning есть не только для RandomAccessIterator

#include <list>
#include <iterator>

#include <cstddef>

int main()
{
	std::size_t s(3U);
	std::list<int> l{0,1,2};
	return std::distance(l.cbegin(),l.cend()) > s;
}

$ g++ -std=c++14 -pedantic-errors -Wall -Wextra -xc++ -Wsign-compare -c -o/dev/null main.c++ 
main.c++: In function 'int main()':
main.c++:10:44: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
  return std::distance(l.cbegin(),l.cend()) > s;
         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
$ g++ --version
g++ (Debian 6.3.0-18+deb9u1) 6.3.0 20170516
Copyright (C) 2016 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Есть, куда без него )

Не понимаю, о чем тут спор. Кто лучше оправдает подозрительный код?

warning: comparison between signed and unsigned integer expressions

такие ворнинги бесят, да. Но по факту они правильные. Есть сомнения в адекватности стандарта? Даже если и так, то противоречить ему в коде - себе дороже.

Ну ptrdiff_t - это, даже на пальцах, совсем не то что size_t. Бывает, выйти из кругов ада с этим связанных очень не просто - другой вопрос...

Но по факту они правильные

Нет же. Ты сам выше цитату приводил, отрицательное значение может возвращаеться при передаче RandomAccessIterator. Для остальных категорий итераторов возврат всегда положительный(там я уже цитировал). Я привёл пример, где нет RandomAccessIterator-а(соответственно, возврат положительный и сравнение проканает) - а warning всё равно есть(априори бесполезный).

такие ворнинги бесят, да

А с -Werror одними матюками не ограничишься - надо будет что-то менять

Ну ptrdiff_t - это, даже на пальцах, совсем не то что size_t. Бывает, выйти из кругов ада с этим связанных очень не просто - другой вопрос...

С этим не спорю(как и с нужностью warning-ов), я против именно -Werror во все поля

Я привёл пример, где нет RandomAccessIterator-а(соответственно, возврат положительный и сравнение проканает) - а warning всё равно есть(априори бесполезный).

Warning есть потому, что прототип std::distance не меняется в зависимости от типа итератора. Как возвращался difference_type, так и возвращается.

И это проблема стандарта, а не компилятора.

Польза от warning-е есть: сегодня у вас в коде std::list, завтра — std::vector. А если вы пишете обобщенный код, то вообще можете не знать, какой тип контейнера вам подсунут.

Нашел решение. убрал using Vertex = TCompoundVertex;
и ниже заменил все включения этого типа параметром TCompoundVertex

#include <iostream>
#include <cstdint>

using s8 = std::int8_t;
using u8 = std::uint8_t;

using s16 = std::int16_t;
using u16 = std::uint16_t;

using s32 = std::int32_t;
using u32 = std::uint32_t;

using s64 = std::int64_t;
using u64 = std::uint64_t;

using f32 = float;
using f64 = double;

using pstr = char*;
using pcstr = const char*;

template <typename TIndex>
struct CVertexManagerFixed
{
    template <typename TPathBuilder, typename TCompoundVertex>
    class CDataStorage : public TPathBuilder::template CDataStorage<TCompoundVertex>
    {
    public:
        using CDataStorageBase = typename TPathBuilder::template CDataStorage<TCompoundVertex>;

    public:
        inline CDataStorage(const u32 vertex_count);
        inline virtual ~CDataStorage();
        inline TCompoundVertex& get_node(const TIndex& vertex_id) const;
    };
};


#define TEMPLATE_SPECIALIZATION                           \
    template <typename TIndex> \
    template <typename TPathBuilder, typename TCompoundVertex>

#define CFixedVertexManager \
    CVertexManagerFixed<TIndex>::CDataStorage<TPathBuilder, TCompoundVertex>

TEMPLATE_SPECIALIZATION
inline CFixedVertexManager::CDataStorage(const u32 vertex_count)
    : CDataStorageBase(vertex_count)
{
}

TEMPLATE_SPECIALIZATION
inline TCompoundVertex& CFixedVertexManager::get_node(const TIndex& vertex_id) const
{
    VERIFY(is_visited(vertex_id));
}

вообще можете не знать, какой тип контейнера вам подсунут.

... тогда оно при инстансе и выстрелило бы!

И это проблема стандарта

ну типа того. Но не очень значительная.

ЗЫ Ох, вот о чем не говорят почему-то - так это скорость компиляции. Боже, мне с 3 часовой дебажной сборкой так дико смотреть на _СЕКУНДЫ_ сборки огромных C# проектов. Я не верю, просто не верю.

прототип std::distance не меняется в зависимости от типа итератора

template< class InputIt >
typename std::iterator_traits<InputIt>::difference_type
distance( InputIt first, InputIt last );

Таки меняется, или я не правильно понял, что имелось ввиду

А если вы пишете обобщенный код, то вообще можете не знать, какой тип контейнера вам подсунут

Но я всегда знаю(в своих кейсах, разумеется), что end достижим из begin

убрал using и сделал всё вручную

Пипец какой-то. using это прям больное место компиляторов

Ох, вот о чем не говорят почему-то - так это скорость компиляции.

А с этим кардинально вряд ли что-то сделаешь. Даже модули, будут они сделаны, могут принципиально ничего не поменять, т.к. уже сейчас на сложном шаблонном коде львиная доля времени тратится не на чтение и парсинг include-ов, а на инстанциирование шаблонов и оптимизацию получившегося добра.

Тут разве что переход на другой ЯП поможет :(

Таки меняется, или я не правильно понял, что имелось ввиду

Имелось в виду, что в конкретных случаях с std::list и std::vector за difference_type скрывается одно и то же.

Не хватает Царя в треде.

Ok. А проблема всё таки не в стандарте - концептуально, расстояние модет иметь знак минус. Короче, лучше хрен сделаешь)

Не нужен

Принеси лучше «офф иссусле» из трекера clang-а.

Странный подход, конечно, но выглядит вполне себе рабочим.

Такой подход какой-то корифей из комитета на cppcon продвигал.