<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    Do i understand correctly that the only difference between this
    checker and VirtualCallChecker is that the option is on the wrong
    checker?<br>
    <br>
    I think we should be able to eventually remove the option. Probably
    we can even do it right now, but i'll double check. <br>
    <br>
    I also believe that we don't have a strong opinion on what exactly
    does disabling the checker do. We only care about being able to
    silence both parts of the checker independently. George originally
    implemented it by disabling modeling (which automatically guarantees
    that reports aren't emitted), but there should be no harm in
    rewriting it to silence the reports instead.<br>
    <br>
    <br>
    <div class="moz-cite-prefix">On 8/27/19 8:19 PM, Kristóf Umann
      wrote:<br>
    </div>
    <blockquote type="cite"
cite="mid:CAGcXOD7ZePJWajeh4SS6YVSDi0aSmcoakr8RwuFTmw-vgCJFoQ@mail.gmail.com">
      <meta http-equiv="content-type" content="text/html; charset=UTF-8">
      <div dir="ltr"><br>
        <div class="gmail_quote">
          <div dir="ltr" class="gmail_attr">On Wed, 28 Aug 2019 at
            05:12, Kristóf Umann <<a
              href="mailto:dkszelethus@gmail.com" moz-do-not-send="true">dkszelethus@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">Hi!<br>
              <br>
              This family of checkers is not under my authority, nor am
              I that knowledgeable about them, but even as a non-user, I
              find its interface confusing. While this doesn't affect me
              much, it is also the greatest sinner of how
              modeling/diagnostic checkers should be structured
              according to our previous discussions.
              <div><br>
              </div>
              <div>I'd be happy to fix it (in fact, I'd prefer to, just
                to gain a better understanding of how the new checker
                system should look like), but I obviously can't make
                decisions on how it should look like -- could you help
                me please?<br>
                <br>
                Here is the problem:</div>
              <div>* OSObjectRetainCountChecker and RetainCountChecker
                are subcheckers of RetainCountBase, yet they seem to
                fine-tune how the modeling should be done, rather then
                what diagnostics should be emitted. Shouldn't we turn
                them into checker options of RetainCountBase instead?</div>
              <div><br>
              </div>
              <div>* OSObjectRetainCountChecker and the checker option
                osx.cocoa.RetainCount:CheckOSObject have a super weird
                interaction -- they are supposed to do the same thing
                (optionally enable some modeling RetainCount does), and
                exist purely for backward compatibility reasons, but in
                a way that I personally find impossible to understand.</div>
              <div><br>
              </div>
              <div>The option was added by George, and was tied to
                osx.cocoa.RetainCount rather then
                osx.OSObjectRetainCountChecker, yet the option is unused
                unless osx.OSObjectRetainCountChecker itself is enabled,
                making it the only checker option ever to have 3
                stances: enabled, disabled, and unspecified, the only
                remaining option not retrievable with
                debug.ConfigDumper, and is also the single reason why we
                can't make AnalyzerOptions' config table private.</div>
              <div><br>
              </div>
              <div><a
href="https://reviews.llvm.org/rGd1081ec5082ba6ba26809c66e410b127ca5819a8"
                  target="_blank" moz-do-not-send="true">https://reviews.llvm.org/rGd1081ec5082ba6ba26809c66e410b127ca5819a8</a></div>
              <div><br>
              </div>
              <div>Would it be possible to just simply make this a
                *regular* option that belongs to osx.RetainCountBase?
                Are there any users relying on this behavior?</div>
            </div>
          </blockquote>
          <div><br>
          </div>
          <div>Another possible solutions:</div>
          <div>* Supply the shouldRegister* functions with more data
            (AnalyzerOptions in particular), don't even enable the
            checker if the option is false.</div>
          <div>* When we're parsing the checker options in
            CompilerInvocation.cpp, manually turn this into an
            -analyzer-disable-checker.</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>* RefCountBug::RefCountBugType is an enum for various
                types of retain count related errors. Shouldn't we turn
                these into subcheckers?</div>
              <div><br>
              </div>
              <div>Mind that we can always do this in a
                backwards-compatible manner, and aside from the
                Schrödinger option, we can also preserve the original
                behavior.</div>
              <div><br>
              </div>
              <div>Cheers!<br>
              </div>
              <div>Husi</div>
            </div>
          </blockquote>
        </div>
      </div>
    </blockquote>
    <br>
  </body>
</html>