[Patch][analyzer][Review request] Move MismatchedDeallocator checker from unix.* to cplusplus.* group.

Anton Yartsev anton.yartsev at gmail.com
Thu Mar 20 15:29:16 PDT 2014


I am all for adding 'generic' category and moving new generic checkers 
into it. There are many of generic checkers in the future checkers list 
('different' category).

As for me I'd also copied generic checkers from the 'unix' 
('alpha.unix') category to 'generic' ('alpha.generic') and added a 
warning-notification to scan-build that, when one of this checkers is 
used, informs that a checker will be deprecated in the future release. 
The 'unix' is the only category that do not match its contents and 
remapping do not solve this problem. In addition, with the appearance of 
'generic' category the 'unix' category with its generic checkers will 
look more confusing.

> Jordan is right.  We can’t just move these wholesale.  We quite 
> honestly don’t know who are using these options, but they are clearly 
> exposed by scan-build.
>
> If we want to move these, we’d need a notion of an “alias”.  It 
> quickly gets complicated, however.
>
> Perhaps related, unlike clang diagnostics, we cannot have a check in 
> multiple groups.  I always saw this as a feature, as it simplified the 
> model.  That said, over time we’ve seen that checks don’t necessarily 
> fall into any single group.  If we had a cogent mechanism for putting 
> checks in multiple groups that was easy to understand then remappings 
> like this might be fairly straightforward.
>
> In the meantime, I don’t think this renaming is worth it.  That said, 
> we should consider whether or not new checks should go into a new 
> category.
>
> On Mar 20, 2014, at 9:28 AM, Jordan Rose <jordan_rose at apple.com 
> <mailto:jordan_rose at apple.com>> wrote:
>
>> Do you (either of you) think it's worth keeping compatibility names 
>> for checkers in 'unix' that move to 'generic'? I don't care so much 
>> about the checkers in 'core' because no one should be explicitly 
>> turning those on or off, but the Unix ones might be in use.
>>
>> Jordan
>>
>>
>> On Mar 19, 2014, at 18:11 , Ted Kremenek <kremenek at apple.com 
>> <mailto:kremenek at apple.com>> wrote:
>>
>>> I’d be fine with breaking ‘core’ into ‘core’ and ‘generic’, which 
>>> clearly delineates between built-in functionality that is part of 
>>> the analyzer basically doing its job and extensions to that behavior 
>>> via the use of opt-in checkers.
>>>
>>> On Mar 19, 2014, at 5:10 PM, Jordan Rose <jordan_rose at apple.com 
>>> <mailto:jordan_rose at apple.com>> wrote:
>>>
>>>> It depends if we want to move existing checkers around or not. If 
>>>> not, I'm not sure it's worth moving MismatchedDeallocators from 
>>>> where it is right now. The "unix" checkers are turned on on all 
>>>> platforms right now. If we do want to break "core" into "core" and 
>>>> "generic" then MismatchedDeallocators, along with Malloc itself, 
>>>> should be moved into "generic" (or whatever we decide to call it.)
>>>>
>>>> My inclination is to leave it where it is right now.
>>>>
>>>> (The reason to move it to cplusplus is to skip the check in C 
>>>> modes. That'd be correct right now but not in the future. We may or 
>>>> may not care.)
>>>>
>>>> Jordan
>>>>
>>>>
>>>> On Mar 19, 2014, at 17:08, Ted Kremenek <kremenek at apple.com 
>>>> <mailto:kremenek at apple.com>> wrote:
>>>>
>>>>> You didn’t really answer my question.  What is the right answer here?
>>>>>
>>>>> On Mar 19, 2014, at 2:05 PM, Jordan Rose <jordan_rose at apple.com 
>>>>> <mailto:jordan_rose at apple.com>> wrote:
>>>>>
>>>>>> "unix" includes an awful lot of generic things (like malloc 
>>>>>> itself). Part of the problem here is that our "core" checkers are 
>>>>>> always on. We don't have "generic-but-not-required" checkers 
>>>>>> right now.
>>>>>>
>>>>>> Jordan
>>>>>>
>>>>>>
>>>>>> On Mar 19, 2014, at 13:55, Ted Kremenek <kremenek at apple.com 
>>>>>> <mailto:kremenek at apple.com>> wrote:
>>>>>>
>>>>>>> By that argument, does it belong in “unix”?  What about 
>>>>>>> Windows-specific allocators?
>>>>>>>
>>>>>>> On Mar 19, 2014, at 1:45 PM, Jordan Rose <jordan_rose at apple.com 
>>>>>>> <mailto:jordan_rose at apple.com>> wrote:
>>>>>>>
>>>>>>>> It's /currently/ specific to C++, but I'm not sure it's 
>>>>>>>> /inherently/ specific to C++. I can imagine the FreeBSD guys 
>>>>>>>> adding support for custom C allocators too. So I'm not sure 
>>>>>>>> it's worth moving.
>>>>>>>>
>>>>>>>> Jordan
>>>>>>>>
>>>>>>>>
>>>>>>>> On Mar 19, 2014, at 13:27, Ted Kremenek <kremenek at apple.com 
>>>>>>>> <mailto:kremenek at apple.com>> wrote:
>>>>>>>>
>>>>>>>>> Seems fine to me.
>>>>>>>>>
>>>>>>>>> On Mar 19, 2014, at 1:12 PM, Anton Yartsev 
>>>>>>>>> <anton.yartsev at gmail.com <mailto:anton.yartsev at gmail.com>> wrote:
>>>>>>>>>
>>>>>>>>>> Hi!
>>>>>>>>>>
>>>>>>>>>> Propose to move the MismatchedDeallocator checker from unix.* 
>>>>>>>>>> to cplusplus.* group if you don't think it's too late. This 
>>>>>>>>>> check is specific for C++.
>>>>>>>>>>
>>>>>>>>>> -- 
>>>>>>>>>> Anton
>>>>>>>>>>
>>>>>>>>>> <MismatchedDeallocator_rebase.patch>_______________________________________________
>>>>>>>>>> cfe-commits mailing list
>>>>>>>>>> cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>
>>>>>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>


-- 
Anton

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


More information about the cfe-commits mailing list