Closed Bug 562152 Opened 10 years ago Closed 10 years ago

Remove typedef LInsp

Categories

(Core Graveyard :: Nanojit, defect, P4)

defect

Tracking

(Not tracked)

RESOLVED FIXED
flash10.2

People

(Reporter: edwsmith, Unassigned)

References

Details

(Whiteboard: fixed-in-nanojit, fixed-in-tracemonkey, fixed-in-tamarin)

Attachments

(1 file, 1 obsolete file)

The inconsistent use of this typedef is bothering us, and it also appears to confuse Eclipse CDT indexing when not used consistently between a definition and declaration of a method.  

Time to go!
Blocks: 537926
Target Milestone: --- → Future
Assignee: nobody → edwsmith
in TR, CDT find references says:

CodegenLIR.cpp (56 matches)
Assembler.cpp (15 matches)
Assembler.h (31 matches)
LIR.cpp (140 matches)
LIR.h (151 matches)
no big rush on this review, posting now to keep my pipeline full
Attachment #444530 - Flags: review?(wmaddox)
Attachment #444530 - Flags: review?(wmaddox) → review+
Comment on attachment 444530 [details] [diff] [review]
Remove uses of LInsp in CodegenLIR.cpp

TR: http://hg.mozilla.org/tamarin-redux/rev/dd2020e89b86
Attachment #444530 - Attachment is obsolete: true
Priority: -- → P4
Target Milestone: Future → flash10.2
Note: Upon request, I can this as a series of patches over time, to reduce merge collisions.  Just easier to post the whole thing for review.

noteworthy:
* two places in LIR.h required fixing more elaborate than search & replace:   LInsp a, b;   => LIns *a, *b;
* testing on TR sandbox tonight, will offer a TR patch too.
* typedef LIns* LInsp; is still present in LIR.h until all uses are removed.  TR is clean.
Attachment #450180 - Flags: review?(nnethercote)
Comment on attachment 450180 [details] [diff] [review]
Remove use of LInsp in nanojit

>diff -r 0a8efeddb4d9 nanojit/LIR.cpp
>--- a/nanojit/LIR.cpp	Wed Jun 09 09:30:23 2010 +0100
>+++ b/nanojit/LIR.cpp	Wed Jun 09 15:02:15 2010 -0400
>@@ -930,10 +930,10 @@
> 
>     // Simplify operator if possible.  Always return NULL if overflow is possible.
> 
>-    LIns* ExprFilter::simplifyOverflowArith(LOpcode op, LInsp *opnd1, LInsp *opnd2)
>+    LIns* ExprFilter::simplifyOverflowArith(LOpcode op, LIns* *opnd1, LIns* *opnd2)

Ew, gross.  "LIns** opnd1" (my preference) or "LIns **opnd1" please.  

>@@ -2021,12 +2021,12 @@
>     {
>         const uint32_t oldcap = m_cap[kind];
>         m_cap[kind] <<= 1;
>-        LInsp *oldlist = m_list[kind];
>-        m_list[kind] = new (alloc) LInsp[m_cap[kind]];
>-        VMPI_memset(m_list[kind], 0, m_cap[kind] * sizeof(LInsp));
>+        LIns* *oldlist = m_list[kind];

Here too!

>-        LIns* simplifyOverflowArith(LOpcode op, LInsp *opnd1, LInsp *opnd2);
>+        LIns* simplifyOverflowArith(LOpcode op, LIns* *opnd1, LIns* *opnd2);

And here.

And can you grep for other occurrences of this, in case I missed any?


The rest looks good.  Furthermore, LInsp isn't used anywhere in TM (I just
tested) so you can remove it immediately.
Attachment #450180 - Flags: review?(nnethercote) → review+
(In reply to comment #5)
> Ew, gross.  "LIns** opnd1" (my preference) or "LIns **opnd1" please.  
> And can you grep for other occurrences of this, in case I missed any?

gross indeed, i'll fix the ones you found and grep for more.  hazards of search/replace.

> The rest looks good.  Furthermore, LInsp isn't used anywhere in TM (I just
> tested) so you can remove it immediately.

hokay!
NJ: http://hg.mozilla.org/projects/nanojit-central/rev/63a0a36e4637
Assignee: edwsmith → nobody
Whiteboard: fixed-in-nanojit
TM: http://hg.mozilla.org/tracemonkey/rev/47e02e48437a
Whiteboard: fixed-in-nanojit → fixed-in-nanojit, fixed-in-tracemonkey
TR: http://hg.mozilla.org/tamarin-redux/rev/7d2ad7374035
Whiteboard: fixed-in-nanojit, fixed-in-tracemonkey → fixed-in-nanojit, fixed-in-tracemonkey, fixed-in-tamarin
http://hg.mozilla.org/mozilla-central/rev/47e02e48437a
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.