Hi Brandon, No objection on the header include guards.
When it comes to private members, I would suggest to just stick with nameOfVar. We have only used _name when there is a constructor parameter name, and it is actually not necessary. It is fine to have the private member be just name, and let the constructor parameter name initialise it with name(name). What macros are causing problems? Adding 5 characters to each and everyone seems rather unfortunate. Andreas On 10/10/2016, 18:52, "gem5-dev on behalf of Potter, Brandon" <gem5-dev-boun...@gem5.org on behalf of brandon.pot...@amd.com> wrote: >Hello all, > >I just posted a patch, http://reviews.gem5.org/r/3650/, which deals with >a name collision between a macro that's defined in our source and a >function declaration included from a standard header. Seems like a rare >collision that I stumbled into, but we should do a better job of naming >our macros given that collisions are a possibility. With macros, we can't >use namespaces to prevent this from happening, but we could make a rule >where we append something like "_GEM5" onto all of the macros. (It is a >much larger problem than function names which is what the issue is on >RB.) Does anyone have thoughts on this suggestion? If it's a good >suggestion, does anyone want to try their hand at sed magic? I don't >expect this is solvable with a 1-liner, but I don't want to spend time >trying to rename all of our macros. (There are ~300 of them in the source >right now that are function macros; there are a lot of defines.) > >In a related vein, our macro naming should never include __ (double >underscore) anywhere in the macro nor should the macro begin with an >_[A-Z] (underscore followed by uppercase letter). It turns out that our >headers all use __X_Y_HH__ format for our include guards and I've found a >couple of example with variables that use the _[A-Z] format. (It's not >just macros; the rule also applies to any identifier.) We need to fix >these issues. I do not know that it will create a problem given that >we're using the 'HH' in the macro, but it's bad form to continue on with >this. The standard dictates that these forms are reserved for the >implementation of the language so we shouldn't be using them. > >I recommend the following (although I am not volunteering to do it all): > >* Change header guards to have the form: NAME_OF_FILE_HH_ > >* Use m_ (like Ruby) for private members in classes: m_nameOfVar >as opposed to _nameOfVar; (consider something like _PID where the >capitalization seems natural) > >* Append a namespace substitute like "_GEM5" onto the end of >macros (or alternatively use GEM5_ at the beginning) > >Regards, >Brandon >_______________________________________________ >gem5-dev mailing list >gem5-dev@gem5.org >http://m5sim.org/mailman/listinfo/gem5-dev IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. _______________________________________________ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
