lichray marked 2 inline comments as done. lichray 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) + { ---------------- Quuxplusone wrote: > 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. `read` is a function decl in `<io.h>`, not redefined in other forms in `<windows.h>`. ================ Comment at: test/support/charconv_test_helpers.h:40 +constexpr bool +_fits_in(T, true_type /* non-narrowing*/, ...) +{ ---------------- mclow.lists wrote: > We don't need to use ugly names here in the test suite. > Err, it's not? Just an impl version of `fits_in` (without the _ prefix). 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