Pull to refresh

Comments 17

Есть ли в этом всём какая-то реальная польза в условиях отсутствия межпроцедурного анализа (который, я так понимаю, у вас не реализован)? Ведь значения, приходящие в функцию, по факту, уже ограничены. А если считать их неограниченными (==ограниченными размером типа), то сплошные false-positive пойдут.
Польза есть — это видно из примеров, особенно в первом примере явный баг. В большинстве случаев находятся просто лишние проверки, но это не false positive.
Как по мне, происходящее в первом примере с натяжкой можно назвать багом. По-моему, там такая же лишняя проверка.
Проверка там как раз не лишняя. Ошибка в первом условии, из-за чего специальная оптимизация для пробела никогда не выполняется. Это баг.
По факту значения приходящие в функцию ничем не ограниченны, при проверке и проектировании нельзя опираться на предположение о корректном поведении потребителя.
Использование контрактов на функции никто не отменял. Довольно распространённый метод проектирования, кстати. Там как раз используются предположения о корректном поведении потребителя, которые можно выразить в виде некоторого предусловия над входными данными.
Если это предусловие написано в виде if (...) throw new ArgumentException(...) — то, в соответствии с разделом «Уточнение виртуальных значений в блоке if/else», анализатор это условие учтет.
Контракты со временем тоже поддержим и будем учитывать
Зато можно опираться на имеющиеся ограничения на аргументы, контекст вызова/выполнения функции которые доступны при межпроцедурном анализе.
Если функция публичная, то она может быть вызвана откуда угодно, и про аргументы тогда ничего нельзя сказать.
Имхо, переменные, попадающие в функцию, надо проверять всегда.

Может быть вы и не задумывали использование функции в каких-либо условиях, но ваш напарник может об этом не знать при написании кода. Даже если и есть документация на проект, он её мог не читать. Или вы сами, в конце концов, можете забыть про ограничения конкретной функции. Или это может произойти из-за бага.

Мы не в идеальном мире живём, в конце концов.
«Анализатор знает про то, что свойство Count у списка не может принимать отрицательные значения».

Откуда он это знает? Ведь в качестве size_t в .NET используется Int32, который может принимать и отрицательные значения тоже (собственно, можно создать массив с отрицательным начальным индексом), да и официальная документация не даёт на счёт области значений List.Count никаких гарантий, кроме неявно вытекающих из слова «number [of elements]». Другое дело, что в самом коде реализации get_Count имеется соглашение Contract.Ensures(Contract.Result() >= 0), но вы же сами признаётесь, что контракты ещё не учитываются.

То есть анализатор использует эмпирически захардкоженные значения для системных классов? А если я унаследуюсь от List, то магия улетучится? А если я создам с нуля свой собственный SuperList, то тоже буду в пролёте?
Да, мы захардкодили информацию о стандартных классах .NET Framework, которая может быть нам полезна при статическом анализе. Для наследников системных классов это тоже будет работать. Для вашего SuperList — нет, в будущем мы возможно сделаем поддержку пользовательских аннотаций для своих классов.
По поводу List.Count — мы не анализируем исходники .NET Framework и соответственно не учитываем их конткракты, то что Count — это количество элементов нам достаточно, чтобы считать его неотрицательным.
А почему бы не идти от стандартных интерфейсов? По-идее, у любой реализации ICollection или ICollection<> свойство Count означает количество элементов и не может быть отрицательным.
Мы сначала так и хотели сделать, но возникли некоторые сложности. Например, List.Reverse() переставляет элементы списка, а IEnumerable.Reverse() возвращает новую коллекцию, соответственно во втором случае анализатор должен сказать что результат вызова Reverse() нужно использовать. В общем, нам просто было удобнее проаннотировать классы, чем как-то различать такие похожие случаи.
Но ведь метод List.Reverse не является реализацией Enumerable.Reverse! И, кстати, последний метод не является членом IEnumerable
Не является, но поддержать нам его в любом случае надо, поэтому нам его нужно как-то у себя зарегистрировать.
Сначала мы зарегистрировали Enumerable.Reverse как метод IEnumerable и List.Reverse как метод List и при анализе выражения list.Reverse() пытались выбрать из двух вариантов. После этого решили зарегистрировать List.Reverse как метод List и Enumerable.Reverse как метод Enumerable.
Sign up to leave a comment.