This is an archive of the discontinued LLVM Phabricator instance.

[libc] Add a span facility
ClosedPublic

Authored by gchatelet on Aug 19 2022, 10:53 AM.

Details

Summary

This is intended to replace ArrayRef.

Diff Detail

Event Timeline

gchatelet created this revision.Aug 19 2022, 10:53 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptAug 19 2022, 10:53 AM
gchatelet requested review of this revision.Aug 19 2022, 10:53 AM

Where do you use enable_if? data_ is libc coding conventions? It is not conforming to the LLVM coding conventions.

  • Address comments

Where do you use enable_if? data_ is libc coding conventions? It is not conforming to the LLVM coding conventions.

Thx @tschuett for the comments. Done.

tschuett added inline comments.Aug 19 2022, 11:59 AM
libc/src/__support/CPP/span.h
18

Nit: You may use nested namespaces.

83

Nit: snake case?

gchatelet updated this revision to Diff 454102.Aug 19 2022, 1:43 PM
gchatelet marked an inline comment as done.
  • Address comments
gchatelet marked an inline comment as done.Aug 19 2022, 1:43 PM
gchatelet added inline comments.
libc/src/__support/CPP/span.h
83

Nit:snake case?

Did you meant camelCase for dynamic_extent?

sivachandra added inline comments.Aug 19 2022, 1:50 PM
libc/src/__support/CPP/span.h
83

I think s/countToSize/count_to_size/

gchatelet added inline comments.Aug 19 2022, 2:17 PM
libc/src/__support/CPP/span.h
83

It was count_to_size (snake case) originally and I thought @tschuett asked me to go to camelCase. Then I figured out that the modifications I made to the file probably messed out with the lines on which the comment was attached (Phabricator... sigh). I inferred that it was probably dynamic_extent that was the culprit rather than count_to_size. But now I'm confused😅

sivachandra added inline comments.Aug 19 2022, 2:27 PM
libc/src/__support/CPP/span.h
83

I am also confused. Normal LLVM conventions do not apply in the libc directory, and the cpp namespace is even more special. So, I think count_to_size is correct. Likewise Data and Size are also not compliant. We can go with span_data and span_size to disambiguate with the methods data and size.

gchatelet updated this revision to Diff 454115.Aug 19 2022, 3:08 PM

Address comments and rebase

gchatelet updated this revision to Diff 454117.Aug 19 2022, 3:10 PM
  • Also convert countToSize to snake_case
gchatelet marked 2 inline comments as done.Aug 19 2022, 3:10 PM
sivachandra accepted this revision.Aug 19 2022, 10:23 PM
This revision is now accepted and ready to land.Aug 19 2022, 10:23 PM

https://llvm.org/docs/CodingStandards.html#id43

"As an exception, classes that mimic STL classes can have member names in STL’s style of lower-case words separated by underscores (e.g. begin(), push_back(), and empty()). Classes that provide multiple iterators should add a singular prefix to begin() and end() (e.g. global_begin() and use_begin())."

This revision was landed with ongoing or failed builds.Aug 22 2022, 12:27 AM
This revision was automatically updated to reflect the committed changes.