<div dir="ltr"><div dir="ltr">Apologies for the inconvenience, though I wonder why I haven't gotten builtbot mails :/</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 16 Jun 2020 at 09:54, Haojian Wu <<a href="mailto:hokein.wu@gmail.com">hokein.wu@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr">On Mon, 15 Jun 2020 at 18:29, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">I don't think the comment's adding value here - it should be fairly<br>
clear from the context that the whole loop only exists for some<br>
assertions.<br>
<br>
Also: Could you remove the "(void)" casts now that the whole thing's<br>
wrapped in NDEBUG?<br></blockquote><div><br></div><div>oops, I think this is coincident, there were two patches at the same time to fix the issue.</div><div>Cleaned it up in  e00dcf61a2f4397c0525db7133503b80d8ecf5ac.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
(an alternative way of phrasing this that doesn't require the #ifdef<br>
would be using nested llvm::all_of calls, but that would mean the<br>
assertion firing wouldn't point to a particular mismatched pair, just<br>
that overall the data structures were mismatched - if you think that<br>
this isn't especially likely/wouldn't be super hard to debug with<br>
that, maybe the assert would be more compact/tidier?<br>
<br>
assert(llvm::all_of(Dependencies, [&](const auto &DepPair) {<br>
  return llvm::all_of(WeakDependencies, [&](const auto &WeakDepPair) {<br>
    return WeakDepPair != DepPair && WeakDepPair.first !=<br>
DepPair.second && WeakDepPair.second != DepPair.second;<br>
  }<br>
}) && "...");<br></blockquote><div><br></div><div>I'm not the author of this code, defer the decision to the author Kirstóf, <a href="mailto:dkszelethus@gmail.com" target="_blank">dkszelethus@gmail.com</a>.</div><div> </div></div></div></blockquote><div><br></div><div>That is a great suggestion, but I find the individual asserts (with a message!) a better solution to guide the developer should an error be made in the TableGen file.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
On Fri, Jun 12, 2020 at 6:45 AM Haojian Wu via cfe-commits<br>
<<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br>
><br>
><br>
> Author: Haojian Wu<br>
> Date: 2020-06-12T15:42:29+02:00<br>
> New Revision: e4b3fc18d33199e2081d300f14687d81be48b6a0<br>
><br>
> URL: <a href="https://github.com/llvm/llvm-project/commit/e4b3fc18d33199e2081d300f14687d81be48b6a0" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/e4b3fc18d33199e2081d300f14687d81be48b6a0</a><br>
> DIFF: <a href="https://github.com/llvm/llvm-project/commit/e4b3fc18d33199e2081d300f14687d81be48b6a0.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/e4b3fc18d33199e2081d300f14687d81be48b6a0.diff</a><br>
><br>
> LOG: Get rid of -Wunused warnings in release build, NFC.<br>
><br>
> Added:<br>
><br>
><br>
> Modified:<br>
>     clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp<br>
><br>
> Removed:<br>
><br>
><br>
><br>
> ################################################################################<br>
> diff  --git a/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp b/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp<br>
> index c2ca9c12b025..4a7e0d91ea23 100644<br>
> --- a/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp<br>
> +++ b/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp<br>
> @@ -285,6 +285,7 @@ CheckerRegistry::CheckerRegistry(<br>
>    resolveDependencies<true>();<br>
>    resolveDependencies<false>();<br>
><br>
> +#ifndef NDEBUG // avoid -Wunused warnings in release build.<br>
>    for (auto &DepPair : Dependencies) {<br>
>      for (auto &WeakDepPair : WeakDependencies) {<br>
>        // Some assertions to enforce that strong dependencies are relations in<br>
> @@ -298,6 +299,7 @@ CheckerRegistry::CheckerRegistry(<br>
>               "A strong dependency mustn't be a weak dependency as well!");<br>
>      }<br>
>    }<br>
> +#endif<br>
><br>
>    resolveCheckerAndPackageOptions();<br>
><br>
><br>
><br>
><br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div>
</blockquote></div></div>