<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Aug 29, 2016, at 10:50 AM, Craig, Ben <<a href="mailto:ben.craig@codeaurora.org" class="">ben.craig@codeaurora.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class="">
<meta content="text/html; charset=utf-8" http-equiv="Content-Type" class="">
<div bgcolor="#FFFFFF" text="#000000" class="">
On 8/29/2016 11:49 AM, Anna Zaks wrote:<br class="">
<blockquote cite="mid:1BE0EF8F-3A60-4DE5-82AB-E1C976E04CCB@apple.com" type="cite" class="">
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" class="">
<br class="">
<div class="">
<blockquote type="cite" class="">
<div class="">On Aug 29, 2016, at 7:59 AM, Gábor Horváth <<a moz-do-not-send="true" href="mailto:xazax.hun@gmail.com" class="">xazax.hun@gmail.com</a>> wrote:</div>
</blockquote>
</div>
<div class="">
<blockquote type="cite" class="">
<div class="">
<div dir="ltr" class="">
<div class="">
<div class="">
<div class="">
<div class="">
<div class="">* Using pragmas one can introduce
convenience macros, it might also be better to
suppress code regions<br class="">
</div>
* It is better to suppress based on bug type or
something like that rather than using the name of
the checker (which is not intended to leak out)<br class="">
</div>
* It is good idea to have line offsets to avoid too
much noise when reading code<br class="">
</div>
<div class="">* Anything against pragmas?<br class="">
</div>
</div>
</div>
</div>
</div>
</blockquote>
<div class=""><br class="">
</div>
Another reason pragmas are better than comments is because this
is what the compiler uses for suppressions.</div>
</blockquote>
<br class="">
I'm not opposed to pragmas in concept, but I don't like the typical
way they are used for compiler warning suppression. In particular,
I strongly dislike the following pattern as the "default" pattern
for warning suppression:<br class="">
<br class="">
#if __clang_analyzer__<br class="">
#pragma static analyzer push<br class="">
#pragma static analyzer disable deadcode<br class="">
#endif<br class="">
// bad code here<br class="">
#if __clang_analyzer__<br class="">
#pragma static analyzer pop<br class="">
#endif<br class="">
<br class="">
Things I don't like about that pattern:<br class="">
* verbose<br class=""></div></div></blockquote><div><br class=""></div><div>I think a certain amount of verbosity is preferable here to steer users away from using it unless it is actually needed. This is a suppression of last resort.</div><br class=""><blockquote type="cite" class=""><div class=""><div bgcolor="#FFFFFF" text="#000000" class="">
* requires modifying code before and after the offending line of
code<br class="">
* usually requires compiler specific guards.<br class="">
* if you forget the trailer, code still works, but you lose
diagnostics<br class="">
* if you don't know the push / pop pattern, you end up losing
diagnostics<br class=""></div></div></blockquote><div><br class=""></div><div>We can always diagnose this. Well-named macros would also mitigate the problem:</div><div><br class=""></div><div>CLANG_ANALYZER_SUPPRESSION_BEGIN()</div><div>CLANG_ANALYZER_SUPPRESSION_END()</div><br class=""><blockquote type="cite" class=""><div class=""><div bgcolor="#FFFFFF" text="#000000" class="">I'm fine with that pattern existing in addition to a much more terse
and friendly syntax. I can see the push / pop pattern being useful
for blocks of code. Just don't make that the default.<br class="">
<br class="">
Here's a pattern that I'd prefer:<br class="">
// Common header...<br class="">
#ifdef __clang_analyzer__<br class="">
#define CLANG_SUPPRESS_WARNING(category, line_offset)
_Pragma(category, line_offset)<br class="">
#else<br class="">
#define CLANG_SUPPRESS_WARNING(category, line_offset)<br class="">
#endif<br class="">
// Source file...<br class="">
CLANG_SUPPRESS_WARNING(deadcode, +1)<br class="">
// bad code here<br class="">
</div></div></blockquote><br class=""></div><div>Any offsets that aren’t +/-1 strike me as very fragile in the face of code evolution. They would make it very easy to have bad merges that lose the suppression. Also: what do you envision the meaning of “line” being within a macro expansion? Would this make it impossible to suppress diagnostics in an expansion itself?</div><br class=""><div class="">Devin</div></body></html>