> On Apr 28, 2015, at 20:57 , Jordan Rose <[email protected]> wrote: > >> >> On Apr 17, 2015, at 6:37 , Aaron Ballman <[email protected] >> <mailto:[email protected]>> wrote: >> >> On Fri, Apr 17, 2015 at 9:35 AM, Sylvestre Ledru <[email protected] >> <mailto:[email protected]>> wrote: >>> On 17/04/2015 15:33, Aaron Ballman wrote: >>>> On Fri, Apr 17, 2015 at 9:21 AM, Sylvestre Ledru <[email protected] >>>> <mailto:[email protected]>> 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
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
