cpp-ru / ideas

Идеи по улучшению языка C++ для обсуждения
https://cpp-ru.github.io/proposals
Creative Commons Zero v1.0 Universal
89 stars 0 forks source link

Замечания к C++20 #374

Closed apolukhin closed 3 years ago

apolukhin commented 3 years ago

Перенос предложения: голоса +5, -0 Автор идеи: yndx-antoshkka

Собираем в коментариях замечания, баги и нодочёты готовящегося C++20.

Пожалуйста, делитесь мыслями и примерами.

apolukhin commented 3 years ago

Vladimir Smirnov, 28 ноября 2018, 23:20 Меня очень сильно смущает текущее предложение по std::chrono::months(). В текущем виде это ровно 1/12 года. Предвижу баги вроде "я расчитал time_point для 1 февраля 2018 00:00, прибавил 1 месяц, получил 3 марта, полтретьего ночи, WTF?". Юзкейсы мне кажется будут из серии "легко сделать неправильно, сложно сделать правильно".

yndx-antoshkka, 29 ноября 2018, 13:27 Vladimir Smirnov, кажется, что просто так сложить не получится:

auto feb1_2018 = 2018y/February/1;
feb1_2018 + month(1); // OK: 2018y/Match/1

sys_seconds{feb1_2018} + month(1); // Compile time error
sys_seconds{feb1_2018} + static_cast<unsigned>(month(1)); // Ну, сам так написал

month и прочие новые типы - это не std::chrono::duration а отдельные классы, которые к нему просто так не преобразоввываются. Но возможно что я неправильно понял ваш пример

Vladimir Smirnov, 29 ноября 2018, 16:08

    //взял отсюда: https://en.cppreference.com/w/cpp/chrono/duration
    using months = std::chrono::duration<int64_t, std::ratio<2629746>>;
    auto now = std::chrono::system_clock::now();
    // print_as_date(now);
    auto month_later = now + months(1);
    // print_as_date(month_later);

я скорее про что-то подобное. вообще странно, sys_seconds это вроде ж и есть timepoint, как now в моем примере, если оно будет запрещать складывать и результат от clock-ов, то тогда скорее всего мои опасения не оправданы. То что с календарями оно работает как задумано, это я догадывался)

у меня нет под рукой компилятора с поддержкой C++20 в библиотеке. chrono в транк libc++ еще вроде не завезли https://libcxx.llvm.org/cxx2a_status.html , вы где смотрели?

yndx-antoshkka, 29 ноября 2018, 19:06 Vladimir Smirnov, методом пристального взгляда на стандарт :)

Посмотрел в libc++, там есть класс month не являющийся duration

Но вы правы, одновременно с ним в стандарте есть и months, являющийся duration

Кажется что стоит убрать std::chrono::months.

yndx-antoshkka, 29 ноября 2018, 19:20 С годами такая же печаль. Поиграться можно тут

Я напишу автору предложения, посмотрим что он об этом думает.

Vladimir Smirnov, 29 ноября 2018, 22:28 yndx-antoshkka, да, я в самом первом посте писал о months, я что-то был невнимателен и не заметил что в ответе вы уже другой тип использовали. к инаму месяца у меня вопросов нет) И да, второе замечание которое вытекает из первого - months и month будут все путать, они даже в одном неймспейсе :D

yndx-antoshkka, 7 декабря 2018, 11:05 Vladimir Smirnov, автор Calendars and Time Zones ответил:

I think I spent about two full months just on the day/days/month/months/year/years name problem.  In the end, this was the best I could come up with.

I wanted the duration naming to be consistent with our current duration naming pattern:  nanoseconds, microseconds, milliseconds, seconds, minutes and hours.  I.e. a plural unit.

I decided against average_months because months (and years) actually does double duty.  It is useful both in chronological computations and calendrical computations.  The downside is that no one thinks in terms of these two different types of computations, and yet they _are_ both useful, and so we have an education problem.

Here is a SO answer that goes into quite a bit more detail comparing and contrasting chronological and calendrical computations with months:

https://stackoverflow.com/a/43018120/576911

If months was named average_months, then there would be a confusing use with calendrical arithmetic, e.g.:

    2018y/December/3 + average_months{6};

vs:

    2018y/December/3 + months{6};

If we had two different types of months:  1 for chronological computations named average_months and one for calendrical computations named months, that too would raise confusion (and increase the size of the apparent API).

I know that the status-quo also raises confusion, but I believe it raises less confusion than the alternatives.  Once educated about the difference between chronological and calendrical computations (as done by https://stackoverflow.com/a/43018120/576911), I believe that the confusion to the average programmer will be reduced.

Howard

Если есть идеи, как исправить или улучшить текущее положение - говорите, автор рад будет услышать рац. предложения.

Vladimir Smirnov, 7 декабря 2018, 13:29 yndx-antoshkka,

Мне все равно не нравится в текущем смысле что прибавление одной и той же сущности дает разное поведение.

Это в моем понимании, как если перегруженные функции немного по-разному себя ведут.

Мое предложение, иметь два типа, которые не допускают перекрестного сложения:

-calendar_months (можно складывать с календарями)

-average_months( можно складывать с time_pointa-ами)

months убрать вовсе.

yndx-antoshkka, 11 декабря 2018, 11:39 Vladimir Smirnov, подработал и предложил вашу идею Говарду:

============================

Hello again,

I've talked with people and here are the conclusions (with a lot of ironong from myself):

At the moment we have multiple major different domains:
* durations
* time_points
* calendars

While the operations stay in the same domain, there's no problem. Problems arise when the name from one domain sounds like a name from other domain or when calendars and duration domains meet. In last case current wording attempts to predict the behavior the user wants, which randomly succeeds. So the idea is to make operations between calendar and duration domains more explicit.

To separate the domains the following changes could be applied:
* add chrono::duration types average_days, average_months, average_years, average_weeks
* Drop weeks and rename days, months, years to calendar_days, calendar_months, calendar_years throughout the [time]
* Make those calendar_* types a separate types, not the chrono::duration ones:

class calendar_months {
int value;
public:
...
};

* add operators to year_month_day that work with calendar_days (calendar_days are not convertible to sys_­days any more)
* [optional] you could drop day and year and use calendar_days and calendar_years instead (this will keep the amount of types exactly the same, as before all the changes)

Now with that API the following surprising constructions will change behavior:

system_clock::now() + months{1};  // Fails to compile. No more months...
system_clock::now() + average_months{1}; // returns duration, explicit, not surprising (mostly)
system_clock::now() + calendar_months{1};  // returns year_month_day{now()} + calendar_months{1}.... and we loose the hours,minutes and seconds :( But at least in a more explicit way

calendar_days{31} + system_­clock::now(); // returns year_month_day
average_days{31} + system_­clock::now(); // returns duration

average_days{31} - average_days{1} + system_­clock::now(); // returns duration
calendar_days{31} - calendar_days{1} + system_­clock::now(); // returns year_month_day{now()} + 30 days

average_days{31} - calendar_days{1} + system_­clock::now(); // comile time error
calendar_days{31} - average_days{1} + system_­clock::now(); // comile time error
calendar_days{31} + system_­clock::now() - average_days{1}; // comile time error
-average_days{1} + system_­clock::now() + calendar_days{31}; // year_month_day
system_­clock::now() - average_days{1} + calendar_days{31}; // year_month_day
system_­clock::now() + calendar_days{31} - average_days{1}; // comile time error

Friday - Monday + 20s; // fails to compile, no calendar_days + duration overload

system_­clock::now() - average_years{1990}; // returns duration
system_­clock::now() - calendar_years{1990}; // returns year_month_day{now()} - calendar_years{1990}

(2018y/March - 2018y/February) + system_­clock::now(); // returns year_month_day
sys_days{2018y/March - 2018y/February}+ system_­clock::now(); // OK, explicit

Monday + 10s; // fail to compile

(March - February) + system_­clock::now(); // returns year_month_day
sys_days{March - February} + system_­clock::now(); // returns duration, surprising :(
1/Febuary + 20ns; // fails to compile

auto birth_time = 2018y/February/1 + average_months{9} // compile time error
auto birth_time = sys_days{2018y/February/1} + average_months{9} // OK, sys_days returned

Proposed changes prefer calendar types if at least one of the operands is calendar (because writing seconds{X} or sys_time{X} is shorter than writing year_month_day{X} and because year_month_day explicitly says that it does not store hours, minutes, seconds).

Now we have implicit operations for time_point <=> calendars, time_point <=> durations; and explicit operations required for durations <=> calendars.

How do you feel about the proposed changes? Do they improve things or, just break the whole word?

============================

Выяснилось, что данный подход очень плох:

============================

This is a pretty radical redesign that violates several of my design principles and has no field experience.

*  It increases the apparent API by adding more types that have nearly identical semantics.

*  It introduces lossy arithmetic (expression which implicitly loose information).

*  It significantly complicates the algebra among chronological and calendrical types, in some of your examples even breaking the type system, e.g. sys_days{March - February}.

*  It hides the relatively expensive conversion between the chronological representation and the calendrical representation.  This point is discussed further here:

https://github.com/HowardHinnant/date/wiki/FAQ#day_arithmetic

I’m quoting the last few paragraphs of this note for emphasis:

> This philosophy is similar to that which we have for containers: It would be super easy to create vector<T>::push_front(const T&). But that would make it too easy for programmers to write inefficient code. The compiler helps remind the programmer that perhaps deque<T> or list<T> would be a better choice when he attempts to code with vector<T>::push_front(const T&).
>
> It would be very easy to add T& list<T>::operator[](size_t index). But that would encourage the programmer to use list<T> when a random-access container would probably be more appropriate for the task.
>
> This library continues in that tradition: The expensive operations are not hidden.

The status-quo is easy to learn:

1.  If you’re doing arithmetic with a time_point, you’re doing a chronological computation.

2.  If you’re doing arithmetic with a calendrical type, you’re doing a calendrical computation.

3.  months and years can be used in both chronological and calendrical computations.

4.  If you want to convert between the calendrical and chronological domains, it must be done at the day-precision level.

5.  Day-precision arithmetic in the calendrical domain is a compile-time error because it is gratuitously inefficient to implement such arithmetic (like list<T>::operator[](size_t index)).

Sorry, I can not propose these changes.

Howard

============================

Vladimir Smirnov, 13 декабря 2018, 16:00 Антон, спасибо вам за формулировки и переписку! Я благодарен за ваши попытки.

Мотивация Говарда мне понятна.

Что ж, придется тогда с этим жить, если и у комитета возражений не возникнет.

В целом, я так понял, в нашем с ним разном представлении о "nearly identical semantics". Надеюсь, мое видение окажется скорее исключением :)

adler3d, 29 ноября 2018, 2:14 В С++ нужно заменит символы для обжатия шаблонных параметров на что-то другое, т.к они не дружат с макросами. А ещё если делать свой компилятор С++ то можно заметить, что они вот прям на ровном месте адово всё усложняют. Предлагаю сначала пропихнуть в стандарт новый вариант символов, а затем через лет 10 пометить старый как deprecated или даже сразу.

PS: символы для обжатия шаблонных параметров - это "<" и ">".

yndx-antoshkka, 29 ноября 2018, 13:31 adler3d, вроде бы сегондя не April/1

adler3d, 2 декабря 2018, 19:43 https://gamedev.ru/flame/forum/?id=185614

yndx-antoshkka, 7 декабря 2018, 19:18 adler3d, Символы поменять никто не согласится. Если есть случаи, когда понятно что хотел написать разработчик, но грамматика C++ не справляется - то это исправимо и надо написать подробный пример. Если не понятно что хотел написать разработчик, то исправлять надо не стандарт C++.

По приведённой ссылке - проблема с макросами и запятыми между <>. Данная проблема давно известна и не требует исправления, так как исправления поломают имеющийся код с макросами. Аналогичная проблема, если в макросы передавать блоки кода с оператором запятая.

Вышеозвученные проблемы - это одни из причин, по которым все разработчики C++ не рекомендуют использовать макросы в C++.

Олег Власов, 10 декабря 2018, 17:54 Меня немного волнует, что пустые std::map и std::set аллоцируют память. Это странно и не правильно.

yndx-antoshkka, 11 декабря 2018, 12:00 Олег Власов, а в какой именно стандартной библиотеке?

Я проверил парочку - map и set не аллоцируют память. По стандарту noexcept не прописан для дефолтных конструкторов deque, list, forwardlist, [multi]map, [multi]set, unordered[multi]map и unordered_[multi]map.

Для deque и unordered_* контейнеров прописать подобное возможно будет неверным, а вот для остальных контейнеров я не вижу преград (если конечно не найдётся имплементация, которая в дефолтном конструкторе аллоцирует).

Олег Власов, 11 декабря 2018, 15:43 yndx-antoshkka, Я ссылался на статью https://habr.com/company/1c/blog/429678/ Я заглянул в исходники стандартной библиотеки, реализация Visual c++, мне показалось, что там происходит аллокация памяти, но могу ошибаться. Я поверхностно посмотрел.

Andrey Davydov, 12 декабря 2018, 22:15 Олег Власов, так сделано у Microsoft, память выделяется под end node (на который указывает .end()). Из-за этого и move-конструкторы map'а, set'a и list'а тоже аллоцируют и не помечены как noexcept. Это сделано, чтобы splice (а начиная с C++11 и move) не инвалидировал итераторы.

yndx-antoshkka, 19 декабря 2018, 11:27 Note: стоит поправить вот это std::bit_cast<T*> должен проверять выравнивание

Andrey Davydov, 6 января 2019, 13:57 Хотелось бы прояснить в стандарте, разрешена ли перегрузка деструктора, если да, то, наверное, есть смысл явно добавить пример в стандарт.

#include <type_traits>

template<typename T>
struct Y {
    ~Y() requires(std::is_trivially_destructible_v<T>) = default;
    ~Y() {}
};

struct A {};
struct B { ~B(); };

static_assert(std::is_trivially_destructible_v<Y<A>>);
static_assert(!std::is_trivially_destructible_v<Y<B>>);

В данный момент и GCC и Clang отвергают этот код: https://gcc.godbolt.org/z/nqklnE но я не нашел, где бы в стандарте это запрещалось. У GСС есть bug на подобный случай (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67348) от 2015-го года, ссылающийся на Concepts TS, однако, по-моему, это не соответствует текущему черновику стандарта.

Andrey Davydov, 6 января 2019, 14:15 Я бы выразился даже сильнее -- оба компилятора ведут себя однозначно неверно: у GCC ICE, Clang кушает такое:

#include <type_traits>

template<typename T>
struct Y {
    ~Y() requires(std::is_trivially_destructible_v<T>) = default;
    ~Y() {}
};

struct B { ~B(); };

static_assert(std::is_trivially_destructible_v<Y<B>> && 
              !std::is_nothrow_destructible_v<Y<B>>);

Andrey Davydov, 7 января 2019, 22:09 Нашел proposal 2017-го года в котором это обсуждается: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0848r0.html. Было бы очень интересно узнать, что именно предлагалось по ссылкам [2] и [3], но у меня нет прав читать "секретную переписку коммитетчиков".

yndx-antoshkka, 24 января 2019, 17:07 Andrey Davydov, завёл багу на GCC https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89036 и на Clang https://bugs.llvm.org/show_bug.cgi?id=40437

Почитать переписку по тем ссылкам я тоже не смог, но на последнем заседании обсуждали эту бумагу и она должна попасть в C++20.

Andrey Davydov, 24 января 2019, 17:33 yndx-antoshkka, для GCC уже был баг: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67348

А к переписке доступ мне дали, но там нет ничего особо ценного, в бумаге все нужное есть. Будет хорошо, если примут.

Vladimir Smirnov, 20 января 2019, 14:40 Что-то меня смущает предложение по настроке build-level для контрактов.

например, я хочу сделать сборку, в которой мой код будет проверять все контракты с максимальным уровнем. Но при этом я не хочу, чтобы шла проверка контрактов в стандартной библиотеке, например в operator [] на то что контейнер не пустой.

В текущем предложении build-level хоть и implementation-defined, но тем не менее подразумевается, что он единый на всю единицу трансляции.

Общая идея такая - я хочу в релизной конфигурации, которая уходит пользователям, оставить проверки, которые будут например кидать exception/писать в лог, если что-то сильно пошло не так. При этом я не хочу этих проверок в стандартной библиотеке (когда они там появятся - а ведь появятся же!). Что мне делать?

yndx-antoshkka, 24 января 2019, 17:09 Vladimir Smirnov, это тянет на целую новую идею. На подобное надо писать отдельный proposal, в качестве замечания она не пройдёт в комитете.

languagelawyer, 1 февраля 2019, 4:52 В C++20 фиксировали представление целых чисел, но не сделали поведение следующего кода определённым

int  i = -1;
auto u = reinterpret_cast<unsigned&>(i);

yndx-antoshkka, 1 февраля 2019, 10:36 languagelawyer, не делайте так. Делайте вот так:

int i = -1;
auto u = static_cast<unsigned>(i);

Andrey Davydov, 11 февраля 2019, 21:51 Хотелось бы прояснить (а скорее запретить) использование requires-clause в template alias declaration. Сейчас, грамматически разрешено писать так:

template<bool condition, typename T = void>
    requires(condition)
using enable_if_t = T;

и на первый взгляд может показаться, что это работает, во всяком случае при попытке проинстанцировать enable_if_t компилятор ругнется "constraints not satisfied..." . Однако если подставлять в enable_if_t<dependent-expr, some-type>, мы получим на выходе some-type, requires-clause будет благополучно проглочено (что, в принципе, ожидаемо, учитывая природу type alias'ов). Например, такой код

template<typename T, typename = enable_if_t<sizeof(T) < 0>>
void foo();

void test() {
    foo<int>();
}

скомпилируется.

yndx-antoshkka, 12 февраля 2019, 13:02 Andrey Davydov, я надеюсь что это ошибка в компиляторах. Например в таком виде clang правильно не собирает: https://godbolt.org/z/WJQzSw

на всякий случай завёл багрепорты на GCC https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89301 и Clang https://bugs.llvm.org/show_bug.cgi?id=40700

Посмотрим что скажут

Andrey Davydov, 12 февраля 2019, 13:15 yndx-antoshkka, Ричард Смит считает также: https://groups.google.com/a/isocpp.org/d/msg/std-discussion/Bwh2d3i8GhE/rHM58wAkGQAJ. К сожалению, я не смог ему ответить из-за чертовых Google Groups, однако такой подход дает новый источник equaivalent but functionally not equivalent функций, к примеру:

template<typename T> requires(sizeof(T) < sizeof(int))
using lt_int = T;

template<typename T> requires(sizeof(T) > sizeof(int))
using gt_int = T;

template<typename T> void foo(lt_int<T>);
template<typename T> void foo(gt_int<T>);

void test() {
    foo<short>(1);
    foo<long >(1);
}

Anton Kudryavtsev, 9 марта 2019, 16:53 почему нет constexpr версии для std::accumulate?

https://en.cppreference.com/w/cpp/algorithm/accumulate

yndx-antoshkka, 11 марта 2019, 16:23 Anton Kudryavtsev, я не успел добавить constexpr к [numeric.ops.overview]

Замечанием к C++20 это не исправить, нужно писать отдульную бумагу, уже для C++23.

yndx-antoshkka, 11 марта 2019, 16:18 В копилку к замечаниям: Починить std::invoke_result_t

Aleksey Miklin, 13 марта 2019, 19:11 Несколько смущает отсутствие в C++ универсальной нешаблонной неконстантной ссылки:

struct Foo
{
    void nonConst(){}
};

void f(Foo& foo){ foo.nonConst(); }
void f(Foo&& foo){ foo.nonConst(); }
// It would be nice to have a non-template universal reference to avoid code duplication above:
// void f(Foo&&& foo){ foo.nonConst(); }

void sample()
{
    Foo foo;
    f(foo);

    f(Foo{});
}

WPMGPRoSToTeMa, 13 марта 2019, 20:34 Должен ли std::flat_map иметь operator[]? Как мне кажется лучше убрать operator[] из него, слишком он уж error-prone. Вообще по идее в C++ можно добавить operator[]= не ломая старый код, так что мне кажется тем более не стоит добавлять operator[] для std::flat_map.

WPMGPRoSToTeMa, 14 марта 2019, 23:34 Если быть точнее, то const версию оставить, а non-const версию убрать.

WPMGPRoSToTeMa, 18 марта 2019, 13:45 Сам себя запутал, никакой const версии у std::flat_map::operator[] нет. В общем-то проблема в том, что если делаем нормальный operator[] у std::flat_map, то тогда получится различие в интерфейсе с std::[unordered_]map, вопрос в том насколько это принципиально. Причём даже если и делать нормальный operator[], то встаёт вопрос об исключениях, ибо по умолчанию в std операторы их не бросают, но будет ли в этом смысл если примут zero-overhead deterministic exceptions? Возможно что всё-таки придётся оставить без выброса исключений. А пока этот пропозал не приняли можно оставить там UB в случае получения доступа к несуществующему элементу.

I'm not a bot, 1 октября 2019, 12:46 Баг, нельзя вызывать шаблон коасса без <T1,T2 и т.д> как функцию Копилятор не понимает что происходит и отрубает подстветку после перезагрузки.

Andrey, 17 октября 2019, 23:40 Концепция discarded деклараций (http://eel.is/c++draft/module#def:discarded,declaration) представляет большую сложность для IDE и прочих желающих "схалявить". Рассмотрим кусочек примера из стандарта (http://eel.is/c++draft/module#global-6.example-1)

// foo.h
namespace N {
  struct X {};
  int h(X);
}

// M.ixx
module;
#include "foo.h"
export module M;
template<typename T> int use_h() {
  N::X x;                       // N​::​X, N, and ​::​ are decl-reachable from use_­h
  return h((T(), x));           // N​::​h is not decl-reachable from use_­h, but
                                // N​::​h is decl-reachable from use_­h<int>
}
int k = use_h<int>();

для того чтобы понять, что N::h не должна быть discarded нужно заглянуть в инициализатор k (обычно IDE этого не делает, если только k не constexpr или его тип не надо выводить) и проинстанцировать use_h, причем не только сигнатуру но и тело.

В принципе, можно взять пример даже попроще, без шаблонов:

// foo.h
void foo();
// M.ixx
module;
#include "foo.h"
export module M;
export inline void bar() {
  foo();
}

Чтобы узнать, что foo не должна быть discarded, надо заглянуть в тело функции bar. Обычно IDE просто пропускает тела функций, если только это не constexpr-функция или не функция с deducible return type.

Можно сказать, что это проблемы разработчиков IDE, которые никого не волнуют, но есть и другой взгляд на ситуацию: то что написано внутри тела функции или инициализатора переменной (не constexpr и не требующей вывода типа) не должно влиять на интерфейс модуля. А по факту, множество non-discarded деклараций входит в интерфейс модуля.

Предлагаемое решение: разрешить имплементациям считать функции non-discarded даже если они не являются decl-reachable. Это приемлимо, так как модули уже и так в некоторые моменты разрешают компилятору делать так как будет для него эффективнее. Во-первых, в том же определении decl-reachable есть довольно большая степень своботы (пункты 3.6--3.9) http://eel.is/c++draft/module#global-3.6, во-вторых в определении instantiation context (http://eel.is/c++draft/module#context-7.example-1).

apolukhin commented 3 years ago

Что успели, обсудили с авторами предложений и передали в комитет. Результаты работы: https://habr.com/ru/company/yandex/blog/474716/