Comments 23
Эмм.... А откуда исходники, версия? Я тут смотрю в поисках вот этого куска кода:
(*variables)["set_has_field_bit_message"] = ""; // <=
(*variables)["set_has_field_bit_message"] = ""; // <=И не вижу его ни в одной версии исходников O_O
1 << 4*xэто странно.
Вне зависимости от того ошибка это или нет, это код который нетривиально парзить человеку.
if (a && b || c && d) { ... } А на это:
x = 1+3*y;А почему такая разница?
if (a && b || c && d) // нет
if (a || b && c || d) // да
x = 1 + 3 * y; // нет
- Первая строка. Тот, кто писал, скорее всего понимает, что хочет объединить операцией || два подвыражения. Типовой код. Ошибки скорее всего нет.
- Вторая строка. Возможно, код работает не так, как ожидает программист. Возможно, ошибки нет, но код лучше перепроверить.
- Третья строка. Невероятно, что кто-то спутает приоритет операций + и *. Ошибки скорее всего нет.
if (a || b && c)?
Я просто правило хочу понять )))
Паттерн вида x << y + z.
Забывают, что у сложения/вычитания/умножения/деления более высокий приоритет, чем у сдвига. Лучше всегда явно ставить скобки.
Пример:
int x = a<<4 + b;Исключения:
- Операция с высоким приоритетом находится в макросе, а сдвиг нет.
- Операция с высоким приоритетом ('-','/','%') выполняется над числом 64(int64), 32(int), 16(short), 8(char).
- Разделение на строки выражения свидетельствует о преднамеренном использовании:
a = 1 << 2 * 8; //ok b = 2 << 2 * 8 //ok c = 3 << 2 * 8 //err - Нет смысл сдвигать вправо константу на константу или влево константу (кроме 1) на константу.
- Сдвиг является условием. В этом случае нет смысла результат сдвига умножать. Пример: if (value >> (i — 1) * 8)
А если речь заходит про && и ||, то это уже V648.
Странно, когда логические операции '&&' и '||' используются вместе и не разделены скобками. Часто их приоритеты путают. Приоритет оператора && выше, чем у ||.
Следует предупреждать в том случае, если написано так: A || B && C.
Исключение:
- Если имеется выражение вида A || !A && B, т.е. оператор || разделяет два противоположных по смыслу высказывания.
- Если выражение вида… || p != nullptr && f(p) ...
- Имеется выражение вида x == 0 || A != 123 && A != 321, т.е. оператор &&
разделяет сравнение одной и той же переменной с разными константами.
При этом, с одной стороны от последовательности || && ничего не должно быть.
Т.е. здесь не ругаемся: b || A != 0 && A != 1 а здесь ругаемся: b || A != 0 && A != 1 || c и здесь ругаемся: b || A != 0 && (A != 1 || c)
size_t offset = static_cast<size_t>(int_var_1 + int_var_2);Мну тут таки не согласно. Проблема тут не в int32/unit64, а скорее в signed / unsigned операциях. В зависимости от компилятора int может быть и 32 и 64, но size_t точно не меньше их размеров и точно unsigned. Так как int_var_1 и int_var_2 тут это количество и смещение (всегда неотрицательные), их сумма точно всегда влезет в size_t -- но не факт что всегда влезет в int.
Похоже здесь static_cast<size_t> был добавлен просто для того, чтобы задушить предупреждение компилятора о преобразовании знакового int в беззнаковый size_t.
Любовь Гугла к int для индексов и количества элементов в контейнере все время наталкивается на реальность стандартной библиотеки, где для этого используется size_t.
Но больше всего меня, как пользователя protobuf, раздражает их использование int для размера буфера. Ведь в Гугле уверены, что 640 кБ 2 ГБ хватит всем. Поэтому я останавливаюсь, вздыхаю и мысленно ругаюсь, когда приходится писать или читать кода вида
const std::string buf = protobufMessage.SerialzeAsString(); // ага, авторы protobuf решили, что std::string это самый подходящий тип для буфера с бинарными данными
protobufMessage.ParseFromArray(buf.data(), static_cast<int>(buf.size())); // избавляемся от предупреждения и верим что тут всегда будет меньше 2 ГБ
Со сменой типа параметров это может быть чуть проще. При желании можно добавить перегруженную версию ParseFromArray(const void* data, size_t size), опционально пометить ParseFromArray(const void* data, int size) как deprecated.
При смене возвращаемого типа тоже все решаемо, например какByteSize() -> ByteSizeLong().
Проблема в точке зрения авторов protobuf, что int для размеров это нормально.
Спасибо! Подготовил исправления по всем пунктам :)
Брутальный Protocol Buffers от Google vs статический анализ кода