> 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_*.
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? 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. 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. - Brandon ----------------------------------------------------------- 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
