Quuxplusone added inline comments.

================
Comment at: include/charconv:244
+    static _LIBCPP_INLINE_VISIBILITY char const*
+    read(char const* __p, char const* __ep, type& __a, type& __b)
+    {
----------------
mclow.lists wrote:
> lichray wrote:
> > mclow.lists wrote:
> > > Same comment as above about `read` and `inner_product` - they need to be 
> > > "ugly names"
> > Unlike `traits` which is a template parameter name in the standard, `read` 
> > and `inner_product` are function names in the standard, which means the 
> > users cannot make a macro for them (and there is no guarantee about what 
> > name you make **not** get by including certain headers), so we don't need 
> > to use ugly names here, am I right?
> I understand your reasoning, but I don't agree. 
> 
> Just last month, I had to rename a function in `vector` from `allocate` to 
> `__vallocate` because it confused our "is this an allocator" detection. The 
> function in question was private, so it shouldn't have mattered, but GCC has 
> a bug where sometimes it partially ignores access restrictions in non-deduced 
> contexts, and then throws a hard error when it comes back to a different 
> context. The easiest workaround was to rename the function in `vector`.
> 
> Since then, I've been leery of public names that match others. This is pretty 
> obscure, since it's in a private namespace, but I'd feel better if they were 
> `__read` and `__inner_product`.
> 
FWIW, +1 to ugly names. Even if the un-ugly code is "technically not broken 
yet", and besides the technical reason Marshall gives,
(1) it's nice that libc++ has style rules and sticks to them, precisely to 
*avoid* bikeshedding the name of every private member in the world;
(2) just because users can't `#define read write` doesn't mean they *won't* do 
it. I would actually be extremely surprised if `read` were *not* defined as a 
macro somewhere inside `<windows.h>`. :)

See also: "should this function call be `_VSTD::`-qualified?" Sometimes the 
answer is technically "no", but stylistically "yes", precisely to indicate that 
we *don't* intend for it to be an ADL customization point. Consistent style 
leads to maintainability.


Repository:
  rCXX libc++

https://reviews.llvm.org/D41458



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to