LINUX.ORG.RU

Werror - контроль качества или занудство?

 ,


0

4

Все, кто занимается низкоуровневым жонглированием байтами в реалых промышленных условиях (с кучей поставщиков со всего мира, а не в уютненьком молодёжном стартапе где-нибудь в Саннивейл), понимают, что C’шка с нами надолго, лет на 20, если не больше. И поэтому возникает желание максимально использовать существующие технологии для обеспечения качества кода.

Например, Werror (и то, что к нему полагается в виде -Wall, -Wextra и проч.). Но возникают такие ситуации, как например с «целочисленным повышением» и последующим сравнением с разным знаком. Например:

const unsigned x = 12;
unsigned char y;
unsigned char z;
... // что-то кладём в y и z
if (x < (y*z))
{
   // тра-ля-ля
}

И y*z превращаются («брюки превращаются, превращаются брюки…») в элегантный int, и вылезает предупреждение о различной знаковости, как бы совершенно на ровном месте. Т.е. теперь, чтобы ублажить компилятор, надо дополнительно, например, явно кастануть x к int’у. Т.е., код уже на пределе читаемости (выше пример - это сильное упрощение возможной реальной ситуации), и тут мы ещё добавляем вовсе не интуитивный (int).

И возникает вопрос: а стоит ли овчинка (-Werror и ко.) выделки? Я сейчас, очевидно, не имею в виду код наивысшей критичности, а такой, который при случае можно просто неспеша поправить, в конце рабочего дня, с нулевыми последствиями для пользователей, окружающей среды и т.п.

P.S. кстати, поскольку в расте тоже есть беззнаковые типы, там тоже нечто подобное должно быть, или как?

UPD: в примере, в нагрузку к умножению надо ещё добавить сложение с ещё одним unsigned char.

★★★★★

Последнее исправление: seiken (всего исправлений: 1)
Ответ на: комментарий от seiken

А в сишке таких проверок нет, и может произойти порча памяти, со всеми вытекающими последствиями.

разумеется, всё не так фатально. В сишке просто все операции, могущие привести к переполнению, надо явно проверять заранее в рантайме на переполнение. Но, блин, кто так делает? Считанные единицы. Для остальных - если скомпилировалось, то всё ОК.

seiken ★★★★★
() автор топика
Ответ на: комментарий от bugfixer

Разве не sizeof() на конкретной платформе имеет значение?

Это то тут причем? Стандарт устанавливает требования и оно вполне четкое, это требование, что не меньше, больше – можно, меньше – нельзя. А sizeof() это чтобы получить точное значение размера в байтах, а в limits.h прописаны точные min и max значения.

soomrack ★★★★★
()
Ответ на: комментарий от soomrack

Это то тут причем?

При том что в контексте integer promotion гипотетические диапазоны допустимые стандартом никого не волнуют, волнуют исключительно конкретные значения на конкретной целевой платформе.

bugfixer ★★★★★
()
Ответ на: комментарий от bugfixer

По факту конкретные значения на конкретной целевой платформе никого не волнуют, и делают integer promotion в соответствии с минимально допустимыми указанными в стандарте. И это правильно.

soomrack ★★★★★
()
Ответ на: комментарий от soomrack

По факту конкретные значения на конкретной целевой платформе никого не волнуют

Вопиющее заблуждение и 4.2. Перечитайте cppreference хотя бы. Если сильно упростить то ножки растут из (a) предположения что int самый быстрый integral type на целевой платформе, (b) все арифметические операции заданы для int и более широких типов. Поэтому все что короче(*) чем int и влезает без потерь в int превращается в int, всё что короче(*) unsigned и влезает в unsigned превращается в unsigned, итд. И ключевое здесь «влезает». Если оба short и int случились 32 битными то unsigned short запромоутят в unsigned. Шах и мат.

(*) «короче» здесь означает «меньший integer conversion rank» как определено туточки.

bugfixer ★★★★★
()
Ответ на: комментарий от bugfixer

Я не понял, что ты имеешь ввиду. Есть стандарт, он говорит, что надо вот так. Можно лучше? Можно, но в зависимости от платформ это лучше будет разным. Чтобы зависимости не было, надо делать вот так, как сейчас делают. Если хочешь чтобы было быстрее или экономичней, то контролируй сам приведение типов, все в твоих руках, языка дает достаточно свободы для этого.

soomrack ★★★★★
()
Ответ на: комментарий от soomrack

Я не понял, что ты имеешь ввиду.

Я утверждаю что выбор к чему промоутить аргументы арифметической операции напрямую зависит от sizeof() вовлеченных integral types на конкретно взятой платформе, и от гипотетических диапазонов допускаемых стандартом вообще ничего не зависит. Точка.

ПыСы. Я кажется понял в чём дело - вечер пятницы…

bugfixer ★★★★★
()

Например, Werror (и то, что к нему полагается в виде -Wall, -Wextra и проч.)

-Wreturn-type -Wpointer-sign -Wsign-compare -Wshadow -Wpointer-arith -Wimplicit -Wformat -Werror -Wno-parentheses -Wuninitialized

Никаких -Wall, там в том числе идиотизм типа -Wlogical-op-parentheses.

if (x < (y*z))

Вторые скобки тут лишние.

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

Тайпкасти в unsigned int.

UPD: в примере, в нагрузку к умножению надо ещё добавить сложение с ещё одним unsigned char.

Вот в сложных формулах (это конечно не примитивная формула a*b+c) как раз явное указание тайпкастов и правильная их расстановка тем более важны, иначе может получиться совсем не то что ты хотел. Да даже тут, если б там был не char а short, или если бы int был 16-битный - y*z могло бы получиться отрицательным и запороть тебе алгоритм.

firkax ★★★★★
()
Ответ на: комментарий от alysnix

Абсурдная идея включать -Werror без списка. Все проекты, которые так делают становится невозможно собрать без мата на другой машине. Даже если актуальная версия +- собирается.
Особенно классно если это большой проект и после правки билдскрипта придётся собирать с нуля. Помимо компиляторов причиной предупреждений могут быть заголовочные файлы (в т.ч системные). В общем, делать так - не уважать всех, кто будет притрагиваться к твоему проекту.
И сама идея -Werror вообще не очень хороша, но может быть полезна например в ci, чтобы не давать комиттить в код новые ошибки. Но даже тут возможны проблемы, например человек делает PR, на ci обновился компилятор и его PR не соберётся. В итоге что, автор PR должен исправлятт ошибки в не связянном с ним коде?

mittorn ★★★★★
()
Ответ на: комментарий от seiken

Можно и чёрного, нг делать это исключительно на ci и машинах разработчиков, чтобы оперативно исправлять новые предупрежления, они часто бывают полезны. Но не пихать эти опции напостоянку, чтобы не создавать неудобства остальным

mittorn ★★★★★
()
Последнее исправление: mittorn (всего исправлений: 1)
Ответ на: комментарий от mittorn

Так же есть неотключаемые ворнинги.

Не сталкивался с неотключаемым вредным варинингом.

Зато у пакостного шланга есть неотключаемый -Werror-варнинг на несоответствие прототипа main() чему-то там.

firkax ★★★★★
()
Ответ на: комментарий от firkax

Есть например предупреждение, что в битовое поле не влезет какой-то тип. Причём в gcc8 у него есть ложное срабатывание. Это последнее, что видел из такого, но были ещё подобные

mittorn ★★★★★
()
Ответ на: комментарий от bugfixer

А что со знаковостью? Я вот про неё не могу понять. По той ссылке что ты приводил выше:

The rank of any unsigned integer type equals the rank of the corresponding signed integer type.

Я не могу найти, где написано что-то типа uchar + uchar = unsigned ? Или это оно и значит?

здесь (Werror - контроль качества или занудство? (комментарий)) приводил такое:

If int can represent the entire range of values of the original type (or the range of values of the original bit-field), the value is converted to type int. Otherwise the value is converted to unsigned int.

anonymous
()
Ответ на: комментарий от anonymous

Речь о том, что «entire range» берётся конкретное из таргет-платформы а не из общей теории.

unsigned short будет промоутиться до int на 32-битной платформе и не будет никуда промоутиться на 16-битной. То есть арифметика из двух unsigned short будет давать беззнаковый результат на 16-bit и знаковый на 32-bit.

А на платформе с 32-битными char, short и int (такое не запрещено) char тоже не будет никуда промоутиться.

firkax ★★★★★
()
Последнее исправление: firkax (всего исправлений: 1)
Ответ на: комментарий от bugfixer

Я утверждаю что выбор к чему промоутить аргументы арифметической операции напрямую зависит от sizeof() вовлеченных integral types на конкретно взятой платформе, и от гипотетических диапазонов допускаемых стандартом вообще ничего не зависит. Точка.

Не без влияния пятницы, конечно…

Стандарты С и С++ отличаются в правилах что надо делать, даже называются по-разному: integer promotion (C11) vs integral promotions (C++17).

Но они одинаковы в части того, что для всего, что короче int и unsigned int – они требуют промошена именно до int или unsigned int (а не short int).

Но в этом примере вывод и С и С++ будет одинаков, произведение двух int до long int не промоутится.

unsigned char x = 0xFF;
printf("%ld\n", sizeof(x));      // 1
printf("%ld\n", sizeof(x * x));  // 4

unsigned int y = 0xFFFFFFFF;
printf("%ld\n", sizeof(y));      // 4
printf("%ld\n", sizeof(y * y));  // 4

printf("%X\n", ((y + 1)>>8));    // 0 

unsigned short int z = 0xFFFF;                                                                                                                                                                  
printf("%ld\n", sizeof(z));      // 2                                                                                                                                                           
printf("%ld\n", sizeof(z * z));  // 4

PS: логика всего этого, как я понимаю, в том, что вычисления идут не быстрее чем с int, поэтому все что короче можно добить до int без потери скорости, а все что длиннее – то уже сам прогер должен озаботиться как поступать… имхо.

soomrack ★★★★★
()
Последнее исправление: soomrack (всего исправлений: 8)
Ответ на: комментарий от firkax

unsigned short будет промоутиться до int на 32-битной платформе и не будет никуда промоутиться на 16-битной. То есть арифметика из двух unsigned short будет давать беззнаковый результат на 16-bit и знаковый на 32-bit.

Вот это я не пойму. Размер что int, что unsigned будет один, согласно чему знаковость будет по-разному меняться?

anonymous
()
Ответ на: комментарий от anonymous

Как я понимаю стандарт, в любом случае будет расширение до int или unsigned int (но если размер int и short int одинаков, то по факту ничего не будет).

  1. short int превратится в int
  2. unsigned short int превратится в unsigned int
soomrack ★★★★★
()
Ответ на: комментарий от anonymous

Если целочисленное promotion случилось то итогом всегда будет int. Беззнакового promotion не бывает. Беззнаковое остаётся только если promotion не было, а его нет если sizeof(исходного выражения)>=sizeof(int).

firkax ★★★★★
()
Ответ на: комментарий от soomrack

Нет. unsigned short int останется само собой (не превратится в unsigned int) если sizeof(unsigned short)>=sizeof(int). Если sizeof(unsigned short)<sizeof(int) то он превратится в signed int.

firkax ★★★★★
()
Последнее исправление: firkax (всего исправлений: 1)
Ответ на: комментарий от firkax

Если sizeof(unsigned short)<sizeof(int) то он превратится в signed int.

Нет. Ты можешь в этом убедиться на примерах выше в треде.

unsigned short int x;
unsigned short int y;

(x+y);  // unsigned int

Как и сказано в стандарте, т.к. 0xFFFF * 0xFFFF не влезут в signed int, а в unsigned int влезут.

soomrack ★★★★★
()
Ответ на: комментарий от soomrack

произведение двух int до long int не промоутится.

Произведение вообще никогда не промоутится.

unsigned char x = 0xFF;
printf("%ld\n", sizeof(x));      // 1
printf("%ld\n", sizeof(x * x));  // 4

Тут промоутятся аргументы умножения каждый до int и потом уже умножаются. Арифметика для типов размером меньше int в Си не определена, они всегда сначала приводятся к signed int.

firkax ★★★★★
()
Ответ на: комментарий от soomrack

С чего ты взял что оно будет unsigned int? Оно будет signed. Куда там влезает умножение - вообще плевать, это тут ни при чём. Промоутится не результат а аргументы.

И хватит ставить мне фейспалмы, лучше почитай стандарт. А конкретно своё сложение (или умножение) можешь и компилятором проверить.

#include <stdio.h>

int main(void) {
  unsigned short a,b;
  unsigned int c,d;
  a = b = c = d = 0xFFFF;
  printf("%u\n", (a*b)/10);
  printf("%u\n", (c*d)/10);
  printf("%u\n", (0xFFFF*0xFFFF)/10);
  return 0;
}

Второй printf выдаст правильный ответ, первый и третий - неправильный из-за int-переполнения.

firkax ★★★★★
()
Последнее исправление: firkax (всего исправлений: 5)
Ответ на: комментарий от firkax

Да, я немного погорячился.

  1. short * int превратится в int * int
  2. (что-то короче int) * int превратится в int * int
  3. (что-то короче int) * (что-то короче int) превратится в int * int
  4. unsigned int * int превратится в unsigned int * unsigned int
  5. long long int * unsigned int превратится в long long int * long long int
soomrack ★★★★★
()
Ответ на: комментарий от firkax

Нет. unsigned short int останется само собой (не превратится в unsigned int) если sizeof(unsigned short)>=sizeof(int).

Строго больше быть не может в смысле точности, поэтому значения short int всегда влезут в int, поэтому short int всегда промоутится до int.

soomrack ★★★★★
()
Последнее исправление: soomrack (всего исправлений: 1)
Ответ на: комментарий от soomrack

Больше не может, но может быть равно. И если равно, то результат short*short будет типа short. Который хоть и то же самое что int, но формально всё-таки другой тип. На что это может повлиять я не знаю. Может быть, ни на что. Если бы был автоматический вывод типов, то можно было бы сделать такое:

void f(short a, short b) {
  auto c = a*b;
  int *d;
  d = &c;
}

И на 16-битной платформе оно выдаст как минимум варнинг о несоответствии типа указателя, поскольку c будет типа short, а типы int* и short* - это разные типы, несмотря на полностью идентичное внутреннее устройство.

firkax ★★★★★
()
Последнее исправление: firkax (всего исправлений: 3)
Ответ на: комментарий от firkax

Равно может, но в этом случае оно же влезает в int и в соотв. со стандартом C11 п.6.3.1.1, оно должно превратится в int. Да, это будет тот же массив бит, но формально другой тип, и должны будут появиться ворнинги в ряде случаев, правда я пока не могу придумать когда даже (на примере long long int и long int).

soomrack ★★★★★
()
Последнее исправление: soomrack (всего исправлений: 4)
Ответ на: комментарий от soomrack

Ну строго говоря, char это не обязательно один байт, он должен быть достаточным, чтобы хранить символ (необходимых чуть больше 90, кажется, по стандарту).

Именно один байт. Но не обязательно 8 бит. (Это только привычка, что байт - это синоним 8 бит).

gag ★★★★★
()
Ответ на: комментарий от soomrack

А да, ты прав, в promotions не по sizeof() конвертация а по rank, которые char < short < int строго, то есть short всегда превращается в int независимо от разрядности. И появляется исключение из «всегда в signed int» - как раз на 16-битной системе unsigned short «превращается» в unsigned int а не в signed. Я про это забыл, поскольку оно реально кажется никаких внешних эффектов не создаёт, считал что при одинаковом sizeof не будет ничего превращаться.

Варнингов из этого кажется никак не сделать.

firkax ★★★★★
()
Последнее исправление: firkax (всего исправлений: 4)
Ответ на: комментарий от firkax

Есть в С++, по части достандартного кода. Отключается только с -fpermissive.

Ещё попадал на неотключаемый варнинг с объявлением типа в аргументах функции и последующим typedef. Тоже из древнего кода ловил, потом правил.

a1ba ★★
()
Ответ на: комментарий от anonymous

А что со знаковостью?

Извините - вынужденно отсутствовал. Разобрались? Тут успели наговорить много несоответствующего действительности. Начиная с того что promotion из char / short в int имеет место быть всегда. Вещь которая возможно неочевидна - char / short / int / long / long long (и их unsigned counterparts) с точки зрения ABI являются разными типами, даже если sizeof() and everything else is the same.

bugfixer ★★★★★
()
Ответ на: комментарий от wandrien

Думал самостоятельно vs прочитал диагностику GCC. )

Одно другому не мешает.

Фактически и ты и @Zhbert (и K&R) правы. Более того, твоё утверждение не противоречит K&R, и утверждение K&R не противоречит твоему: ты сам решаешь, что есть ошибка в коде, а что нет, GCC/llvm и/или статический анализаторы кода тебе лишь помогают найти потенциальные проблемы, а что из этого является фактической проблемой кроме тебя, как программиста этого самого кода, никто знать не может. Собственно, в п.2 ты именно это и написал, просто изложил иначе.

mord0d ★★★★★
()
Ответ на: комментарий от anonymous

Вот это я не пойму. Размер что int, что unsigned будет один, согласно чему знаковость будет по-разному меняться?

Одинаковость sizeof() не означает покрытие диапазона: -1 ни в один unsigned X не уложится, вне зависимости от X.

bugfixer ★★★★★
()
Ответ на: комментарий от wandrien

где размеры char и short равны размеру int.

Я, если честно, запамятовал - в 16ти битах short он 8ми или 16ти битный? Готов предположить что от компилятора могло зависеть.

bugfixer ★★★★★
()
Последнее исправление: bugfixer (всего исправлений: 2)
Ответ на: комментарий от bugfixer

Эти размеры зависят от конкретных «правил игры» в рамках ОС, libc, режима компиляции и т.п. Не от самого Си.

Конкретно под DOS скорее всего на всех режимах компиляции было short == int == 16 бит, и long == 32 бита. А long long в то время не было вообще.

Но я только под Turbo C писал, другие компиляторы не смотрел.

wandrien ★★★
()
Ответ на: комментарий от bugfixer

К сожалению нет. Еще больше запутался. Смотрите, такой пример:

#include <stdio.h>

unsigned short u_s_1 = 1;
unsigned short u_s_2 = 1;
unsigned u_i = 1;
int s_i = 1;
short s_s = 1;

int main()
{
    typeof(u_s_1 + u_s_2) a = 0;
    a -= 10;
    if (a < 0) printf("signed\n");

    if (u_i < u_s_1 + u_s_2) a++;

    if (u_i < a) a++;

    if (u_i < s_i) a++;

    if (u_i < s_s) a++;

    return 0;
}

> gcc -O0 -Wall -Wextra -pedantic tmp.c && ./a.out
tmp.c: In function ‘main’:
tmp.c:17:13: warning: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Wsign-compare]
   17 |     if (u_i < a) a++;
      |             ^
tmp.c:19:13: warning: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Wsign-compare]
   19 |     if (u_i < s_i) a++;
      |             ^
tmp.c:21:13: warning: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘short int’ [-Wsign-compare]
   21 |     if (u_i < s_s) a++;
      |             ^
signed

Кто-то может из уважаемых участников треда обьяснить, почему нет варинга для «if (u_i < u_s_1 + u_s_2)», а для «if (u_i < a)» есть?

anonymous
()
Ответ на: комментарий от anonymous

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

Сравнение будет выполнено как беззнаковое, и все операнды выражения так же беззнаковые. Результат сложения побитово от знаковости не зависит (на арифметике с two’s complement representation).

wandrien ★★★
()
Ответ на: комментарий от anonymous

if (u_i < u_s_1 + u_s_2)

unsigned и unsigned – норм

а для «if (u_i < a)

unsigned и signed – варнинг

typeof(u_s_1 + u_s_2) a = 0;

unsigned short int + unsigned short int (если они короче int) превращаются в int + int, в итоге int, т.е. a имеет тип signed int.

soomrack ★★★★★
()
Последнее исправление: soomrack (всего исправлений: 2)
Ответ на: комментарий от anonymous

я наврал, будет все же:

if (u_i < u_s_1 + u_s_2)

unsigned и signed – ненорм, но варнинга нет

if (u_i < u_s_1 + u_s_2 + u_s_2)

unsigned и signed – ненорм, gcc – варнинг есть, clang – варнинга нет.

Компилятор просто не ловит эти ситуации, имхо.

soomrack ★★★★★
()
Последнее исправление: soomrack (всего исправлений: 1)