Rcsp cleanup

[andreacassioli]

In this PR I introduce a breaking change to the RCSP algorithm interface along several modernization of the code

Breaking change:

• the RCSP interface does NOT require an edge map (it never had) and thus it should not be required. Thus I remove it all along. This fixes Is EdgeIndexMap needed in r_c_shortest_paths ?  #342

Other changes:

• the edge feasibility flag is moved to be local in to the extension function evaluation
• when accessing the final node list at destination, use a reference instead of a copy
• when a single solution is asked for, move it instead of copy.
• removed macro selecting the allocator deponding on C++ version is obsolete because it tests C++11 which is not supported.
• docs updated

Modernization

• the code is simplified by using auto extensively, that allows fewer type definitions and less verbose code
• some typedef has been rewritten with using
• use std::tie instead of boost::tie

[jeremy-murphy]

Great, thanks, I'll try to have a close look soon.

[Becheler]

HI @jeremy-murphy !
Just pinging you here, as this PR is interface-breaking it will require your supervision.
I had a glance over the PR and it seems good. Maybe some aggressive auto-ing but without proper guidelines from us it's expected 😄

It should also be merged before #491 since it touches documentation page removed by #491

[andreacassioli]

@Becheler I think this PR shall not be a blocker for #491 - I can update the new docs after the merge if it fits better.

I am curious about the guidelines for auto, maybe some more to consider in the contribution guidelines?

[Becheler]

@andreacassioli the merge order is mostly to simplify your life so you don't have to rebase again + solve conflicts once the file you edited got deleted 😄

As for the auto guidelines, yes I'm wondering about what those could be. My current mental model is that auto optimises for the writer's convenience at the cost of the reader's understanding. A new contributor should understand a variable's type, semantic and purpose without running a compiler. I guess something around those lines:

Use auto for:

• Range-based for loops: for (auto v : boost::make_iterator_range(vertices(g)))
• Lambdas: auto fun = [](){ ... }
• Iterator variables: auto it = container.begin()
• Structured bindings (when/if C++17): auto [ei, ei_end] = edges(g)

Do not use auto :

• When a meaningful type alias already exists (use the alias)
• Function return types in headers: forces readers to mentally execute the function
• Anywhere a narrowing conversion could hide: size_t/int mismatches should be visible, not obscured
• Variables where the type carries semantic meaning : spell out vertex_descriptor, edge_descriptor, size_type
• Property map expressions : auto x = get(vertex_index, g) hides what kind of map x is. Use a type alias or explicit type

I think in highly generic library code like BGL we will tend to desire explicitness for review more than type hiding.

But this is open discussion at this point, what do you think? I haven't talked about it with Jeremy - I will raise the point next time we chat :)

[github-actions]

Compiler-warning counts vs develop (auto-generated).
PR run 27164047951 vs develop run 27130796819 (1f9a0c76c2).

Job	Baseline	After	Delta
macos (clang, 14)	46	42	-4
macos (clang, 17)	41	37	-4
macos (clang, 20)	41	37	-4
ubuntu (clang-19, 14)	46	42	-4
ubuntu (clang-19, 17)	46	42	-4
ubuntu (clang-19, 20)	46	42	-4
ubuntu (clang-19, 23)	46	42	-4
ubuntu (gcc-14, 14)	28	24	-4
ubuntu (gcc-14, 17)	28	24	-4
ubuntu (gcc-14, 20)	28	24	-4
ubuntu (gcc-14, 23)	28	24	-4
windows_msvc_14_3 (msvc-14.3)	974	974	0

[andreacassioli]

@Becheler I have a bit of a different habit on auto: basically use it always except

• if a cast of sort is needed
• if the type shall be used after somehow (like for traits)
• if readibility is a problem (like call to a library that use some unclear naming)

that's pretty much it. I actually think that in a generic setting it really fulfill its purpose, i.e. when the logic should abstract the type. Also cumbersome long typedef to extract hidden types are not readable at all to me.

But again, personal taste. And it is a topic worth an entire discussion thread!

For this PR, shall I take a round to see if I have push a bit far? or shall I wait for the review?

[Becheler]

Hi @andreacassioli 🥳

Yes it's an entire discussion thread to have to reach a proper guideline. I will try to summarize my take, your take and Jeremy's in a distinct RFC. From what I understand Jeremy leans more towards your philosophy 🤓

Regardless of the auto thing, I think it should not prevent this PR from being merged: you cleaned the code, removed 4 warnings on all matrix (congrats 🥳🤩) and the tests pass and the code base ends better. I will have a closer look tomorrow on the details - I got stuck today trying to rerun this stubborn ci drone and was putting your PR in the red 😛

Good night up there in the North !