Last Comment Bug 536427 - Flag functions that should be static if they are only called within a particular compilation unit
: Flag functions that should be static if they are only called within a particu...
Status: ASSIGNED
:
Product: Core
Classification: Components
Component: Rewriting and Analysis (show other bugs)
: Trunk
: x86 Linux
: -- normal with 2 votes (vote)
: ---
Assigned To: Ehren Metcalfe
:
Mentors:
Depends on: 537857 538014
Blocks: analyses
  Show dependency treegraph
 
Reported: 2009-12-22 11:31 PST by (dormant account)
Modified: 2010-06-02 08:05 PDT (History)
16 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
analysis results (644.90 KB, text/plain)
2010-01-04 19:19 PST, Ehren Metcalfe
no flags Details
callgraph patch (2.38 KB, patch)
2010-01-05 08:59 PST, Ehren Metcalfe
dwitte: review+
Details | Diff | Splinter Review

Description (dormant account) 2009-12-22 11:31:01 PST
Marking functions as static should let the compiler be smarter about inclining and will make GCC produce a warning if a function ever goes unused.
Comment 1 Ehren Metcalfe 2010-01-04 19:19:07 PST
Created attachment 420025 [details]
analysis results

This was generated against revision 89d33cf3490c. The modifications for callgraph to generate the extra columns in 'node' are straightforward but I can post them. Total functions identified: 3987
Comment 2 (dormant account) 2010-01-04 19:58:26 PST
This list looks pretty good. I don't think there is much we can do about the C code.

Looks like we should static-ify content/layout/accessibility. Should probably to whip a basic pork tool to do these en-mass(module granularity). C

A bunch of these are public APIs, if we end up maintaining this list we should deal with that. 

You should definitely integrate your changes into callgraph. I think once we take care of the obvious/easy/to/fix stuff we should flag this info in dxr.

Aside: roc mentioned that these issues are most likely to be fixed by contributors working on patching the affected area and these should somehow be flagged during the patch review cycle.
Comment 3 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2010-01-04 20:46:49 PST
Yeah, this is the kind of thing it would be really good to have during review.
Comment 4 Jeff Muizelaar [:jrmuizel] 2010-01-04 21:05:08 PST
cairo decorates all of it's public symbols with 'cairo_public' so it should be possible to avoid the false positives there.
Comment 5 Ehren Metcalfe 2010-01-05 08:59:33 PST
Created attachment 420090 [details] [diff] [review]
callgraph patch

Patch for callgraph to add 'isStatic' and 'isMethod' to 'node'. Conceivably, we could do without 'isMethod' since separate sqlite views for functions and methods could be created by parsing the 'name' column.
Comment 6 dwitte@gmail.com 2010-01-05 14:55:37 PST
Comment on attachment 420090 [details] [diff] [review]
callgraph patch

Looks good!
Comment 7 Worcester12345 2010-06-02 08:05:03 PDT
It seems there is a patch for this. Is it current, and ready to go?

Note You need to log in before you can comment on or make changes to this bug.