Do we actually plan on running this feature/test someday? Or could we just delete it? If we keep it, we need to port at least: *http://hg.mozilla.org/mozilla-central/rev/54cf0cbe42e8 *http://hg.mozilla.org/mozilla-central/rev/cc49a7b96493 *Bug 512726. *Bug 500870.

(In reply to comment #0) > Do we actually plan on running this feature/test someday? > Or could we just delete it? Not sure, Joshua and Benjamin might know about that stuff much better.

The following are the current analyses from reading mozilla's static-checking: * NS_FINAL_CLASS: make sure no one extends this class * NS_OVERRIDE: @Override for mozilla code * NS_MUST_OVERRIDE: subclasses must override this method * outparams: don't write outparams on error * NS_STACK_CLASS: don't allow this to be made on the heap * flow: All control through a function must flow through a certain label * jsstack: allow only certain functions to access the JS stack * frame-verify: some layout frame requirements Of these, the last one is pretty much pointless for comm-central (it's a layout-specific thing). I suspect jsstack is also mostly pointless. I doubt we'd have a problem with not using NS_STACK_CLASS and NS_FINAL_CLASS; indeed, I think NS_STACK_CLASS was written with xpcomgc in mind. NS_OVERRIDE has to be used to be helpful, and I doubt most committers would care to use it. NS_MUST_OVERRIDE, outparams, and flow are the only ones I see as possibly being helpful. However, I think our implementing hierarchies wouldn't benefit from NS_MUST_OVERRIDE all that much (we don't really have any of these except for what we do = 0 on anyways). Flow is probably useful in a few places (I know I've thought about a few times in some of the more complex parsing code), but I wouldn't miss it too much. I think outparams could find bugs in our code, given our liberal ignorance of certain XPCOM rules (apparently passing in null outparams is technically against the rules...). However, the one case I had where I expected it to complain, it didn't, so I'm not entirely certain about its efficacy. I regularly build on a static-checking build, but it doesn't seem to catch any obvious errors (our build process is noisy enough that I'm not sure I'd notice to begin with). I think asuth mentioned wanting to do so, but besides the two of us, I don't think anyone has even played around with a dehydra-enabled build. All of this is, I guess, a roundabout way of saying "While it would be a shame to lose some potentially useful tools, probably no one would miss it." It is, after all, a decision that could be reevaluated should m-c see a massive revitalization of more widely-applied static checking.

That thoughtful analysis aside, keeping the file in c-c doesn't cost you anything, and even syncing up to m-c's copy every so often should be pretty painless, no? (You can just copy it wholesale, can't you?)

(In reply to comment #3) > keeping the file in c-c doesn't cost you anything, Sure, it costs little. But we have started work to clean up useless/unused "config" things from c-c: see bug 513709 for example. (Not knowing the details of this feature, I even wonder why these files have to be duplicated in c-c and couldn't be used directly from m-c.?.) > and even syncing up to m-c's copy every so often should be pretty > painless, no? (You can just copy it wholesale, can't you?) Not painless really: see http://dev.seamonkey.at/?d=x&i=mozilla&m=c for example. It takes a long time to sort out, patch, review, land ... repeat each time. "wholesale copy" is just what lead us to this situation in c-c :-| So, while I would fully support setting up a "tinderbox" using this feature (if there is an interest in that), I would rather remove it otherwise.

Serge, Ted is right when it comes to that file itself that we can just copy every m-c change for it, so if there are people who try to run this stuff and might get something out of it, I'd prefer to have that done. The changes in other files going along with those config/static-checking-config.mk changes are hopefully not too painful to take along and port as well.

Agree with c#5, adjust summary to reflect current consensus

(In reply to comment #4) > (Not knowing the details of this feature, I even wonder why these files have to > be duplicated in c-c and couldn't be used directly from m-c.?.) Fwiw, could someone point me to an answer to this?

From all I know, static analysis performs several analysis tasks on the code to find bug or deprecated constructs, etc. Joshua has given much better explanations here already. The reason why we need to copy the file is because it has a few references that we need to point to Mozilla directories with our special variables for that.

As a side note, this blog post just came around and explained static checking: http://blog.mozilla.com/tglek/2010/01/21/state-of-static-analysis-at-mozilla/

This ports: * Changeset: http://hg.mozilla.org/mozilla-central/rev/54cf0cbe42e8 * Changeset: http://hg.mozilla.org/mozilla-central/rev/cc49a7b96493 * Bug 512726 * Bug 500870

Comment on attachment 430798 [details] [diff] [review] fully sync it [checkin c#11] http://hg.mozilla.org/comm-central/pushloghtml?fromchange=386d5b686699&tochange=a803df412b50