Page MenuHomePhabricator

Add hasNItemsOrLess and container variants of hasNItems and friends
ClosedPublic

Authored by jurahul on Fri, Jun 19, 1:15 PM.

Details

Summary
  • Fixed a bug in hasNItems()
  • Extend STLExtras unit test to add testing for hasSingleElement() and hasNItems() and friends.

Diff Detail

Event Timeline

jurahul created this revision.Fri, Jun 19, 1:15 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald Transcript
rriddle accepted this revision.Fri, Jun 19, 2:01 PM
This revision is now accepted and ready to land.Fri, Jun 19, 2:01 PM
jurahul updated this revision to Diff 272190.Fri, Jun 19, 2:45 PM

Fix clang-tidy failures

jurahul updated this revision to Diff 272214.Fri, Jun 19, 4:44 PM

Another attemp to fix clang-tidy issues

mehdi_amini added inline comments.Fri, Jun 19, 5:17 PM
llvm/include/llvm/ADT/STLExtras.h
1951

Nit: period.

1966

Seems like two patches here, this added function to STLExtras seems to be the only thing relevant for the MLIR part. Can you split this?

jurahul added inline comments.Fri, Jun 19, 6:01 PM
llvm/include/llvm/ADT/STLExtras.h
1966

Sure, I can split the MLIR adoption of this into its own change.

mehdi_amini added inline comments.Fri, Jun 19, 8:31 PM
llvm/include/llvm/ADT/STLExtras.h
1966

I meant that hasNItems and the MLIR change can go in together, but the addition of the others hasNItemsOrLess and hasNItemsOrMore don't seem related here.

jurahul updated this revision to Diff 272519.Mon, Jun 22, 12:43 PM

Split the MLIR adoption into its own change, and add unit tests

jurahul edited the summary of this revision. (Show Details)Mon, Jun 22, 12:44 PM
jurahul updated this revision to Diff 272520.Mon, Jun 22, 12:46 PM

Complete sentence for comments

Harbormaster completed remote builds in B61283: Diff 272519.
jurahul marked 2 inline comments as done.Mon, Jun 22, 3:00 PM
jurahul added inline comments.
llvm/include/llvm/ADT/STLExtras.h
1966

I have split the MLIR change and added unit tests for these functions (and fixed a bug in hasNItems)

mehdi_amini accepted this revision.Mon, Jun 22, 3:03 PM

Thanks for adding the tests! Did you find the bug while adding these by the way?

This revision was automatically updated to reflect the committed changes.
jurahul marked an inline comment as done.