<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Sat, Aug 18, 2018 at 11:47 AM JVApen via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi all,<br><br>It looks like I did not get the responses of this thread in my mailbox.<br>Let me try to pick in on: <a href="http://lists.llvm.org/pipermail/cfe-dev/2018-August/058861.html" target="_blank">http://lists.llvm.org/pipermail/cfe-dev/2018-August/058861.html</a> and <a href="http://lists.llvm.org/pipermail/cfe-dev/2018-August/058903.html" target="_blank">http://lists.llvm.org/pipermail/cfe-dev/2018-August/058903.html</a><br><br>If I understood correctly, clang-tidy warns on all occurrences of missing overrides while the compiler warning suppresses the warning if override doesn't appear in the class.<br>(PS: This nuance ain't clear from reading <a href="http://clang.llvm.org/docs/DiagnosticsReference.html#id300" target="_blank">http://clang.llvm.org/docs/DiagnosticsReference.html#id300</a> or <a href="https://clang.llvm.org/extra/clang-tidy/checks/modernize-use-override.html" target="_blank">https://clang.llvm.org/extra/clang-tidy/checks/modernize-use-override.html</a>)<br><br>From a personal perspective, I enable -Werror and -Weverything.<br>With this, I can disable (and document why) warnings I don't like (as the C++98 compatibility) or suppress the warnings locally when explicitly violating.<br>From this perspective, I agree with Matthieu that this makes sense for every occurrence, especially in old code bases since I want to all hidden cases to be fixed asap.<br></div></blockquote><div><br></div><div>It's still a rather stylistic choice about how you write your C++ code - it has a very low rate of finding bugs on a codebase that isn't already override-clean. (ie: it'll suggest /lots/ of code changes (adding override everywhere) but very few of those highlighted locations will be bugs).</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">(I might care less if we manage to setup an automated system for checking the clang-tidy warnings to ensure that override ain't forgotten in new files.)<br></div></blockquote><div><br></div><div>Yeah, I'd love to see better ways of automating clang-tidy into an existing build system for adding new/other diagnostics (likely not suitable for the cases where clang-tidy diagnostics are there because they're too slow for normal compiles, but when they're too stylistic/esoteric for clang, but still fast enough - would be great to be able to still get them as part of the build & have them fail the build under -Werror, etc)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I also understand the reasoning of Dave for why we have clang-tidy as a separate executable: some checks can take a long time to run or need to keep track of a lot of state.<br>However, if I'm understanding this case correctly, neither of these conditions are met as the compiler code already detects the issue and decides to ignore it since no override is available in that file.<br>If we really care about keeping this distinction, would reporting this case in a separate warning (-Winconsistent-missing-override-pedantic) be a solution both scenarios, similar to -Winconsistent-missing-destructor-override?<br></div></blockquote><div><br>If it were added, yes it'd be under a separate flag (though not likely with the "inconsistent" part of the name - because that's specifically about the current low-false positive filter (where the warning only fires if the missing override is inconsistent with the rest of the class (because there are overrides specified elsewhere in the class)).<br><br>- Dave<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br>Thanks!<br>PS: I could not find the code on github where the warning was actually triggered.<br></div><br><div class="gmail_quote"><div dir="ltr">On Thu, 9 Aug 2018 at 19:26, JVApen <<a href="mailto:JVApen@gmail.com" target="_blank">JVApen@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="auto">Hello all,<div dir="auto"><br></div><div dir="auto">Today we have discovered a case where we did not have a compiler warning for a missing override. To verify, we explicitly enabled the warning with the pragma.</div><div dir="auto"><br></div><div dir="auto">While investigating this in the IDE, we noticed that the Visual Assist plugin did notice that missing override. As this uses clang-tidy to determine this, we were wondering if these differences we noticed are intentional. If so, is their any documentation on these differences?</div><div dir="auto"><br></div><div dir="auto">Tnx</div></div>
</blockquote></div>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote></div></div>