Closed
Bug 436342
Opened 17 years ago
Closed 6 years ago
Treehydra: Look for bad sql in mozilla
Categories
(Developer Infrastructure :: Source Code Analysis, defect)
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: taras.mozilla, Unassigned)
References
Details
Attachments
(2 files, 5 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
taras.mozilla
:
review+
|
Details | Diff | Splinter Review |
Not sure what qualifies as inappropriate sql, but I have a script that can dump out sql statements on linux http://hg.mozilla.org/users/tglek_mozilla.com/index.cgi/dehydra-mq/file/e935d5f002ae/sql.diff Would like to get that working on mac. i think that means modifying gcc_compat a bit
Updated•17 years ago
|
Assignee: nobody → vladimir.sukhoy
Updated•17 years ago
|
Status: NEW → ASSIGNED
Comment 1•17 years ago
|
||
I quickly (and dirtily) went over tree processing problems I spotted in current test suite. This patch makes the test suite (locks) pass on mac, the problems seen are for the most part known or simple to track ast differences. Eventually the outcome of this bug should make outparams and other esp-based stuff more feasible on mac as well as this sql thing. I also suspect that MODIFY_EXPR is mac's GIMPLE_MODIFY_STMT but this has to be looked into in more detail.
Reporter | ||
Comment 2•17 years ago
|
||
Thanks for looking into this. That's bad, I underestimated the difficulty of this. I didn't realize there would be so many differences. I wonder if there is even a sane way to do cross-version scripting. Thanks for looking into it.
Comment 3•17 years ago
|
||
Attachment #323030 -
Attachment is obsolete: true
Attachment #323186 -
Flags: review?(tglek)
Reporter | ||
Updated•17 years ago
|
Summary: Look for bad sql in mozilla → Treehydra: Look for bad sql in mozilla
Reporter | ||
Comment 5•17 years ago
|
||
Reporter | ||
Comment 6•17 years ago
|
||
Comment on attachment 323186 [details] [diff] [review] libs compat patch > function CALL_EXPR_FN (node) { >- return TREE_OPERAND (TREE_CHECK (node, CALL_EXPR), 1) >+ return TREE_OPERAND (TREE_CHECK (node, CALL_EXPR), isGCCApple ? 0 : 1) > } Above should be a constant defined outside of the function. No need to calculate static stuff every call. >+ case (isGCCApple ? MODIFY_EXPR : GIMPLE_MODIFY_STMT): Cute use of js syntax here. However most of these changes seem mechanical. I think in this case it's safe to just do if(isGCCApple) { this.GIMPLE_MODIFY_STMT = MODIFY_EXPR ... the other renames like POINTER_PLUS_EXPR go here } This gives linux scripts a higher chance of working on osx out of the box. It's a bit of hack, but I think it's our best bet. I think the rest of the patch is excellent as usual. Sorry it took so long to review.
Attachment #323186 -
Flags: review?(tglek) → review-
Comment 7•17 years ago
|
||
Early work on doing what we already dynamically check at runtime in debug builds and issue an NS_WARNING for. Completely untested at the moment.
Assignee: vladimir.sukhoy → sdwilsh
Attachment #323187 -
Attachment is obsolete: true
Attachment #324063 -
Attachment is obsolete: true
Comment 8•17 years ago
|
||
Fixed both issues with the libs compat patch as requested.
Attachment #323186 -
Attachment is obsolete: true
Attachment #324102 -
Flags: review?(tglek)
Comment 9•17 years ago
|
||
Comment on attachment 324102 [details] [diff] [review] libs compat patch v2 Oops, nevermind.
Attachment #324102 -
Flags: review?(tglek)
Comment 10•17 years ago
|
||
The actual patch.
Attachment #324102 -
Attachment is obsolete: true
Attachment #324104 -
Flags: review?(tglek)
Reporter | ||
Comment 11•17 years ago
|
||
Comment on attachment 324104 [details] [diff] [review] actual libs compat patch v2 >+var call_expr_arg_iterator = isGCCApple ? >+ function (node) { >+ let arg_holder = TREE_OPERAND(node, 1); >+ while (arg_holder) { >+ yield TREE_VALUE(arg_holder); >+ arg_holder = TREE_CHAIN(arg_holder); >+ } Please rewrite this into the for(let arg_holder= ...;arg_holder;arg_holder =...) form and commit.
Attachment #324104 -
Flags: review?(tglek) → review+
Comment 12•17 years ago
|
||
pushed the libs compat patch. case (isGCCApple ...): is no longer necessary in the analysis.
Updated•16 years ago
|
Assignee: sdwilsh → dwitte
Comment 13•14 years ago
|
||
Reassigning to nobody. If anyone wants to work on this, feel free!
Assignee: dwitte → nobody
Status: ASSIGNED → NEW
Updated•7 years ago
|
Product: Core → Firefox Build System
Comment 14•6 years ago
|
||
Let's give up on this. Nothing for 8 years, this isn't probably critical.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
Updated•2 years ago
|
Product: Firefox Build System → Developer Infrastructure
You need to log in
before you can comment on or make changes to this bug.
Description
•