accellera-official / cci

SystemC Configuration, Control and Inspection (CCI)
https://systemc.org/overview/systemc-cci/
Apache License 2.0
12 stars 13 forks source link

Reduce/remove namespace & class name redundancy #44

Closed twieman closed 9 years ago

twieman commented 9 years ago

In our 3/16/2015 WG meeting, we agreed to investigate removing repeated namespace information in class names. Consider cci::cnf::cci_cnf_broker_if as an example. After some initial brainstorming we felt an improvement would be:

Laurent subsequently suggested (http://workspace.accellera.org/apps/org/workgroup/cciwg/email/archives/201504/msg00002.html): Since SystemC has no general namespace like "sc" and defines few sub-level namespaces like "sc_core", "sc_dt", "scboost" + names its classes with prefix "sc" , I propose to do the same and define a single namespace "ccicnf::" for everything in current CCI Config source code, then to name the classes with prefix "cci", e.g. "cci_cnf_broker", "cci_param": cci_cnf::cci_cnf_broker cci_cnf::cci_param In that way, header files would be named for instance "cci_cnf_broker.hpp", "cci_param.hpp", what is clear when using them.

It would be very valuable to have someone think this through, propose a scheme, and provide a summary (maybe in Excel) of the resulting namespace, file & class names.

A few relevant notes:

Ballpark effort estimate: 1/2 week.

eswallace commented 9 years ago

Is there a reason we're avoiding an sc_ prefixed name for this new work? Why not sc_cfg, giving us sc_cfg::param?

And I suppose this is a nit, but isn't config almost universally abbreviated as "cfg" and not "cnf"?

erikengineer commented 9 years ago

I wondered about such things myself.

twieman commented 9 years ago

Avoiding the sc prefix is well established precedent at this point; for examples: TLM: tlm SystemC Analog/Mixed-Singal: sca SystemC Verification: scv

We don't have to live with "cnf" though! Enrico asked me this week why I keep referring to "description" when the parameter API uses "documentation. We should definitely fix such things that we can all agree are quirky/clunky.

ccigit commented 9 years ago

Hi,

I also think we shouldn't use prefix "_sc" to avoid ambiguities, like TLM, SCV, SCA standards do ( _tlm__, _scv, _sca__) We should keep prefix _cci__ for CCI standard.

However, I agree to use "cfg" (more universal short name) instead of "cnf".

Regards, Laurent

On 4/4/2015 1:23, twieman (via GitHub) wrote:

Avoiding the sc prefix is well established precedent at this point; for examples: TLM: tlm SystemC Analog/Mixed-Singal: sca SystemC Verification: scv

We don't have to live with "cnf" though! Enrico asked me this week why I keep referring to "description" when the parameter API uses "documentation. We should definitely fix such things that we can all agree are quirky/clunky.

— Reply to this email directly or view it on GitHub https://github.com/OSCI-WG/cci/issues/44#issuecomment-89450212.

twieman commented 9 years ago

As agreed in our April 13th meeting, we will replace the "cnf" abbreviation with "cfg"; this fits well in this issue.

eswallace commented 9 years ago

My 2 cents about the namespace: I would prefer to keep a short and non-redundant namespace name so that users do not feel compelled to import everything into their own namespaces. Something like cci::cfg_param might read pretty well. I don't see a particular need to fragment into three namespaces (cci_cfg, cci_ctl, cci_inst, or what have you), as I suspect the second half of the name will simply be redundant with the class names. As we have total control over the contents of all three namespaces, there should not be any worry about conflicts. And while cci is pretty arbitrary, it does stand a reasonable chance of being unique in a program.

gleary commented 9 years ago

Here is my proposal for the namespaces/directories/top-level includes:

Namespaces: cci

Have a single namespace "cci". This was decided for a couple of reasons: 1) to align with what is done in SystemC (sc_core) and TLM (tlm) currently, 2) the simplicity of a single namespace, and 3) reducing the need for later discussions to decide where new content belongs, where to move content that belongs in more than one component. This would be a drawback to having multiple namespaces; such as cci_core, cci_cfg, etc.

Directories: {cci_root}/src/[cci_core, cci_cfg, cci_ins, cci_ctl, ...]/{files}

For directories I propose having directories for each logical component (cci_core, cci_cfg, ...). This would stay consistent with what has been done in SystemC and TLM. We would then have the top-level includes described below to allow for selective inclusion of CCI components.

Top Level Includes: 1) cci_configuration 2) cci_inspection 3) cci_control

***Note: no trailing .h on top-level includes

Class Names:

This will remain consistent with what has been done in SystemC and TLM, sc and tlm respectively. There may be a need for minor re-factoring in the future to address name clashes if they arise. Hopefully, we will be able to foresee and plan accordingly to avoid most of these occurrences.

Any questions or comments are welcomed.

Thanks, Glenn

lobernard commented 9 years ago

I globally agree with this last proposal.

Few comments:

twieman commented 9 years ago

In May 11 WG meeting, there is agreement with Glenn's proposal along with Laurent's refinement (ccicfg*). Also we want to ensure a different namespace is used for the POC.

pah commented 9 years ago

Sorry for not being able to join today's meeting. I've a question on the consensus:

Glenn's proposal along with Laurent's refinement (ccicfg*).

Are we then talking about cci_cfg_param and cci_cfg_value? Do we expect another cci_*_param in the future?

For cci_value, one of the reasons for going with a single namespace cci has been the difficulty to decide for a "unique" component upfront and cci_value is expected to be useful in the other domains as well...

Thanks for short clarification, Philipp

gleary commented 9 years ago

I have concerns with this (and part of my original proposal as well) after some discussion over the LRM:

Glenn's proposal along with Laurent's refinement (ccicfg*).

I think we are focused on Configuration, Control, and Inspection being very clear and distinct lines and boundaries in the source code and this is not the case - the lines are much more blurred. For instance, where would you put registers/memories? Control? Inspection?

I think the better way to look at it is we are developing the Configuration, Control, and Inspection Specification, which is comprised of the following components: parameters, registers, memories, checkpointing, logging, commands, etc.

With this approach we should not put "cfg", "ctrl", or "insp" on the class names.

Rather we should use the components: "param", "reg", "mem", etc.

We already agreed to using "cci" at the front of each class name, so it is clear everything is part of the CCI standard. The second portion should distinguish which component it is part of, if necessary.

So things like cci_param, would remain the same. As it is clear it is a parameter and part of the "parameters" component. We could end up with a "cci_param_value" if it is determined more than one "value" will exist (or if we decide to do a blanket naming with component).

Another example, we may end up with a "cci_param_broker", "cci_reg_broker", and maybe "cci_mem_broker".

For the high-level include files we may also want to use the component names:

  1. parameters
  2. registers_memories
  3. checkpointing
  4. logging
  5. commands

Just a thought...

ccigit commented 9 years ago

On 13 May 2015, at 01:49, gleary (via GitHub) github@lists.accellera.org wrote:

I have concerns with this (and part of my original proposal as well) after some discussion over the LRM:

Glenn's proposal along with Laurent's refinement (ccicfg*).

I think we are focused on Configuration, Control, and Inspection being very clear and distinct lines and boundaries in the source code and this is not the case - the lines are much more blurred. For instance, where would you put registers/memories? Control? Inspection?

I think the better way to look at it is we are developing the Configuration, Control, and Inspection Specification, which is comprised of the following components: parameters, registers, memories, checkpointing, logging, commands, etc.

With this approach we should not put "cfg", "ctrl", or "insp" on the class names.

Rather we should use the components: "param", "reg", "mem", etc.

I agree with all you have said - only I would add - whats the difference between a reg and a param (and, come to that, a memo) - they are all just parameters of some sort, typedefed over different things (perhaps), but with the same ability to control them, configure them and inspect them…

To me the “Control, Configure and Inspect” is about what we do to the parameters, not different ‘objects’…

We already agreed to using "cci" at the front of each class name, so it is clear everything is part of the CCI standard. The second portion should distinguish which component it is part of, if necessary.

So things like cci_param, would remain the same. As it is clear it is a parameter and part of the "parameters" component. We could end up with a "cci_param_value" if it is determined more than one "value" will exist (or if we decide to do a blanket naming with component).

Another example, we may end up with a "cci_param_broker", "cci_reg_broker", and maybe "cci_mem_broker".

I’m hoping thats a mythical example :-)))

Cheers

Mark.

For the high-level include files we may also want to use the component names:

  1. parameters
  2. registers_memories
  3. checkpointing
  4. logging
  5. commands

Just a thought...

— Reply to this email directly or view it on GitHub https://github.com/OSCI-WG/cci/issues/44#issuecomment-101461846.

This is a GitHub notification mail, automatically forwarded to the Accellera SystemC CCIWG reflector cciwg@lists.accellera.org.

GitHub repository: https://github.com/OSCI-WG/cci

Please send replies to the 'Reply-To' address to include the discussion at GitHub.com and keep the reflector in CC.

 +44 (0)20 7100 3485 x 210

+33 (0)5 33 52 01 77x 210

+33 (0)603762104
mark.burton

applewebdata://860ABF66-ECA2-49C3-9299-7C177DC52DA6/www.greensocs.com

gleary commented 9 years ago

I just want to recap the consensus of the working group in this weeks meeting. In general, we decided to take a keep it simple approach and on a as needed basis we will expand.

Namespace: cci

Directories: {cci_root}/src/[cci_core, cci_cfg, cci_ins, cci_ctl, ...]/{files}

*\ For now, this will include cci_core and cci_cfg

Top-level includes: 1) cci_configuration

*\ More will be added as needed

Class Names: 1) All class names will be prefixed with "cci_"

*\ when/if conflicts arise we will discuss how to handle them then.

Thanks, Glenn

twieman commented 9 years ago

Closed by PR #50 (namespace branch).