boostorg/hana

Your standard library for metaprogramming

Louis Dionne

@ldionne

@ricejasonf It’s fine, it was just a timeout when downloading Boost. I restarted that job.

@ricejasonf Would you mind squashing these commits together, and then push —forceing to update the PR?

Otherwise, I can just squash it myself; you decide.

Jason Rice

@ricejasonf

I would like to do it

..once I figure it out :P

Louis Dionne

@ldionne

The following should work. Execute the commands one by one, and make sure it makes sense at each step, because I haven't proofread the steps and I don't want to delete your whole hard drive:

git checkout the-name-of-your-branch
# make sure your working directory is clean
git rebase -i HEAD~3

A file should pop-up in your default editor. Follow the instructions written in that file. Basically, you should put squash instead of pick in front of your 2 last commits. Once you're done, save it and close it. The rebase should take place, and you'll be asked to edit the commit message of the squashed commit; do it. Normally, you'll be left with a single commit containing the changes of your 3 current commits, and with the commit message that you'll have chosen. Now, you can push --force your new commit to your branch, overwriting the 3 previous commits:

git push --force

Again, take these steps with a grain of salt.

@ricejasonf Ah, I just saw that you already did it. Cool, seems like you figured it out.

Ok, so now we have a single logical commit. If I merged this, the history would make sense; there would be a single commit showing you added this feature, and not showing too much of the sausage making (the 3 patches on which I commented and which you improved).

Jason Rice

@ricejasonf

I did this

git reset --soft HEAD~3 && git commit

Louis Dionne

@ldionne

Oh, I guess it works too. But do check git rebase —interactive out, it’s wonderful.

Jason Rice

@ricejasonf

I should probably get more organized with my own project instead of grinding wip commits into master all of the time.

Louis Dionne

@ldionne

As you can see with Hana, I personally enforce a fairly strict discipline w.r.t. my Git usage, even when I am working alone on a project. At first it was a bit daunting, but now I’ve got my workflow and it’s really fast. I also find it much easier to search through my commit history and stuff like that, because the history makes sense.

Jason Rice

@ricejasonf

Yes, thanks for your help. I like how Hana is organized, but what is the rationale for separating declaration/definition with the fwd header files?

Jason Rice

@ricejasonf

nm. found it in doc/tutorial.hpp

Louis Dionne

@ldionne

Really, did I document that?

Anyway, the rationale is so that we can #include lightweight declarations instead of full blown definitions when only a declaration is requried. This can help with compile times. Also, it’s necessary for some circularly-dependent stuff.

Jason Rice

@ricejasonf

Yes, under "Header Organization" in the doc menu.

aha

Louis Dionne

@ldionne

And finally, I think it makes for a nice separation between interface and implementation, and also it gives a place to systematically put documentation. Since the implementation is sometimes hairy, I think it’s nice not to entangle the documentation with it.

Jason Rice

@ricejasonf

oh I get it fwd is for "forward" declaration :laughing:

Louis Dionne

@ldionne

Yes, yes. Lol, that would have been a strange idea otherwise.

Jason Rice

@ricejasonf

I was thinking that I could use a function that returns int_c as a predicate. What am I doing wrong here?

#include<boost/hana.hpp>

namespace hana = boost::hana;

int main()
{
  static_assert(hana::find_if(hana::make_tuple(
      hana::make_tuple(),
      hana::make_tuple(),
      hana::make_tuple(hana::int_c<0>, hana::int_c<1>),
      hana::make_tuple()), hana::length)
        ==
      hana::just(hana::make_tuple(hana::int_c<0>, hana::int_c<1>)), "");
}

/usr/local/include/boost/hana/detail/index_if.hpp:56:50: error: implicit instantiation of undefined
      template 'boost::hana::detail::index_if<const boost::hana::length_t &,
      boost::hana::detail::pack<boost::hana::tuple<boost::hana::integral_constant<int, 0>,
      boost::hana::integral_constant<int, 1> >, boost::hana::tuple<> >, boost::hana::when<true> >'
        static constexpr std::size_t value = 1 + index_if<Pred, pack<Ts...>>::value;

Jason Rice

@ricejasonf

trying to rule out "operator" abuse here :laughing: get it?

Louis Dionne

@ldionne

Can you please create an issue?

Jason Rice

@ricejasonf

k, just wanted to make sure there wasn't something glaringly obvious here

[integral_constant] ??

Louis Dionne

@ldionne

Well, it’s because your predicate does not return a boolean.

I’m not a fan of returning an int from a predicate, but it should probably still work.

Yeah, it should work cause int is a Logical.

Jason Rice

@ricejasonf

"the predicate has to return an IntegralConstant holding a value that can be converted to bool" - from find_if

Louis Dionne

@ldionne

Right, then it’s a bug.

Just open an issue on GitHub to document the problem (basically paste your above code). I’ll commit a fix in a minute.

Jason Rice

@ricejasonf

cool!

Jason Rice

@ricejasonf

I could add more checks to that test to check all functions that take a predicate (in cases where it makes sense)

unless you think that is a big can of worms

Louis Dionne

@ldionne

No, I would love that actually.

There shouldn’t be too much code changes required to make it work. It probably just requires sprinkling a couple of static_cast<bool>s around.

If you have the time, make a PR. Otherwise, I’ll check it out when my exams are done.

Jason Rice

@ricejasonf

:D I do, but it might be tomorrow. I'll see if I can fix any of them that might be broken.

Louis Dionne

@ldionne

Cool, thanks!

Jason Rice

@ricejasonf

hana::plus.by(1)

Jason Rice

@ricejasonf

though I still need to fix the formatting https://github.com/ricejasonf/hana/blob/3ce90a7d4ea945e9269c7dadac219732c0f0bb28/test/bugs/github_221.cpp

Louis Dionne

@ldionne

@ricejasonf That looks good for a test case!

Jason Rice

@ricejasonf

I should probably have a sort prove it's not using the default predicate.

Would using 'tuple_c' in all of that be okay?

Louis Dionne

@ldionne

I should probably have a sort prove it's not using the default predicate.

What do you mean?

Would using 'tuple_c' in all of that be okay?

It would, but I'd rather you leave it like it is, since tuple_c might go at some point. Since it's already written with make_tuple, there's little benefit to removing it.

Louis Dionne

@ldionne

@ricejasonf I just saw that you were using tuple_c in some places. I don’t care, just do as you please.

Jason Rice

@ricejasonf

I wanted to make sure that the tests on functions with a default predicate would not accidentally be using the default predicate because of some substitution failure, though that is certainly not the case with the current implementations.

@ldionne tuple_c and tuple_t are really convenient and make for more concise, readable code, although I guess I can't see many practical use cases for tuple_c. I did see it used sparingly in other tests, which is why I didn't completely avoid using it. I definitely hope you keep tuple_t and range_c though.

Where communities thrive

People

Repo info

Activity