<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 5 December 2016 at 09:05, Stephan Bergmann via cfe-dev <span dir="ltr"><<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-m_-193018081956014813m_-3942641379453014096gmail-">On 12/04/2016 11:20 PM, Mat Sutcliffe via cfe-dev wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Comments?<br>
</blockquote>
<br></span>
In LibreOffice, we use a plugin to detect those (<<a href="https://cgit.freedesktop.org/libreoffice/core/tree/compilerplugins/clang/staticaccess.cxx" rel="noreferrer" target="_blank">https://cgit.freedesktop.org<wbr>/libreoffice/core/tree/compile<wbr>rplugins/clang/staticaccess.cx<wbr>x</a>>), for two reasons:<br>
<br>
* There were cases where removing the redundant object expression made some variables or function parameters become unused, which allowed further code clean-up.<br>
<br>
* In some cases MSVC indirectly warns about such redundant object expressions, when they involve otherwise unused function parameters (producing a warning about an unused parameter, which can be somewhat puzzling given the parameter /is/ used in the program text).  So we could just as well warn about these things consistently.<br>
<br>
On the other hand, these warnings are sometimes perceived as being on the border of "style police", so likely makes sense to have them only enabled explicitly.<br></blockquote><div><br></div><div>Good to know. I went the route of implementing a compiler warning after I read the following in <<a href="http://clang-analyzer.llvm.org/checker_dev_manual.html#ast" target="_blank">http://clang-analyzer.llvm.or<wbr>g/checker_dev_manual.html#ast</a>><wbr>: "Some checks might not require path-sensitivity to be effective. Simple AST walk might be sufficient. If that is the case, consider implementing a Clang compiler warning."</div><div><br></div><div>So now the question is whether a plugin is satisfactory, or if there would be sufficient benefit to having a warning available in the core compiler. I believe there would be, but of course I am biased. The existence of a plugin shows that there is at least one other user who desires this feature, and that the concept has received a certain amount of testing already. I think it meets the seven criteria at <<a href="http://clang.llvm.org/get_involved.html" target="_blank">http://clang.llvm.org/get_inv<wbr>olved.html</a>>. The advantages of a warning available in the vanilla compiler would be increased user reach, less burdensome usage, exposure to more vigorous testing. I think the runtime overhead would be negligible, as the diagnosis fits naturally into the procedure for building a member access expression.</div><div><br></div><div>Maybe it's too simple an issue for anyone to waste time talking about, so it might just be a case of submitting the patch and seeing what happens. I'm just being extra cautious because it's my first patch to LLVM. Coincidentally, this came up on StackOverflow just the other day: <<a href="http://stackoverflow.com/q/41018973/1639256" target="_blank">http://stackoverflow.com/q/41<wbr>018973/1639256</a>></div><div><br></div><div>I don't have much to say on the overreach of the style police. Because of the large body of user code that would trigger this warning, and which is required by coding standards to compile cleanly with -Wall -Wextra it would be disabled by default and not controlled by either of those flags. It seems no more authoritarian than many existing warnings and I'm aware of no potential false positives or justifiable leniency, except for cases I've previously identified which can be easily filtered out.</div></div></div></div>