Thx very much for the patch of Yongchong. Seems that eq_const_st is correspond with the sequence of field in ST. Is such comment need? "Which part of fields are not cared by eq_const_st of two ST?"
-----Original Message----- From: Wu Yongchong [mailto:wuyongch...@gmail.com] Sent: Monday, October 31, 2011 10:47 AM To: Sun Chan Cc: open64-devel Subject: Re: [Open64-devel] [open64-devel] Code review request for bug 883 [IPA] Hi, what about this, I change the position of the comment to top of the class member definition Index: osprey/common/com/symtab_defs.h =================================================================== --- osprey/common/com/symtab_defs.h (revision 3775) +++ osprey/common/com/symtab_defs.h (working copy) @@ -223,6 +223,8 @@ class ST { public: + // after add new member, Make sure to update function eq_const_st::operator() + // in file ipc_symtab_merge.cxx union { STR_IDX name_idx; // index to the name string TCON_IDX tcon; // constant value Index: osprey/ipa/common/ipc_symtab_merge.cxx =================================================================== --- osprey/ipa/common/ipc_symtab_merge.cxx (revision 3775) +++ osprey/ipa/common/ipc_symtab_merge.cxx (working copy) @@ -159,7 +159,7 @@ struct eq_const_st { bool operator() (const ST* st1, const ST* st2) const { - return memcmp (st1, st2, sizeof(ST) - sizeof(ST_IDX) * 2 - sizeof(TY_IDX)) == 0; + return memcmp (st1, st2, sizeof(ST) - sizeof(ST_IDX) * 2 - sizeof(TY_IDX) - sizeof(mUINT32)) == 0; } }; On Mon, Oct 31, 2011 at 7:09 AM, Sun Chan <sun.c...@gmail.com> wrote: > your comment should be at where future new member may be added. > sun > > On Sun, Oct 30, 2011 at 11:01 PM, Wu Yongchong <wuyongch...@gmail.com> > wrote: >> >> Hi, all >> Can a gate keeper help review this patch. >> See https://bugs.open64.net/show_bug.cgi?id=883 for reference. >> Revision 3760 add a new member "mINT32 line" to structure ST, it >> cause the function eq_const_st::operator() get wrong return value, >> then make the symbol merge process in IPA not work. following is the >> patch. I add a common to make sure anyone who add new member to >> structure ST also update function eq_const_st::operator() >> >> Index: osprey/common/com/symtab_defs.h >> =================================================================== >> --- osprey/common/com/symtab_defs.h (revision 3776) >> +++ osprey/common/com/symtab_defs.h (working copy) >> @@ -261,6 +261,8 @@ >> >> mUINT32 line; // The line num where define the sym in >> the source file. >> >> + // Make sure to update function eq_const_st::operator() in file >> ipc_symtab_merge.cxx >> + >> // operations >> >> ST () {Fail_FmtAssertion("ST default constructor must not be >> called.");} >> Index: osprey/ipa/common/ipc_symtab_merge.cxx >> =================================================================== >> --- osprey/ipa/common/ipc_symtab_merge.cxx (revision 3776) >> +++ osprey/ipa/common/ipc_symtab_merge.cxx (working copy) >> @@ -159,7 +159,7 @@ >> struct eq_const_st >> { >> bool operator() (const ST* st1, const ST* st2) const { >> - return memcmp (st1, st2, sizeof(ST) - sizeof(ST_IDX) * 2 - >> sizeof(TY_IDX)) == 0; >> + return memcmp (st1, st2, sizeof(ST) - sizeof(ST_IDX) * 2 - >> sizeof(TY_IDX) - sizeof(mUINT32)) == 0; >> } >> }; >> >> -- >> yongchong >> >> >> ---------------------------------------------------------------------------- -- >> Get your Android app more play: Bring it to the BlackBerry PlayBook >> in minutes. BlackBerry App World™ now supports Android™ Apps >> for the BlackBerry® PlayBook™. Discover just how easy and simple >> it is! http://p.sf.net/sfu/android-dev2dev >> _______________________________________________ >> Open64-devel mailing list >> Open64-devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/open64-devel > > -- yongchong ---------------------------------------------------------------------------- -- Get your Android app more play: Bring it to the BlackBerry PlayBook in minutes. BlackBerry App World™ now supports Android™ Apps for the BlackBerry® PlayBook™. Discover just how easy and simple it is! http://p.sf.net/sfu/android-dev2dev _______________________________________________ Open64-devel mailing list Open64-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/open64-devel ------------------------------------------------------------------------------ Get your Android app more play: Bring it to the BlackBerry PlayBook in minutes. BlackBerry App World™ now supports Android™ Apps for the BlackBerry® PlayBook™. Discover just how easy and simple it is! http://p.sf.net/sfu/android-dev2dev _______________________________________________ Open64-devel mailing list Open64-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/open64-devel
