Как плохо "если (!это) " в функции-члене C++?

Как и все неопределенное поведение

if (!this)
{
   return NULL;
}

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

следующий выпуск тех же компиляторов может быть более агрессивным и рассматривать это как мертвый код. Как this никогда не может быть null, код может быть "безопасно" удален.

Я думаю, что это лучше, если вы удалили его!

если у вас есть много GetLookup функции возвращают NULL, то вам лучше исправить код, который вызывает методы с помощью нулевого указателя. Во-первых, заменить

if (!this) return NULL;

С

if (!this) {
  // TODO(Crashworks): Replace this case with an assertion on July, 2012, once all callers are fixed.
  printf("Please mail the following stack trace to myemailaddress. Thanks!");
  print_stacktrace();
  return NULL;
}

теперь продолжайте свою другую работу, но исправьте их, когда они закатываются. Заменить:

GetLookup(x)->Lookup(y)...

С

convert_to_proxy(GetLookup(x))->Lookup(y)...

где conver_to_proxy делает возвращает указатель без изменений, если это не NULL, в этом случае он возвращает FailedLookupObject как и в моем другом ответ.

Он не может аварийно завершить работу в большинстве компиляторов, так как невиртуальные функции обычно либо встроены, либо переведены в функции, не являющиеся членами, принимая "это" в качестве параметра. Однако стандарт конкретно говорит, что вызов нестатической функции-члена вне времени существования объекта не определен, и время существования объекта определяется как начало, когда память для объекта была выделена и конструктор завершен, если он имеет нетривиальную инициализацию.

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

TL: DR: я бы убил его огнем, даже если это займет много времени, чтобы очистить все сайты вызовов.

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

Это то, что называется "умный и уродливый Хак". Примечание: умный != мудрый.

найти все сайты вызовов без каких-либо инструментов рефакторинга должно быть достаточно легко; разбить GetLookup() так или иначе, чтобы он не компилировался (например, изменить подпись), чтобы вы могли статически идентифицировать неправильное использование. затем добавьте функцию DoLookup (), которая делает то, что все эти хаки делают прямо сейчас.

в этом случае я бы предложил удалить нулевую проверку из функции-члена и создать функцию, не являющуюся членом

CThingy* SafeLookup(CLookupThing *lookupThing) {
  if (lookupThing == NULL) {
    return NULL;
  } else {
    return lookupThing->Lookup();
  }
}

тогда это должно быть достаточно легко найти каждый вызов Lookup функция-член и замените ее безопасной функцией, не являющейся членом.

Если это что-то, что беспокоит вас сегодня, это будет беспокоить вас через год. Как вы указали, изменение его почти наверняка приведет к некоторым ошибкам , но вы можете начать с сохранения return NULL функциональность, добавить немного журнала, пусть он работает в дикой природе в течение нескольких недель, и найти, сколько раз он даже получает удар?

Вы можете безопасно исправить это сегодня возвращая неудачный объект поиска.

class CLookupThingy: public Interface {
  // ...
}

class CFailedLookupThingy: public Interface {
 public:
  CThingy* Lookup(string const& name) {
    return NULL;
  }
  operator bool() const { return false; }  // So that GetLookup() can be tested in a condition.
} failed_lookup;

Interface *GetLookup() {
  if (notReady())
    return &failed_lookup;
  // else etc...
}

этот код все еще работает:

CThingy *pFoo = GetLookup()->Lookup( "foo" ); // will set pFoo to NULL without crashing

это "бомба замедленного действия", только если вы педантичны в формулировке спецификации. Однако, несмотря на это, это ужасный, непродуманный подход, потому что он скрывает ошибку программы. Только по этой причине я бы удалить его, даже если это означает значительную работу. Это не непосредственный (или даже среднесрочный) риск, но это просто не очень хороший подход.

такое поведение сокрытия ошибок действительно не то, на что вы хотите положиться. Представьте, что вы полагаетесь на такое поведение (т. е. это не имеет значения, являются ли мои объекты действительными, он будет работать в любом случае!) и затем, по некоторой опасности, компилятор оптимизирует if заявление в конкретном случае, потому что он может доказать, что this не является нулевым указателем. Это законная оптимизация не только для некоторых гипотетических будущих компиляторов, но и для очень реальных компиляторов настоящего времени.
Но, конечно, поскольку ваша программа не очень хорошо сформирована, это происходит что в в какой-то момент Вы передаете ему null this около 20 углам. Бах, ты мертв.
Это очень сокрушенно, по общему признанию, и этого не произойдет, но вы не можете быть на 100% уверены, что это все еще не может произойдет.

обратите внимание, что когда я кричу " удалить!", это не значит, что все они должны быть удалены тут или в одной массивной деятельности силы человека. Вы можете удалить эти проверки один за другим, как вы сталкиваетесь с ними (при изменении что-то в том же файле в любом случае, избегайте перекомпиляции) или просто текстового поиска (предпочтительно в сильно используемой функции) и удалите его.

так как вы используете GCC, вы можете быть заинтересованы в __builtin_return_address, что может помочь вам удалить эти проверки без массивной рабочей силы и полностью нарушить весь рабочий процесс и сделать приложение полностью непригодным для использования.
до удаление чека, измените его, чтобы вывести адрес вызывающего абонента, и addr2line сообщит вам местоположение в вашем источнике. Таким образом, вы должны быть в состоянии быстро определить все места в приложении, которые ведут себя неправильно, так что вы можете исправить их.

так вместо

if(!this) return 0;

изменить одно место в то время, чтобы что-то вроде:

if(!this) { __builtin_printf("!!! %p\n", __builtin_return_address(0)); return 0; }

это позволяет идентифицировать недопустимые сайты вызовов для этого изменения, все еще позволяя программе "работать по назначению" (вы также можете запросить вызывающего абонента, если это необходимо). Исправьте каждое плохое место, одно за другим. Программа все равно будет "работать" как обычно.
Как только больше адресов не появится, удалите чек alltogether. Возможно, Вам все равно придется исправить один или другой сбой, если вам не повезло (потому что он не показывался во время тестирования), но это должно быть очень редкой вещью. В любом случае, это должно помешать вашему коллеге кричать на вас.
Удалите один или два чека в неделю, и в конечном итоге ни один не останется. Между тем, жизнь продолжается и никто замечает, что ты вообще делаешь.

TL; DR
Что касается" текущих версий GCC", вы отлично подходите для невиртуальных функций, но, конечно, никто не может сказать, что может сделать будущая версия. Однако я считаю маловероятным, что будущая версия приведет к тому, что ваш код сломается. Не мало существующих проектов имеют такую проверку (я помню, что у нас были буквально сотни из них в Code::Blocks code completion в какой-то момент, не спрашивайте меня, почему!). Вероятно, создатели компиляторов не хочу, чтобы десятки/сотни крупных сопровождающих проектов были недовольны специально, только чтобы доказать свою точку зрения.
Также рассмотрим последний абзац ("с логической точки зрения"). Даже если эта проверка выйдет из строя и сгорит с будущим компилятором, она все равно выйдет из строя и сгорит.

The if(!this) return; утверждение несколько бесполезно, поскольку this никогда не может быть нулевым указателем в хорошо сформированной программой (это означает, что вы назвали функцию-член указатель на null). Это не значит, конечно конечно, это не могло произойти. Но в этом случае программа должна аварийно завершить работу или прерваться с утверждением. Ни при каких условиях такая программа не должна продолжаться молча.
С другой стороны, вполне возможно вызвать функцию-член на недействительным объект, который оказывается not null. Проверки this является нулевым указателем, очевидно, не улавливает этот случай, но это тот же самый UB. Итак, помимо сокрытия неправильного поведения, это проверка также обнаруживает только половину проблемных случаев.

если вы собираетесь по формулировке speficication, используя this (что включает в себя проверку, является ли это нулевым указателем) является неопределенным поведением. Поскольку, строго говоря, это "бомба замедленного действия". Однако я бы не достаточно считаем, что проблема, как с практической точки зрения, так и с логической единицы.

• с практические точка зрения, это не так действительно имеет значение, если вы читать указатель, который не действителен до тех пор, пока вы не разыменования его. Да, строго по букве, это не допускается. Да, теоретически кто-то может построить процессор, который будет проверять недопустимые указатели, когда вы загрузить их, и вина. Увы, это не так, если вы говорите правду.
• с логическое точка зрения, предполагая, что проверка будет взорвать, он все еще не собирается происходить. Для выполнения этого оператора должна быть вызвана функция-член и (виртуальная или нет, встроенная или нет) использует недопустимый this, который он делает доступным внутри тела функции. Если одно незаконное использование this взрывается, другой тоже будет. Таким образом, проверка устарела, потому что программа уже аварийно завершает работу ранее.

---

Н.б.: эта проверка очень похожа на "безопасное удаление идиомы", которая устанавливает указатель на nullptr после ее удаления (с помощью макроса или шаблонного

Это мое личное мнение, что вы должны как можно раньше не предупредить вас о проблемах. В этом случае я бы бесцеремонно удалил каждое появление if(!this) Я мог бы найти.