Dear all,
We have already renamed most of the existing namespaces to snake case. Before
moving on to the last ones, which may generate more conflicts, I have
encapsulated most of gem5 in a gem5 namespace. This will ensure that these last
- and possibly most likely to generate conflicts - renames generate less
issues. The respective chain can be found in
https://gem5-review.googlesource.com/c/public/gem5/+/46323/4 .
The approach taken to implement the gem5 namespace was not ideal, since it is a
huge dump on the whole project instead of a per-dir approach; however, it was a
trade-off between number of rebasing conflicts that I would have to deal with,
and the time I had available for this project. To make it at least a bit
reviewable, I have split it into sub-patches with common approaches. These
patches will be squashed as soon as they are approved.
Finally, every time I do a git push, many conflicts pop up. This requires a lot
of effort to support, and processing power to recompile everything and make
sure everything still works (I don't currently have the latter). Also, the next
version is on the corner, and it would be great to deliver the gem5 namespace
with it. Therefore, I would like to get these changes in ASAP. When problems
show up - and they will - they can be more easily fixed, since this change is
trivial - one will most likely just need to add a "namespace gem5 {" to a
specific file that is compiled in a specific simulation configuration.
tl;dr: Could you review the namespace patches so that we can try to get them in
for the next version?
Regards,Daniel
Em sexta-feira, 7 de maio de 2021 02:30:13 BRT, Gabe Black via gem5-dev
<[email protected]> escreveu:
Be warned though, that there are some pitfalls with this namespace deprecation
approach. The namespaces here are not actually equivalent, and so the old
deprecated namespace can have things added to it that won't show up in the new
one. This is probably not that big a deal in practice, and should be pretty
useful letting people know what's going on, but it's still important to be
aware of limitations.
Gabe
On Thu, May 6, 2021 at 7:36 AM Jason Lowe-Power via gem5-dev
<[email protected]> wrote:
Thanks for putting this all together, Daniel!
IMO, we should do our best with providing deprecation notices, but not bend
over backwards. For things that are easy to add deprecations to (e.g., function
names / class names) we should do it, and for things that have a big impact on
our users we should provide the warnings. However, if it's very difficult to
provide the notice *and* if it's for something that is unlikely to affect
users, then the deprecation warnings are less important.
Example: if we change `panic` to `gem5_panic` (or `GEM5_PANIC`?) we definitely
need a deprecation warning. This will significantly impact users. If, on the
other hand, we change a macro that is used in exactly one place, it's probably
less important
Thanks for coming up with a way to do namespaces! This will be useful.
Cheers,Jason
On Thu, May 6, 2021 at 7:06 AM Daniel Carvalho via gem5-dev <[email protected]>
wrote:
Glad to see that we are reaching a consensus! Then we will create the "gem5"
namespace, rename (most) macros to use a "GEM5_" prefix, and will rename all
namespaces to snake case.
I agree that we should do the renaming on a case-by-case basis. I've created a
new Jira Epic to cover converting all namespaces to snake case:
https://gem5.atlassian.net/jira/software/c/projects/GEM5/issues/GEM5-974 .
Regarding deprecating namespaces, aliases cannot be assigned attributes (thus
they cannot be marked as deprecated), but I believe this will do the trick:
#define GEM5_DEPRECATE_NAMESPACE(old_namespace, new_namespace) \
namespace [[gnu::deprecated("Please use the new namespace: '" \
#new_namespace "'")]] old_namespace { \
using namespace new_namespace; \
}
Example:
// Suppose we want to rename a namespace from Test to test
namespace test { int var;
} // This can be added to the base file (i.e., the one we know everybody
will include)
GEM5_DEPRECATE_NAMESPACE(Test, test)
...
// In code, somewhere: test::var = 2; // Does not show deprecation
warning
Test::var = 2; // Shows deprecation warning
Cheers,
Daniel
Em quarta-feira, 5 de maio de 2021 23:28:31 BRT, Gabe Black via gem5-dev
<[email protected]> escreveu:
Yeah, we don't have a ton of namespaces already, but having two copies of all
of them for a while might be messy. I also don't think you can really mark a
namespace as deprecated without even more macro trickery.
Using snake case for the namespaces would be a change to acclimate to and I'm
not *excited* to make a big change like that, especially to something I'm so
used to, but importantly it would maintain consistency which is arguably more
important. It would bring us in line with namespaces like "std" too, which,
given how common it is, wouldn't be the worst thing.
We would have to be careful that short, simple namespace names like "loader"
don't conflict with existing local variable names. Given the potential for
problems there and that we only have a handful of namespaces currently, it
might make sense to convert each namespace one by one, which might make it
easier to deal with fallout.
Gabe
On Wed, May 5, 2021 at 6:45 AM Giacomo Travaglini via gem5-dev
<[email protected]> wrote:
Hi all,
I agree with Daniel's analysis and solution, as enforcing snake_case for
namespaces would probably make everyone happy.
We could in theory adopt namespace aliases for backward compatibility, to
transition smoothly from one "convention" (PascalCase for namespaces is not
mentioned in our coding style) to the other, but I think it will complicate
things even further.
Kind Regards
Giacomo
> -----Original Message-----
> From: Jason Lowe-Power via gem5-dev <[email protected]>
> Sent: 03 May 2021 21:27
> To: gem5 Developer List <[email protected]>
> Cc: Jason Lowe-Power <[email protected]>
> Subject: [gem5-dev] Re: gem5 namespace
>
> Hey Daniel,
>
> Sorry, I didn't mean to add to the confusion :). I may have gotten my case
> names confused! Also, I really appreciate the thoughtfulness and effort
> you're putting into this conversation! I believe I agree with your email
> below.
>
>
> I think that most people don't care that much (which is why we haven't heard
> from many). From my experience, our users only care when they get merge
> conflicts :D. That said, I'm not sure if "straightforward" is a way most of
> our
> users ever feel about merge conflicts. IMO, stability and ease of update
> should be high priority. If we are going to be changing names, etc. in a way
> that means external users of gem5 will have compiler errors, we should
> probably provide backwards compatibility and warnings. Most of our users
> are not software engineers and do not have as much experience with git,
> compilers, etc. as we do.
>
>
> Cheers,
> Jason
>
>
> On Mon, May 3, 2021 at 12:40 PM Daniel Carvalho via gem5-dev <gem5-
> [email protected] <mailto:[email protected]> > wrote:
>
>
> I'm confused, Jason. I thought you were in favor of adopting snake
> case as a "general convention" (i.e., "the google way"), so by adopting snake
> case we would be adopting the "general convention", not forging our own
> path. However, if by "general convention" you mean "the convention vastly
> adopted by gem5", I don't have the exact numbers, but I'd guess it is 70%
> pascal, 30% snake case - i.e., IMHO, not a "general convention", just a
> preference. Adding "gem5" or "Gem5" would only extend this mess, not add
> an exception.
>
> Regarding changing the name of the variable, indeed, it is not a
> necessary step.
>
> Again, as stated before, I don't care which solution is taken. The
> important thing is to come to a decision that appeases most community
> members; however, it is hard to reach a consensus when we seem to have
> strong opinions on both sides, yet we only have 5 votes. That is why I have
> suggested officially adopting snake case namespaces: it would solve the
> "gem5 VS Gem5" problem, it is feasible to do it without having exceptions (at
> least as of now), Giacomo expressed preference towards lowercase
> namespaces, Jason suggested affinity with the google approach (snake case)
> and "gem5", Bobby prefers "gem5", and Gabe and I like consistency/patterns.
> The (huge) downside to this solution is that it would affect users, but 1)
> we'd
> already be affecting users anyway with the new namespace, and 2) solving
> the merge conflicts would be straightforward if the patches were created
> with that in mind.
>
>
> Regards,
> Daniel
>
> Em segunda-feira, 3 de maio de 2021 15:44:46 BRT, Jason Lowe-
> Power <[email protected] <mailto:[email protected]> >
> escreveu:
>
>
> Just a few quick replies below. Overall, I think we're being too
> focused on "standards" and not on enough on ease of use. It's not a problem
> if there are exceptions to guidance, if there's good reasons for the
> exceptions.
>
>
> On Mon, May 3, 2021 at 11:36 AM Daniel Carvalho via gem5-dev
> <[email protected] <mailto:[email protected]> > wrote:
>
>
> As mentioned by Gabe in the Jira issue, some of the
> namespaces using snake declared in the codebase case are defined as parts
> of a standard, and thus cannot be modified. Realistically, this means that if
> we wanted to follow a namespace naming convention, it'd be snake case:
> despite having way more occurrences of pascal case namespaces, they are all
> "ours"; this means that, although inconvenient, it would be doable to
> standardize all of them.
>
>
>
> If we just say "all namespaces should be PascalCase. gem5, the name
> of this project, is an exception" would be OK with me.
>
>
>
> Would you be willing to adopt to enforce a snake case
> namespace naming convention? From the previous replies to this thread, it
> seems that this solution would appease most - if not all - participants.
>
>
> I really don't think this is too important. I would definitely prefer to
> follow the *general* conventions rather than forging our own path.
>
>
>
>
> For this change to take place, we need to (not necessarily
> sorted):
> 1) Rename the python variables named "gem5" in
> src/systemc/tlm_bridge/TlmBridge.py (or remove them - I don't remember
> finding where they are used);
>
>
> Why does this need to be done? These are class names, not
> namespaces.
>
>
>
> 2) Add a section to our coding style mandating snake case for
> namespaces;
>
>
> 3) Change all existing namespaces to snake case;
>
>
> 4) Create a verifier to enforce this convention.
>
>
> 5) Rename/Move elements starting with gem5X/m5X as
> gem5::X
>
>
>
> Finally, regarding macro names, we could improve the
> corresponding Jira issue by listing all current macros so that we can start
> applying the change and raise objections to particular instances.
>
>
> Cheers,
> Daniel
>
>
> _______________________________________________
> gem5-dev mailing list -- [email protected] <mailto:gem5-
> [email protected]>
> To unsubscribe send an email to [email protected]
> <mailto:[email protected]>
> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
>
> _______________________________________________
> gem5-dev mailing list -- [email protected] <mailto:gem5-
> [email protected]>
> To unsubscribe send an email to [email protected]
> <mailto:[email protected]>
> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
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 -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s _______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
