Are you OK to commit this patch or do you see more issues? //Anders ________________________________________ Från: Anna Zaks [[email protected]] Skickat: den 9 december 2013 23:06 Till: Daniel Marjamäki; Anders Rönnholm Ämne: Re: [PATCH] [StaticAnalyzer] New checker Sizeof on expression
CC-ing Anders. On Dec 4, 2013, at 11:35 AM, Anna Zaks <[email protected]<mailto:[email protected]>> wrote: Off the list. Daniel, I don't see any outstanding issues with the patch. I would reply to this email, CC all the people who reviewed this patch and ask if it is OK to commit it. Cheers, Anna. On Nov 27, 2013, at 7:27 PM, Matt Calabrese <[email protected]<mailto:[email protected]>> wrote: On Wed, Nov 27, 2013 at 8:54 AM, Daniel Marjamäki <[email protected]<mailto:[email protected]>> wrote: Hello! >> Sure. I think it's even fine for a compiler warning, since sizeof(size_t) is >> a clearer way to get the same result. > I know I'm a little late here, but I think the reason why you wouldn't want > to use sizeof(size_t) is that you need to include certain standard library > headers to pull in size_t, I.E. <cstddef>. The same goes for ptrdiff_t. Doing > sizeof(sizeof(whatever)) frees you from that requirement. To me it sounds like they prefer misleading code instead of an #include then. I wonder if you or anybody else here have seen such project. Or was it an idea when there could in theory be false positives? I'm just looking for potential false positives and trying to understand the rationale of such valid code. I'm not one of the people who has done sizeof(sizeof(whatever)), so I can't speak for them, but I don't think it's that unreasonable to want to avoid including a standard header file. One reason might be that you simply might not have a standard library implementation available. Not wanting to pull in unnecessary code into translation units that depend on it is another. As for the SFINAE concern, that is a use-case that I genuinely have encountered (and many have encountered, specifically when implementing type traits for automatically detecting expression validity, such as if + is defined for a given pair of operands). That said, regarding SFINAE, there are alternatives that don't require sizeof and that are generally better overall anyway. In particular, using decltype instead of sizeof in such situations is the better approach since decltype works in places where the expression results in void (whereas the sizeof version would break) and it doesn't require instantiating the argument whereas sizeof does. Using decltype would also not trigger this warning. Simply not doing the check in such SFINAE contexts obviously gets rid of such false positives too, but it's worth noting that these uses of sizeof are still dubious even with respect to SFINAE in modern code. Because of that, recommending decltype instead of sizeof could even be a possibility here rather than not producing a warning during SFINAE at all. This has the benefit of alerting people to the more subtle bugs coming from using sizeof as opposed to decltype for expression validity tests. I want to get this warning into clang. I wonder if it will make you happy if we only write the warning only when we see a #include anywhere in the file. If we see an #include then the project uses includes. Most real files include files so this heuristic wouldn't hurt the hit rate much imho. I'm happy already without stuff like that. Again, I'm just trying to enumerate and understand the potential false positives, especially if it makes it possible to recommend an alternative when the warning is triggered. -- -Matt Calabrese _______________________________________________ cfe-commits mailing list [email protected]<mailto:[email protected]> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
sizeofonexpression.diff
Description: sizeofonexpression.diff
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
