[PATCH] D67867: [libc] Add few docs and implementation of strcpy and strcat.

[David L. Jones via Phabricator via llvm-commits]

dlj added a comment.

In D67867#1684296 <https://reviews.llvm.org/D67867#1684296>, @MaskRay wrote:

> In D67867#1683099 <https://reviews.llvm.org/D67867#1683099>, @sivachandra wrote:
>
> > In D67867#1682894 <https://reviews.llvm.org/D67867#1682894>, @jyknight wrote:
> >
> > > I think one directory per function is going to be annoying, not helpful. Most libc functions are expressible as a single function of less than 100 LOC. I'd suggest, instead, to have e.g. `src/stdlib/strcpy.cc`, implementing the strcpy function. It seems like a good rule that every file implements one public function. If additional files are needed for implementation clarity, I'd suggest to put such helper code into something like `src/stdlib/internal/strcpy_internal.cpp` (obviously not _actually_ for strcpy)
> >
> >
> > My choice here is driven by my preference to keep tests co-located with the implementation. Since we are in general going to have different kinds of testing like unit-testing, differential testing, fuzz-testing, I have chosen to put implementations of individual functions in their own directories. This way, the tests (which will span multiple files) will all be in a single directory and avoid clutter in the higher level directory. In the rare occasion that implementation also spans multiple files, all the implementation files will also live together.
>
>
> Some tests will involve several functions from a header, e.g. flockfile+ftrylock+funlockfile. Where will you place the test, under flockfile/, or funlockfile/ ?
>
> I prefer jyknight's proposed hierarchy.


The source tree layout should probably reflect the structure of implementation (which we decide and work within), not just the structure of the interface (which is specified for us).

Another way to think about it: within the implementation, how can we localize changes?

Siva mentioned tests, and I think that's quite instructive: the testing boundary provides a natural cut point for functions that must be tested mutually.

File locking functions are practically inseparable, so it's reasonable to keep them together. However, that doesn't imply they should be grouped with unrelated functions, nor that all functions should be lumped together. A finer-grained directory structure seems quite intuitive for expressing these chunks of inseparable functionality.



================
Comment at: libc/docs/header_generation.md:7
+approaches have served them well, parts of their systems have become extremely
+complicated making it hard to modify, extend or maintain. To avoid these
+problems in llvm-libc, we use a header generation mechanism. The mechanism is
----------------
sivachandra wrote:
> dlj wrote:
> > Unrequested opinion: my personal complaint in this area is that reading the files (as a human) requires understanding the entirety of the configuration macros, plus usually double-checking `clang -E -dM`, just to figure out what the preprocessor result will be. Generating files up-front at least carves out the internal configuration part.
> > 
> > I'm not sure how to word that eloquently, but it might be possible to say something a bit more concrete than the current wording.
> I made starts to address this comment, but scrubbed them all out as I do not think I am doing a good job. Do you have any suggestion, or some specific points you want to see covered here?
Eh, I'll think about it. Don't block on me... I can send a patch if I come up with something.


================
Comment at: libc/docs/header_generation.md:38
+
+## Parameter Syntax
+
----------------
sivachandra wrote:
> dlj wrote:
> > Wording nit: the distinction between "parameter" and "argument" caught me a bit by surprise: the "arguments" here are what I would call "parameters" -- things that parameterize the target; the term "parameters" here are free-floating values, but they could be passed as as what I would call "arguments." (This is mostly following the way these terms are used in C++: functions are parameterized, call expressions have arguments.)
> > 
> > I really don't want to get into a bikeshed argument here, though. If you don't agree with my reasoning, I can live with this as-is, too. :-)
> I agree there was some mix up. I made minor changes now. How does it look?
This is definitely more consistent, but I think it's still the reverse of what I had in mind: "${thing}" is a specific value that exists at call sites, so I would call that an argument. The thing defined by the called function (with unspecified value) is a parameter.


```
int foo(int a); // 1 parameter named "a"
int b;
int c = foo(b); // 1 argument named "b"
```

The usage below of "Parameters" in `include_file` matches my expectation, but "Arguments" for `begin` seems at odds with this.

(The specific wording I'm thinking of is in the C++ standard: [dcl.fct] and [expr.call].)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67867/new/

https://reviews.llvm.org/D67867