Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PR #22822: Fix ambiguous constructor call in SourceTargetPairs initialization #23044

Merged
merged 1 commit into from
Feb 24, 2025

Conversation

copybara-service[bot]
Copy link

PR #22822: Fix ambiguous constructor call in SourceTargetPairs initialization

Imported from GitHub PR #22822

Description

Resolve a build failure (with GCC-11) in collective_permute_cycle_test caused by an ambiguous constructor call when initializing SourceTargetPairs with an empty list ({{}}).

Issue

When calling SourceTargetPairs({{}}), the compiler could not determine whether to use the std::vector<std::pair<int64_t, int64_t>> constructor or the default copy/move constructors, leading to an error:

xla/service/collective_permute_cycle_test.cc:130:48: error: call of overloaded 'SourceTargetPairs(<brace-enclosed initializer list>)' is ambiguous
  130 |   EXPECT_EQ(GetCycleType(SourceTargetPairs({{}})), CycleType::kNone);

Solution

  1. Explicitly define an initializer_list constructor for SourceTargetPairs to properly handle {} and {{src, tgt}} initializations.
  2. Update the test case to use default ctor SourceTargetPairs() instead of SourceTargetPairs({{}}), ensuring clarity and correctness.

This fix ensures proper initialization and eliminates ambiguity

Tested with GCC-11
Copybara import of the project:

--
f97c38d by Alexander Pivovarov [email protected]:

Fix ambiguous constructor call in SourceTargetPairs initialization

Merging this change closes #22822

FUTURE_COPYBARA_INTEGRATE_REVIEW=#22822 from apivovarov:fix_source_target_pairs f97c38d

…lization

Imported from GitHub PR #22822

### Description
Resolve a build failure (with GCC-11) in `collective_permute_cycle_test` caused by an ambiguous constructor call when initializing `SourceTargetPairs` with an empty list (`{{}}`).

#### Issue
When calling `SourceTargetPairs({{}})`, the compiler could not determine whether to use the `std::vector<std::pair<int64_t, int64_t>>` constructor or the default copy/move constructors, leading to an error:
```
xla/service/collective_permute_cycle_test.cc:130:48: error: call of overloaded 'SourceTargetPairs(<brace-enclosed initializer list>)' is ambiguous
  130 |   EXPECT_EQ(GetCycleType(SourceTargetPairs({{}})), CycleType::kNone);
```

#### Solution
1. Explicitly define an `initializer_list` constructor for `SourceTargetPairs` to properly handle `{}` and `{{src, tgt}}` initializations.
2. Update the test case to use default ctor `SourceTargetPairs()` instead of `SourceTargetPairs({{}})`, ensuring clarity and correctness.

This fix ensures proper initialization and eliminates ambiguity

Tested with GCC-11
Copybara import of the project:

--
f97c38d by Alexander Pivovarov <[email protected]>:

Fix ambiguous constructor call in SourceTargetPairs initialization

Merging this change closes #22822

COPYBARA_INTEGRATE_REVIEW=#22822 from apivovarov:fix_source_target_pairs f97c38d
PiperOrigin-RevId: 730610452
@copybara-service copybara-service bot merged commit d4b3756 into main Feb 24, 2025
@copybara-service copybara-service bot deleted the test_730590032 branch February 24, 2025 23:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant