xpcom analysis type-printer.js error with function pointer parameter

ASSIGNED
Assigned to

Status

Firefox Build System
Source Code Analysis
ASSIGNED
9 years ago
3 months ago

People

(Reporter: Roger Dicke, Assigned: Roger Dicke)

Tracking

({student-project})

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

9 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1.5) Gecko/20091102 Firefox/3.5.5 (.NET CLR 3.5.30729)
Build Identifier: mozilla-central

After generating docs for external strings functions with function pointers in the parameters dump the object. (See "PRInt32 Compare(const PRUnichar*, PRInt32 (*)(const PRUnichar*, const PRUnichar*, PRUint32))" in the link provided for example)

Reproducible: Always
(Assignee)

Updated

9 years ago
Depends on: 529978, 530166
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee: nobody → redicke
Keywords: student-project
(Assignee)

Comment 1

9 years ago
Created attachment 417322 [details] [diff] [review]
If the passed in type as yet another type and it's name is not undefined return the type's name

I am not sure if there is a better way to display when a function pointer is a parameter. With this it shows the return type and I have never seen a name other than "c".
Attachment #417322 - Flags: review?(benjamin)

Comment 2

9 years ago
Comment on attachment 417322 [details] [diff] [review]
If the passed in type as yet another type and it's name is not undefined return the type's name

I'd need an example here to verify behavior, but I don't think this can be right. You should either find a typedef name for the function pointer, or print the full function pointer type, e.g. bool (*)(int, int). Printing the return type of the function will be confusing, not helpful.
Attachment #417322 - Flags: review?(benjamin) → review-
(Assignee)

Comment 3

8 years ago
Created attachment 425527 [details] [diff] [review]
Slight modification to getShortname() and typeName() in xpcom/analysis/type-printer.js

See: 
https://developer.mozilla.org/User:EndersTruth/nsAString#Compare%28const%20PRUnichar*%2C%20PRInt32%20%28*%29
for an example of the malbehavior.
Here:
https://developer.mozilla.org/User:EndersTruth/nsAString_(External)_example#Compare
is an example created using this patch.
Attachment #417322 - Attachment is obsolete: true
Attachment #425527 - Flags: review?(benjamin)

Comment 4

8 years ago
Since this has a typedef, it would be really nice to actually use the typedef name (and even link to the typedef declaration!) instead of expanding the function-pointer type, since most people can't read function pointer types effectively. Can you do that?

Comment 5

8 years ago
Specifically, I'd like this to prettyprint to:

PRInt32 Compare(const PRUnichar*, ComparatorFunc) const

Comment 6

8 years ago
Comment on attachment 425527 [details] [diff] [review]
Slight modification to getShortname() and typeName() in xpcom/analysis/type-printer.js

That said, this patch is acceptable since we want it for non-typedefed function pointers.
Attachment #425527 - Flags: review?(benjamin) → review+
(Assignee)

Comment 7

8 years ago
I would like to have this working using the typedef name, if available, as well. However I am not getting any typedef info out of Dehydra. This is because instead of processing the function pointer as a single parameter it goes into an extra type layer and starts defining all of the types in the function it is pointing to. (See my blog post for details on what the type-printer expects and what it receives instead: http://enderstruth.wordpress.com/2010/01/28/reviewing-the-function-pointer-parameter-problem/) This is really what the problem was in the first place, but it was simple to fix higher up. The problem is deeper (I'm guessing??) when Dehydra is extracting the information. I am sure when working only with predefined types it is easy(ier) to pass the data (i.e. to the type-printer), but when any function can be used as a parameter it must complicate things to a great extent. With that, any pointers as to where I should be looking?
Status: NEW → ASSIGNED

Updated

3 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.