[webkit-dev] Tagged assertions...

2014-11-06 Thread Said Abou-Hallawa
Recently I ran into an issue where I wanted to find if there is a bug filed against a certain assertion. The only way I was able to use is to search for the name of the function which has the assertion. The function can be overloaded or the function can have more than one assertion. So I

Re: [webkit-dev] Tagged assertions...

2014-11-06 Thread Geoffrey Garen
In debug builds, in addition to a backtrace, don’t assertions already provide a text description and a line number? Perhaps we need a more searchable way to record these fields in our bug tracking databases. But it’s not immediately clear to me why adding a third field would help. Geoff On

Re: [webkit-dev] Tagged assertions...

2014-11-06 Thread Said Abou-Hallawa
The function name and the line number may change if the code changes. A unique tag associated with the assertion should stay the same. So when you want to search for the assertion, you just need the tag since it is unique. We can even extend the dump to do the search in Bugzilla as well.

Re: [webkit-dev] Tagged assertions...

2014-11-06 Thread Geoffrey Garen
The function name and the line number may change if the code changes. How often do these kinds of changes cause problems? When function names and line numbers do change, doesn’t the message still identify the assertion? Geoff ___ webkit-dev mailing

Re: [webkit-dev] Tagged assertions...

2014-11-06 Thread Said Abou-Hallawa
These assertions can’t be uniquely identified if they are repeated in the same function ASSERT(false); ASSERT_NOT_REACHED(); And more importantly the same function can have the same assertion repeated in multiple places void function() { int result = 0; result +=

Re: [webkit-dev] Tagged assertions...

2014-11-06 Thread Said Abou-Hallawa
When the logical expression in the ASSERT is changed, you should do the following: - ASSERT_TAG(condition, “abcde”); + ASSERT(new_condition); Then the pre-build tagging tool will generate a new tag for the new assertion: + ASSERT(new_condition, “fghij”); So the assertions do not get

Re: [webkit-dev] Tagged assertions...

2014-11-06 Thread Benjamin Poulain
I don't think the problem is big enough to justify ASSERT_TAG. Why not just add FIXME with some explanations and a link to the bug number? Benjamin On 11/6/14 12:34 PM, Said Abou-Hallawa wrote: When the logical expression in the ASSERT is changed, you should do the following: -

Re: [webkit-dev] Tagged assertions...

2014-11-06 Thread Michael Saboff
This will add a step that developers will have to remember when changing an ASSERT. There may be developers that want to knowingly keep the same tag. What about creating my own tag values? How will we guard against reusing a tag? So what would be an appropriate tag for an ASSERT in a

Re: [webkit-dev] Tagged assertions...

2014-11-06 Thread Said Abou-Hallawa
Are you saying when an assertion fires I have to do the following? 1. Search Bugzilla for the assertion and if it is no found, file a new one 2. Submit a change just for adding a FIXME comment just above the assertion with new bug filed Would not be easier to just have the tag work both ways?