Flag functions that should be static if they are only called within a particular compilation unit

ASSIGNED
Assigned to

Status

()

Core
Rewriting and Analysis
ASSIGNED
7 years ago
7 years ago

People

(Reporter: (dormant account), Assigned: Ehren Metcalfe)

Tracking

(Blocks: 1 bug)

Trunk
x86
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

7 years ago
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.
(Assignee)

Updated

7 years ago
Assignee: nobody → ehren.m
Status: NEW → ASSIGNED
(Reporter)

Updated

7 years ago
Depends on: 537857
(Assignee)

Comment 1

7 years ago
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
(Reporter)

Comment 2

7 years ago
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.
Yeah, this is the kind of thing it would be really good to have during review.
cairo decorates all of it's public symbols with 'cairo_public' so it should be possible to avoid the false positives there.
(Assignee)

Comment 5

7 years ago
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.
Attachment #420090 - Flags: review?
(Assignee)

Updated

7 years ago
Attachment #420090 - Flags: review? → review?(dwitte)
(Reporter)

Updated

7 years ago
Depends on: 538014

Comment 6

7 years ago
Comment on attachment 420090 [details] [diff] [review]
callgraph patch

Looks good!
Attachment #420090 - Flags: review?(dwitte) → review+

Comment 7

7 years ago
It seems there is a patch for this. Is it current, and ready to go?
You need to log in before you can comment on or make changes to this bug.