Reject explicit generations after auto generation on the same communicator

[iemAnshuman]

Fixes #

Proposed Changes

• Latch auto-generation use on a communicator and reject later explicit generation_arg values with bad_parameter, so callers cannot guess a generation after the internal counter has advanced unobservably.
• Keep explicit → auto transitions valid; document that all participants of one operation must use the same generation mode.
• Store the communicator basename by value so diagnostic messages do not use a dangling char const*.
• Add a regression test for auto → explicit rejection, explicit → auto success, and persistence of the latch after the reverse transition.

Any background context you want to provide?

After an auto/default generation advances the and-gate, the caller no longer knows a reliable next explicit generation: too small previously threw from deep in the gate, too large could hang. This change rejects that transition at the communicator boundary with a clear error.

Checklist

Not all points below apply to all pull requests.

• I have added a new feature and have added tests to go along with it.
• I have fixed a bug and have added a regression test.
• I have added a test using random numbers; I have made sure it uses a seed, and that random numbers generated are valid inputs for the tests.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: bb3ff814-fb58-4733-8935-aa328d9bde23

📥 Commits

Reviewing files that changed from the base of the PR and between cd8b7d3 and 9145fc5.

📒 Files selected for processing (2)

• libs/full/collectives/include/hpx/collectives/detail/communicator.hpp
• libs/full/collectives/src/create_communicator.cpp

🚧 Files skipped from review as they are similar to previous changes (2)

• libs/full/collectives/include/hpx/collectives/detail/communicator.hpp
• libs/full/collectives/src/create_communicator.cpp

---

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes
  • Improved validation of hierarchical communicator generation-mode usage.
  • If a communicator instance has used the default/auto generation for an operation, later operations using an explicit generation number are rejected with a clear parameter error.
  • Switching from explicit generation back to default/auto remains supported.
  • All participants in a single operation must use the same generation mode.
• Documentation
  • Clarified generation sequencing rules and valid transition patterns.
• Tests
  • Added coverage for generation-mode transition behavior in hierarchical mixed collective cases.

Walkthrough

The change documents and enforces generation-mode ordering for hierarchical communicators. Communicators retain whether auto-generation was used, reject later explicit generations with bad_parameter, and add tests covering valid and invalid transitions.

Changes

Generation Mode Enforcement

Layer / File(s)	Summary
Generation contract and enforcement libs/full/collectives/include/hpx/collectives/argument_types.hpp, libs/full/collectives/include/hpx/collectives/create_communicator.hpp, libs/full/collectives/include/hpx/collectives/detail/communicator.hpp, libs/full/collectives/src/create_communicator.cpp	Documentation defines valid generation-mode transitions. Communicator state records auto-generation usage and rejects later explicit generations with hpx::error::bad_parameter; constructor storage changes support the new state.
Generation transition validation libs/full/collectives/tests/unit/cross_collective_hierarchical_mixed.cpp	Tests verify auto-to-explicit rejection, explicit-to-auto success, and rejection of explicit generation after switching back to auto across configured local site counts and arities.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested labels: type: compatibility issue, category: components

Suggested reviewers: hkaiser

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: rejecting explicit generations after auto generation on the same communicator.
Description check	✅ Passed	The description is directly related to the code changes and test coverage in the pull request.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[StellarBot]

Can one of the admins verify this patch?

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[iemAnshuman]

@hkaiser One note on storing basename_ as std::string rather than char const* (this came up in #7340).

I prefer keeping the owned string here for three reasons:

1. Hierarchical create really does pass a short-lived buffer. recursively_fill_communicators builds a stack std::string name, then calls create_communicator(name.c_str(), ...). That pointer is what ends up in the server; the stack string is gone once the recursive fill returns, while the component lives on.
2. AGAS does not backstop this on the server. Registration keeps a name on the client (register_as / registered_name). The server’s basename_ is a separate diagnostics-only field and is not that registered name.
3. This PR makes the field more likely to be read on hierarchical servers. The generation-mode latch throws bad_parameter with basename_ in the message, and the new regression test exercises that path on hierarchical communicators. With a borrowed pointer that is use-after-free on the error path (and noise/ASan risk), not just a theoretical concern.

I know the pointer form was intentional for flat/literal basenames and keeps the ctor noexcept. For the common flat + string-literal case it is fine; hierarchical tree construction is the path that breaks the “always stable” assumption.

Alternatives I considered and would rather not take in this PR:

• Keep char const*: leaves hierarchical diagnostics dangling.
• Keep the pointer but make hierarchical names long-lived elsewhere (e.g. own name storage on hierarchical_communicator): correct in principle, but a larger lifetime redesign than this fix warrants.

Happy to adjust if I’m missing a reason the hierarchical name.c_str() is guaranteed to outlive the server, from the create path I don’t see one. Cost-wise this is a single copy at communicator construction, not on the collective hot path.