Skip to content

Change uint32_t to std::uint32_t in FileException #674

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.

Sign up for GitHub

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

Jump to bottom
Merged
gopalrs merged 6 commits into from
Nov 3, 2025
Merged

Change uint32_t to std::uint32_t in FileException #674

gopalrs merged 6 commits into from
Nov 3, 2025

Conversation

@gptabhinav
Copy link
Contributor

@gptabhinav gptabhinav commented Sep 27, 2025

  • [ +] Does this PR have a descriptive title that could go in our release notes?
  • Does this PR add any new dependencies?
  • Does this PR modify any existing APIs?
    • Is the change to the API backwards compatible?
  • Should this result in any changes to our documentation, either updating existing docs or adding new ones?

Reference Issues/PRs

Fixes #673

What does this implement/fix? Briefly explain your changes.

The build was failing with:

include/ann_exception.h:22:65: error: ‘uint32_t’ has not been declared

This PR adds #include to include/ann_exception.h and updates the function signature to use std::uint32_t for portability.

Any other comments?

  • Verified the fix on Ubuntu 22.04 with g++ 11.4.
  • This is a minimal, backward-compatible change that resolves the compilation error without impacting APIs or introducing dependencies.

@Essisore
Copy link

Essisore commented Sep 28, 2025

It seems "include/pq_data_store.h" also has this problem

@gptabhinav
Copy link
Contributor Author

gptabhinav commented Oct 7, 2025

@gopalrs review please?

@gptabhinav
Upgrade upload-artifact action to version 4
a446f57 
v3 is deprecated and breaking build
@gptabhinav
Copy link
Contributor Author

gptabhinav commented Oct 12, 2025 •
edited
Loading

@gopalrs, this had some formatting issues, and also,
workflow was breaking, because it was still using one of the deprecated artifacts here -->
https://github.com/microsoft/DiskANN/actions/runs/18065906241/job/52552125885. i have modified this.

can you help approve?

@gptabhinav
Fix clang-format violations in utils.h and windows_aligned_file_reader.h
6ecd3fd 
- Consolidate multi-line cout statement in utils.h
- Add proper spacing in WindowsAlignedFileReader constructor
@gptabhinav
Fix all clang-format violations across codebase
c45b967 
- Fix multi-line cout statements in src/in_mem_data_store.cpp
- Fix multi-line cout statements in src/index.cpp
- Fix multi-line cout statements in apps/utils/vector_analysis.cpp
- Fix multi-line cout statements in apps/utils/stats_label_data.cpp
- Fix multi-line cout statements in apps/test_streaming_scenario.cpp
@gptabhinav
Set up Python 3.11 and upgrade pip in workflow
452139c 
- latest available build for diskannpy in pypi is compatible with 3.11, no compatible version exists for 3.12 which is the default python version for runner os ubuntu 24.04
- Added steps to set up Python 3.11 and upgrade pip in the workflow.
@gptabhinav
Copy link
Contributor Author

gptabhinav commented Oct 12, 2025

@microsoft-github-policy-service agree

@gptabhinav
Copy link
Contributor Author

gptabhinav commented Oct 12, 2025

@gopalrs, updated python version for the ubuntu build. latest ubuntu LTS version has python 3.12.x, which is not compatible with diskannpy, so had to revert to last compatible version 3.11.x for the build.
checked locally, and build completed successfully for my fork -- https://github.com/gptabhinav/DiskANN/actions/runs/18449799506
can you please approve the updated workflow

@gptabhinav
Copy link
Contributor Author

gptabhinav commented Oct 13, 2025

@gopalrs, this is pending workflow approval and merge. can you please help take a look?

@gopalrs
Copy link
Contributor

gopalrs commented Oct 17, 2025

@gptabhinav Sorry for the delay, we are not actively maintaining this repo. I've approved the changes. There are some formatting issues, so you may want to re-run the format tool and resubmit.

@gptabhinav
Copy link
Contributor Author

gptabhinav commented Oct 17, 2025

@gopalrs makes sense. i was just working on this for my personal project, but was facing build failures, so added a fix here. i have updated the yaml to create different build artifact names based on jobid. this should fix the duplicate artifact build issue. reran clang formatting, but did not get any formatting changes this time. I think it already updated everything in the last push i did. can you review when you see this. and merge and close the pr it there is nothing else. thanks!

@gptabhinav
Copy link
Contributor Author

gptabhinav commented Nov 1, 2025

@gopal-msr, not sure why, but merging is blocked

@gopalrs gopalrs merged commit a26f824 into microsoft:main Nov 3, 2025
20 of 32 checks passed
@gopalrs
Copy link
Contributor

gopalrs commented Nov 3, 2025

@gptabhinav, merge done now. Thanks for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gopalrs gopalrs gopalrs approved these changes

+1 more reviewer

@gopal-msr gopal-msr gopal-msr approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] Missing <cstdint> include in ann_exception.h causes uint32_t compilation error

4 participants

@gptabhinav @Essisore @gopalrs @gopal-msr