r235190 - Remove the assertion as it was useless and broken.

[Jordan Rose]

> On Apr 28, 2015, at 20:57 , Jordan Rose <jordan_rose at apple.com> wrote:
> 
>> 
>> On Apr 17, 2015, at 6:37 , Aaron Ballman <aaron at aaronballman.com <mailto:aaron at aaronballman.com>> wrote:
>> 
>> On Fri, Apr 17, 2015 at 9:35 AM, Sylvestre Ledru <sylvestre at debian.org <mailto:sylvestre at debian.org>> wrote:
>>> On 17/04/2015 15:33, Aaron Ballman wrote:
>>>> On Fri, Apr 17, 2015 at 9:21 AM, Sylvestre Ledru <sylvestre at debian.org <mailto:sylvestre at debian.org>> wrote:
>>>>> Author: sylvestre
>>>>> Date: Fri Apr 17 08:21:39 2015
>>>>> New Revision: 235190
>>>>> 
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=235190&view=rev <http://llvm.org/viewvc/llvm-project?rev=235190&view=rev>
>>>>> Log:
>>>>> Remove the assertion as it was useless and broken.
>>>>> 
>>>>> Enforcing the assert caused the following tests to fail:
>>>>> Clang :: Analysis__bstring.c
>>>>> Clang :: Analysis__comparison-implicit-casts.cpp
>>>>> Clang :: Analysis__malloc-interprocedural.c
>>>>> Clang :: Analysis__malloc.c
>>>>> Clang :: Analysis__redefined_system.c
>>>>> Clang :: Analysis__string.c
>>>>> Clang :: Analysis__weak-functions.c
>>>> 
>>>> While the assert may have been broken, I am concerned that the
>>>> author's assumptions are being violated in some way. Can the original
>>>> code author weigh in on whether that assert is truly useless or not?
>>>> That appears to be Jordan in this case, according to a quick svn
>>>> blame.
>>> Yes, sorry about that. I fixed it quickly and maybe not using the best way.
>>> However, the incorrect assertion (fixed r235188) has been there for a few years.
>> 
>> I agree that this is an improvement over broken bots and an assert
>> that behaves differently in debug vs release mode. ;-) I just want to
>> make sure we double-check that this is the right move long-term.
> 
> I wrote this a long time ago, but the intent was that you wouldn't get to this point without setting CurrentFunctionDescription for your specific caller. I haven't actually looked at how it's being called now, though, since CStringChecker is a beta checker.

Ah. The code as written originally was correct:

  // Make sure each function sets its own description.
  // (But don't bother in a release build.)
  assert(!(CurrentFunctionDescription = nullptr));

...except that the canonical way to write this is

#ifndef NDEBUG
CurrentFunctionDescription = nullptr;
#endif

and apparently past me didn't know that.

Jordan

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150428/cbd0b03d/attachment.html>