Skip to content

ROCM-20419 - Remove dep on MAX_SIZE.#4241

Open
jaydeeppatel1111 wants to merge 2 commits intodevelopfrom
ROCM-20419
Open

ROCM-20419 - Remove dep on MAX_SIZE.#4241
jaydeeppatel1111 wants to merge 2 commits intodevelopfrom
ROCM-20419

Conversation

@jaydeeppatel1111
Copy link
Contributor

Motivation

Remove dep on MAX_SIZE.

Technical Details

Remove dep on MAX_SIZE.

JIRA ID

ROCM-20419

Test Plan

NA

Test Result

NA

Submission Checklist

@jaydeeppatel1111 jaydeeppatel1111 requested a review from a team as a code owner March 20, 2026 10:41
Copilot AI review requested due to automatic review settings March 20, 2026 10:41
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes the local dependency on MAX_SIZE in the HIP multiprocess device visibility tests by replacing fixed-size buffers/arrays with std::string and std::vector.

Changes:

  • Replaced MAX_SIZE-based snprintf buffer with std::to_string() + std::string for *_VISIBLE_DEVICES env var values.
  • Replaced MAX_SIZE-based device list arrays with std::vector<int> and passed raw pointers via .data() where needed.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 14 to 18
#include <hip_test_common.hh>

#ifdef __linux__
#include <sys/wait.h>
#include <unistd.h>
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file now uses std::string/std::to_string and std::vector, but it doesn’t include or . Relying on transitive includes (e.g., via iostream/Catch2) is non-portable and can break builds with different standard library implementations. Add the missing standard headers explicitly near the top of the file.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback

Copy link
Contributor

Copilot AI commented Mar 20, 2026

@jaydeeppatel1111 I've opened a new pull request, #4251, to work on those changes. Once the pull request is ready, I'll request review from you.

….cc (#4251)

`hipSetGetDeviceMproc.cc` uses `std::string`, `std::to_string`, and
`std::vector` without explicitly including their respective headers.
These are not transitively provided by `hip_test_common.hh`.

## Motivation

Missing standard library headers cause implicit dependency on transitive
includes, which is fragile and non-portable. This ensures the file's
dependencies are explicitly declared.

## Technical Details

- Added `#include <string>` and `#include <vector>` to
`projects/hip-tests/catch/multiproc/hipSetGetDeviceMproc.cc` following
the existing `hip_test_common.hh` include.

Addresses feedback from
[#4241](#4241 (comment)).

## JIRA ID

ROCM-20419

## Test Plan

No functional changes — header-only fix. Relies on compiler to catch any
regressions if headers are missing.

## Test Result

N/A — compilation correctness ensured by explicit include declarations.

## Submission Checklist

- [ ] Look over the contributing guidelines at
https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.

<!-- START COPILOT CODING AGENT TIPS -->
---

🔒 GitHub Advanced Security automatically protects Copilot coding agent
pull requests. You can protect all pull requests by enabling Advanced
Security for your repositories. [Learn more about Advanced
Security.](https://gh.io/cca-advanced-security)

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: jaydeeppatel1111 <106300970+jaydeeppatel1111@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants