Skip to content

Conversation

@miniksa
Copy link
Member

@miniksa miniksa commented Jan 6, 2020

Summary of the Pull Request

Introduces a type that is basically an array (stack allocated, fixed size) that reports size based on how many elements are actually filled (from the front), iterates only the filled ones, and has some basic vector push/pop semantics.

PR Checklist

  • I work here
  • I work here
  • I work here
  • I'd love to roll this out to SomeViewports.... maybe in this commit or a follow on one.
  • We need a TIL tests library and I should test this there.

Detailed Description of the Pull Request / Additional comments

The original gist of this was used for SomeViewports which was a struct to hold between 0 and 4 viewports, based on how many were left after subtraction (since rectangle subtraction functions in Windows code simply fail for resultants that yield >=2 rectangle regions.)

I figured now that we're TIL-ifying useful common utility things that this would be best suited to a template because I'm certain there are other circumstances where we would like to iterate a partially filled array and want it to not auto-resize-up like a vector would.

Validation Steps Performed

  • TIL tests added

@miniksa miniksa requested a review from DHowett-MSFT January 6, 2020 17:55
src/inc/til.h Outdated
return _Elems;
}

void push_back(const _Ty& _Val)
Copy link
Member Author

Choose a reason for hiding this comment

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

@DHowett-MSFT, in vector, there's a lot about push_back turning into emplace_back which is very complicated. I'd like if you look over this with me and see if you can decode whether any of the stuff that is used for push/pop/emplace in things like vector is something we need here.

Copy link
Contributor

Choose a reason for hiding this comment

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

not totally sure here. need to look a bit.

Copy link
Contributor

@DHowett-MSFT DHowett-MSFT left a comment

Choose a reason for hiding this comment

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

NAK on using STL internals and perhaps deriving array

@ghost ghost added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Jan 6, 2020
… privates, make a til test library, make some tests for some.
Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

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

I'll give you a check when the scripts are updated 😜

@zadjii-msft zadjii-msft added Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Product-Meta The product is the management of the products. labels Jan 7, 2020
@DHowett-MSFT
Copy link
Contributor

Whoa, did you murder the non-const versions of things?

Copy link
Member Author

@miniksa miniksa left a comment

Choose a reason for hiding this comment

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

Whoa, did you murder the non-const versions of things?

Yes because when I was testing it, you could use the non-const ones to edit things outside the _Used count pretty easily. Do you want them back? I think most of the use of this is to fill it up the once and then give it to someone to iterate constantly, so I don't think we need them.

@DHowett-MSFT
Copy link
Contributor

want them back

nope, no need

@miniksa miniksa marked this pull request as ready for review January 7, 2020 18:37
@miniksa miniksa merged commit 735d2e5 into master Jan 9, 2020
@miniksa miniksa deleted the dev/miniksa/some branch January 9, 2020 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Product-Meta The product is the management of the products.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants