[libcxx-commits] [PATCH] D144994: [Draft][libc++][modules] Adds std module.

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/stdmodules/std-format.cppm:12
+// TODO This file is two-fold
+// - look at how we want to integrate a synopsis
+// - test whether the experimental library works properly
----------------
ldionne wrote:
> philnik wrote:
> > Mordante wrote:
> > > aaronmondal wrote:
> > > > philnik wrote:
> > > > > My stance is still that we should just not have it/remove it. Having notes on the papers/extra status pages for large papers is way more reliable than looking through the synopsis, but I guess nobody agrees with me here.
> > > > +1 from me.
> > > > 
> > > > Personally, I find the style you suggested in https://reviews.llvm.org/D135507 very readable.
> > > > 
> > > > Potentially with links to the corresponding sections/papers even. A broken/outdated link is still a lot better than a potentially ever so slightly outdated docstring.
> > > > My stance is still that we should just not have it/remove it. Having notes on the papers/extra status pages for large papers is way more reliable than looking through the synopsis, but I guess nobody agrees with me here.
> > > 
> > > I disagree that nobody agrees with you. I'm also not fond of the current synopsis, since it's quite often out of date. I discussed that a few weeks ago privately with Louis and when we discussed modules he thought these files would be a better place for the synopsis. That's why I added it here, however for now I only do that for one header, where I exactly know the implementation status. For other headers this will be more effort.
> > Good to know.
> Well, if nobody like synopsis in headers, we can also remove them :-). Let's chat about that.
> 
> To be clear, you are not opposed to keeping the synopsis comment in test files that describes what we're testing, right? I think those are definitely useful and we should not stop those.
> 
> Pinging @var-const @huixie90 for thoughts. If folks agree, I don't want to impose it on people.
> 
> FWIW I do agree that storing those in the `.cppm` files results in something less readable than I thought it would.
> Well, if nobody like synopsis in headers, we can also remove them :-). Let's chat about that.
> 
> To be clear, you are not opposed to keeping the synopsis comment in test files that describes what we're testing, right? I think those are definitely useful and we should not stop those.
I'm not sure they are always super useful, but they are definitely in enough cases that it would probably be a bad idea to remove them. They are also much easier to maintain and less error-prone.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144994