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

Remove cbegin and cend calls which do not exist in std::span or gsl::span #10426

Merged
merged 1 commit into from
Jan 28, 2022

Conversation

fdwr
Copy link
Contributor

@fdwr fdwr commented Jan 28, 2022

Description: Remove calls to cbegin and cend introduced in #10193 which are removed methods (see #1 #2), not present on std::span or gsl::span.

Motivation and Context

  • Why is this change required? What problem does it solve? It breaks the WindowsAI build, conflicting because we use GSL's span, not GSL lite (which has the stale cbegin/cend). Plus, the iterators are already const anyway, given the function parameter is gsl::span<const int64_t>.
  • If it fixes an open issue, please link to the issue here. NA

@fdwr fdwr requested a review from yuslepukhin January 28, 2022 07:36
@snnn
Copy link
Member

snnn commented Jan 28, 2022

It seems that you are using a different gsl version than us. We should make it consistent.

@yuslepukhin
Copy link
Member

yuslepukhin commented Jan 28, 2022

I have not objection to this PR. The only thing, the number of places seems awfully small. I use cbegin() and cend() left and right. We should add it to the Coding Guidelines?

@fdwr
Copy link
Contributor Author

fdwr commented Jan 28, 2022

The only thing, the number of places seems awfully small. I use cbegin() and cend() left and right.

@yuslepukhin: Yeah, you use it often, but for vector and string and such. For span, those were the only two places I encountered it.

@fdwr
Copy link
Contributor Author

fdwr commented Jan 28, 2022

@snnn I see orttraining-amd-gpu-ci-pipeline failing, but my change is orthogonal to that (whatever the cause). So I'll submit given @yuslepukhin said he has no objections. As for how you want to update coding guidelines, I'll leave that to you 😊. Thanks.

@fdwr fdwr merged commit b02f4ec into master Jan 28, 2022
@fdwr fdwr deleted the user/dwayner/SpanCbeginCend branch January 28, 2022 22:25
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.

3 participants