Add db::creation_tag and db::tag_depends_on metafunctions #4782
Conversation
|
Not sure what the best way to handle the build failures is. It seems that in this context |
| * resolves base tags and converts subitems to full items. | ||
| */ | ||
| template <typename Tag, typename Box> | ||
| using real_tag = |
There was a problem hiding this comment.
If I understand what this is doing, in the type aliases in DataBox I have been calling the tags returned by this metafunction creation_tags. If so can we use that nomenclature?
There was a problem hiding this comment.
Yes, those appear to be the same thing. I will adopt that name.
| * themselves, and an item and its subitems all depend on one another. | ||
| */ | ||
| template <typename Provider, typename Consumer, typename Box> | ||
| using tag_depends_on = |
There was a problem hiding this comment.
Not a change request, but I will probably get this backwards when I use/read it without looking up the documentation, as I was expecting the first template argument to be the Consumer tag that depends upon the Provider.
There was a problem hiding this comment.
I copied the order from tt::is_a<Template, Type>, but std::is_base_of<Base, Derived> is the opposite, so yeah, not obvious which is better.
There was a problem hiding this comment.
The choice on is_a is the same as is_base_of in my opinion. It is <generic, specific>. That is, the generic template in is_a plays a similar role to the generic base class in is_base_of. That's why the ordering is that way 🤷
There was a problem hiding this comment.
The English is backward, though: "X is base of Y" and "Y is a X".
There was a problem hiding this comment.
That's fair. I have no preference for what you want to do in this PR :) Just wanted to give context as to why I made is_a the way it is!
There was a problem hiding this comment.
If neither of you have a preference, I would prefer to reverse the order of the template parameters
Add a similar code block as we do for the false branch of ASSERT? |
|
Right. Of course. Added a commit for that (at the beginning). Did the rename without a fixup, because it was a find-replace rename. Waiting to see if anyone else wants to weigh in on argument ordering. |
e181460 to
394c523
Compare
|
okay sounds good |
Avoids warnings about extra semicolons.
We haven't supported removing tags for a while.
This file seems to contain a fairly random set of things: Tags::DataBox, db::const_item_type, and some internal DataBox helpers. It should probably be split and reorganized, but that's a bigger project.
Also includes two cleanup commits related to DataBox to avoid recompilations for a second PR touching DataBox.hpp.
Proposed changes
Upgrade instructions
Code review checklist
make docto generate the documentation locally intoBUILD_DIR/docs/html.Then open
index.html.code review guide.
bugfixornew featureif appropriate.Further comments