bug id harvesting should match signatures with just [@ Foo] instead of requiring [@ Foo(...)]

RESOLVED WONTFIX

Status

--
enhancement
RESOLVED WONTFIX
9 years ago
a year ago

People

(Reporter: timeless, Assigned: lonnen)

Tracking

Trunk
x86
Mac OS X

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

9 years ago
having full arguments in bug summaries reduces the ability to have a bunch of signatures in summaries and is rarely if ever beneficial, please recognize the shorter syntax always.
WSM asked lars about this on IRC yesterday, and lars noted that the reason it's implemented as it is is because the full signature is an indexed column, whereas doing a partial match would require table scanning, which kills the db.

That being said, I think we could implement this by adding another column to the reports table which contained the signature minus the argument list.

Comment 2

9 years ago
(In reply to comment #1)
> That being said, I think we could implement this by adding another column to
> the reports table which contained the signature minus the argument list.

would adding that column also potentially allow search "exactly" to find signatures without the arguments?
Doing this is a vote in favor of (re)building signaturedims table
Just a reminder that functions in C++ are overloaded on parameter type (which is what I think wsmwk means by 'arguments') so "search 'exactly'" without the parameter types might sometimes find overload function sets rather than a specific function.
What would be the purpose of maintaining two "overall signature" columns?  Wouldn't it be better to just change how signatures are created for the existing column?  I don't see the benefit of having one for display and one for associations.

The purpose in C++ for including the parameters in the signature is to disambiguate overloaded functions.  By ignoring parameters, are we introducing ambiguity that will cause problems in the future?

What about template parameters?  There are a number of other 'inconvenient' syntax artifacts in signatures with square brackets, angle brackets, braces, back tics and apostrophes.  Should they be simplified too?

Examples:
  nsTHashtable<nsBaseHashtableET<nsISupportsHashKey, nsCOMPtr<nsIXULTemplateBuilder> > >::s_MatchEntry(PLDHashTab
le*, PLDHashEntryHdr const*, void const*)
  [thunk]:ATL::CComContainedObject<CWMPButtonCtrl>::Release`adjustor{12}' ()

I think this bug is really a call for a reexamination on how signatures are normalized.

Comment 6

9 years ago
Template information is usually important. nsTHashtable...nsIXULTemplateBuilder is going to be a very different bug than the same method with some other interface.

I'm not sure why we can't just use the long signatures... are we actually running up against length limits in the whiteboard/summary field?
When I talked with Lars about this, I didn't see any harm including the entire signature in the summary field. I still don't. Unless someone has a good reason to change (and I haven't seen one yet), I don't think we should add the complication to Socorro.
I think timeless' point is just that historically people have not used the full signatures in bug summaries. I sort of agree that full argument lists don't add much value in bug summaries, and I doubt there would be much confusion with overloaded methods.
(Reporter)

Comment 9

9 years ago
we do run into limits with the summary field, and it makes summaries more painful.

Supporting both syntaxes is ideal, people can certainly file bugs w/ template signatures, in those cases, they'll know if they think they're relevant.

Frank: in practice we rarely have cases where overloads are a problem, and again, if it actually is relevant someone can change their bug summary to have the full signature.

Comment 10

9 years ago
IMO "Supporting both syntaxes is ideal". Question - If parameters are so useful in bug summaries, wouldn't they be more prevalent? 

Summaries of open crash bugs:
* FF has 600+ without parameters, and 100 with
* TB has ~80 without parameters and 20 with
Question: How does a signature get into a bug? If we are doing it by hand, then cut and paste seems obvious and whatever is easily cut should be used (and we should think about how to automate it). If somehow automagically, then the automaton and the database should work together.

(In reply to comment #9 and comment #4): If you know that function overload sets are unlikely to be a problem, I'm perfectly happy to agree with you. But since some connections are automatic now, it is probably better in general to use the full signature to avoid unintended mis-connections: If a problem is rare but real, then people tend to forget how to deal with it, and the cost of (re) discovering the problem each time is high.
It is indeed by hand. I think Wayne's point is that if you look in bugzilla, the vast majority of existing bugs with signatures in the summary do not contain the parameter list.

Comment 13

9 years ago
(In reply to comment #12)
> I think Wayne's point is that if you look in bugzilla,
> the vast majority of existing bugs with signatures in the summary do not
> contain the parameter list.

Correct. That fact leads to these points:
a) bug summaries seem to have been sufficient to date without parameters, so a1) why has it been sufficient? and a2) does it continue to be sufficient?
b) if either choice is not a slam dunk, do we consider the inertia of what people have been accustomed to doing?
c) if there is reason to change, surely we aren't going to change ~700 old bugs, so then we still would want to bridge matching these to old bugs crash-stats
(In reply to comment #9)
> we do run into limits with the summary field, and it makes summaries more
> painful.

> nsObjCExceptionLogAbort | nsNativeThemeCocoa::DrawWidgetBackground(nsIRenderingContext*, nsIFrame*, unsigned char, nsRect const&, nsRect const&)

is more characters than a normal bug summary by itself (145) and is therefore an awful lot to put in a bug summary in order to get Bugzilla integration (150 when you add [@  ]).  

[@ nsObjCExceptionLogAbort | nsNativeThemeCocoa::DrawWidgetBackground] is still a bit long, but far more manageable; you could still manage to fit a "why" before or after and maybe have a chance of seeing it, or a chunk of the signature, in bug lists.
(Reporter)

Updated

9 years ago
Duplicate of this bug: 505598
If we're not going to fix this, then we really need to train developers hard to copy/paste the whole stupid signature from the crash report H1 when filing bugs.  

I maintain it's far easier to teach the software to work the way everyone expects it to work than to retrain developers to work the way the software currently wants developers to work ;-)

(Also inserting "signature" into the summary so this can be found more easily.)
Summary: bug id harvesting should recognize [@ Foo] instead of requiring [@ Foo(...)] → bug id harvesting should match signatures with just [@ Foo] instead of requiring [@ Foo(...)]
(In reply to comment #6)
> I'm not sure why we can't just use the long signatures... are we actually
> running up against length limits in the whiteboard/summary field?

Yes; I just did while trying to put three signatures in the summary of bug 521844; I gave up and put only the two shorter ones.
I also hit summary length limits a couple of times last weekend when I was filing new bugs based on crash-stats reports :(
In bug 541743, I wanted to add the "plain" [@ nsDocShell::Destroy() ] signature to the summary, but I'm out of room.  There's enough characters in the arguments to nsCOMPtr_base::assign_from_qi to allow me to do that, if only we'd match on [@ Foo ] instead of [@ Foo(…) ].
Component: Socorro → General
Product: Webtools → Socorro
(Assignee)

Comment 20

7 years ago
This was fixed by: https://bugzilla.mozilla.org/show_bug.cgi?id=634276
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED

Comment 21

7 years ago
(In reply to Chris Lonnen :lonnen from comment #20)
> This was fixed by: https://bugzilla.mozilla.org/show_bug.cgi?id=634276

Not really. We don't match Foo when Foo(…) is the signature, AFAIK. But not sure if we really need to do that given that the Crash Signature field allows multiple entries more easily.
(Assignee)

Comment 22

7 years ago
My misunderstanding. Reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Lonnen: did rewriting the cron job in Q1 address this? If yes please mark RESOLVED.
Assignee: nobody → chris.lonnen
Flags: needinfo?(chris.lonnen)
(Assignee)

Comment 24

5 years ago
Not at all. Is this still desired?
Flags: needinfo?(chris.lonnen)

Comment 25

5 years ago
I want it because I often paste signatures from local debuggers, and I don't know if they all show parameters the same way as crash-stats. (There's also the problem that different functions might be inlined, of course.)
(Assignee)

Comment 26

a year ago
We are unlikely to pursue this further. If this is still desired, please reopen.
Status: REOPENED → RESOLVED
Last Resolved: 7 years agoa year ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.