> On April 5, 2016, 2:59 p.m., Jason Lowe-Power wrote: > > src/mem/ruby/filters/MultiBitSelBloomFilter.hh, line 84 > > <http://reviews.gem5.org/r/3420/diff/1/?file=54280#file54280line84> > > > > Why this change? isParallel is what the style guide says, not m_*. > > Brandon Potter wrote: > The rest of the class uses the m_* notation so it's an issue of ruby > conventions clashing with gem5 conventions. Moreover, slicc does the > conversion where it adds the m_ automatically when it generates the > controllers and other classes so it's a substantial change to deal with the > problem properly. > > __gem5 style guide:__ "Class member names (method and variables, > including const variables) are mixed case, start with a lowercase letter, and > do not contain underscores (e.g., aMemberVariable). Class members that have > accessor methods should have a leading underscore to indicate that the user > should be using an accessor. The accessor functions themselves should have > the same name as the variable without the leading underscore." > > When I run into conflicts like this, I usually defer to whatever the file > contains so the file is at least consistent. > > Do you mind if I drop the issue?
Not at all. On April 5, 2016, 2:59 p.m., Brandon Potter wrote: > > Couple of small notes, but other than that it's fine. > > > > Any idea why it was originally like this? Seems kinda crazy. > > Brandon Potter wrote: > I do not know anything about this code or how it was suppossed to be > used. I inherited this alteration from Nilay in a larger patch and factored > it out here for posting. > > From what I can tell, nothing includes the header for this filter class. > The src directory only shows that the cc file needs it. After running the > util/regress script, I search for the include with > __find ./build -type f | xargs grep -I "#include > \".*MultiBitSelBloomFilter.hh\""__; I still come up with nothing. > > The change was made in the context of these other patches because code > around this alteration gets touched. Yeah, the change seems fine. I was just curious. - Jason ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3420/#review8140 ----------------------------------------------------------- On April 4, 2016, 11:39 p.m., Brandon Potter wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3420/ > ----------------------------------------------------------- > > (Updated April 4, 2016, 11:39 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11422:6fc675f956dc > --------------------------- > ruby: change MultiBitSelBlockFilter constructor signature > > The previous format required a string which was then tokenized to retrieve > the constructor arguments. The string is removed and the arguments are > passed directly in their correct format. > > > Diffs > ----- > > src/mem/ruby/filters/MultiBitSelBloomFilter.hh > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/filters/MultiBitSelBloomFilter.cc > cfad34a15729e1d5e096245f5a80ded6e2c379ca > > Diff: http://reviews.gem5.org/r/3420/diff/ > > > Testing > ------- > > > Thanks, > > Brandon Potter > > _______________________________________________ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
