Page MenuHomePhabricator

EricWF (Eric Fiselier)
User

Projects

User does not belong to any projects.

User Details

User Since
Jun 26 2014, 12:44 PM (319 w, 6 d)

Recent Activity

Today

EricWF accepted D84943: [libc++] Remove workarounds for missing rvalue references.
Wed, Aug 12, 8:40 AM · Restricted Project

Wed, Jul 15

EricWF accepted D82490: [libcxx] Put clang::trivial_abi on std::unique_ptr.

LGTM as well. Thanks for working on this!

Wed, Jul 15, 7:41 AM · Restricted Project

Jul 13 2020

EricWF accepted D82702: [libc++] Use a proper CMake target to represent libc++ headers.

IDK if I know what's causing the issue, but this change LGTM.

Jul 13 2020, 10:30 AM · Restricted Project

Jun 19 2020

EricWF accepted D82221: Add optimization to basic_string::assign for compile-time known constant values..
Jun 19 2020, 1:36 PM · Restricted Project

Jun 18 2020

EricWF accepted D78763: Add optimization to basic_string::assign for compile-time known constant values..
Jun 18 2020, 1:06 PM · Restricted Project
EricWF added a comment to D80376: [libc++] [P1208] [C++20] Adopt source location from Library Fundamentals V3 for C++20..

This is not the ABI we want.

Are you thinking of a single pointer to information stored in the data segment by the compiler?

Jun 18 2020, 12:34 PM · Restricted Project
EricWF requested changes to D80376: [libc++] [P1208] [C++20] Adopt source location from Library Fundamentals V3 for C++20..

This is not the ABI we want.

Jun 18 2020, 11:26 AM · Restricted Project
EricWF accepted D82111: Optimize 'construct at end' loops in vector.
Jun 18 2020, 10:54 AM · Restricted Project

Jun 17 2020

EricWF accepted D81770: Modifications to the algorithm sort benchmark.

LGTM

Jun 17 2020, 2:02 PM · Restricted Project

Jun 15 2020

EricWF accepted D81823: adds equality for spaceship types for themselves.
Jun 15 2020, 12:04 PM · Restricted Project
EricWF accepted D81853: [TLI] Add four C++17 delete variants..

LGTM.

Jun 15 2020, 12:04 PM · Restricted Project

Jun 9 2020

EricWF added inline comments to D80891: [libcxx] adds operator<=> for basic_string_view.
Jun 9 2020, 3:28 PM
EricWF added inline comments to D80891: [libcxx] adds operator<=> for basic_string_view.
Jun 9 2020, 11:32 AM
EricWF added a comment to D80902: [libcxx] adds lexicographical_compare_three_way.

How do we implement the "Mandates" requirement specified here?

Jun 9 2020, 10:58 AM · Restricted Project

Jun 4 2020

EricWF added a comment to D81133: Use allocator_traits to consistently allocate/deallocate/construct/destroy objects in std::any.

std::any, like other type erased types, cannot follow the allocator model and therefore does not use allocators.

Jun 4 2020, 1:34 AM · Restricted Project
EricWF added a comment to D81133: Use allocator_traits to consistently allocate/deallocate/construct/destroy objects in std::any.

All of that being said, this code is correct and LGTM**.

Jun 4 2020, 1:34 AM · Restricted Project

May 29 2020

EricWF accepted D80821: [libc++] Fix issues with the triviality of std::array.
May 29 2020, 1:06 PM · Restricted Project
EricWF requested changes to D80790: [libc++] Remove redundant empty specialization in std::array.

Why is this code an improvement?

May 29 2020, 9:45 AM · Restricted Project
EricWF requested changes to D80588: Optimize vector push_back to avoid continuous load and store of end pointer..

@mvels Do we have macro-benchmark results for this change?

May 29 2020, 8:06 AM · Restricted Project

May 13 2020

EricWF added a comment to D70343: Add a `_LIBCPP_HARDEN` define.

I'm a bit confused by the build size numbers you gave compared to the patch.

May 13 2020, 12:31 PM
EricWF added a comment to D79888: libc++: include unistd.h if available to get some preprocessor macros.

We could lift the two macros that depend on _NEWLIB_VERSION into the headers that use them.
Then we'll avoid the include in config.

May 13 2020, 12:31 PM · Restricted Project
EricWF requested changes to D79888: libc++: include unistd.h if available to get some preprocessor macros.
May 13 2020, 11:25 AM · Restricted Project
EricWF requested changes to D70343: Add a `_LIBCPP_HARDEN` define.
May 13 2020, 10:50 AM

May 7 2020

EricWF accepted D79323: Remove unused _LIBCPP_RAW_ITERATORS.

LGTM.
I can't find any usages of this internally.

May 7 2020, 1:02 PM · Restricted Project
EricWF added a comment to D78200: [libc++] [test] Generate static_test_env on the fly.

Sorry for jumping in without context, but wanted to chime in on one thing.

Windows doesn't really have a concept of symlinks. So, when the monorepo is cloned, those symlinks turn to ordinary text files. Previously, if we cross-compiled libc++ for some symlink-friendly system (e. g. Linux) and ran tests on the target system, some tests would fail.

This isn't right. Windows has supported symbolic links since Vista. The big limitation was that you needed to be in an elevated process to be able to create symbolic links. Windows 10 lifted that limitation in Developer Mode starting with the April 2017 release (https://blogs.windows.com/windowsdeveloper/2016/12/02/symlinks-windows-10/). If git for Windows isn't creating symbolic links properly, that's a git issue and not a Windows issue (although I recognize it doesn't make much difference for the end user). Some searching suggests that you should be able to configure Git on Windows to handle symlinks correctly (https://github.com/libgit2/libgit2/pull/4713, and https://github.community/t5/How-to-use-Git-and-GitHub/git-bash-symbolic-links-on-windows/m-p/11488#M3732 shows an option to enable symbolic links when running the setup).

I am aware of this, and I even had some success turning on support for symlinks on Windows. Which gives me the right to say that this process requires a little more involvement than it should :-)

However, things are even more complicated on the buildbot (which is the main purpose of this effort). Namely, there are some limitations of the buildbot platform that won't allow passing custom flags to Git when cloning the repo.

May 7 2020, 1:02 PM · Restricted Project

May 6 2020

EricWF accepted D72640: [libcxx] Qualify make_pair in searcher implementations to prevent ADL.

LGTM.

May 6 2020, 1:35 PM · Restricted Project
EricWF added inline comments to D78200: [libc++] [test] Generate static_test_env on the fly.
May 6 2020, 1:00 PM · Restricted Project
EricWF requested changes to D78200: [libc++] [test] Generate static_test_env on the fly.

If we don't want the static env, we should remove it entirely.
Meaning, we should convert each of these tests to use dynamic_test_env and setup the required files like the remainder of the tests do.

May 6 2020, 1:00 PM · Restricted Project
EricWF added a comment to D78200: [libc++] [test] Generate static_test_env on the fly.

I'm sorry, I've messed up. I first committed this as rG645ad5badbabdeca31de5c98ea8135c5a6e7d710, but then realized that I've committed the previous version of this patch. So I reverted it and recommitted as 52cc8bac7780dbfb90dcc8cfe24696618eeaa06e.

That said, I don't understand how this patch makes these tests pass on Windows.
If Windows doesn't support symlinks, how does dynamically creating them make a difference?

This patch is not about making them pass on Windows, but rather about making them pass on Linux in the case when we cross-compile them on Windows.

May 6 2020, 1:00 PM · Restricted Project
EricWF added a comment to D79427: [libcxx] Explicitly mark erroneous string_view ctors as deleted.

While I support this change, it should really be accompanied by a library working group issue or paper.

May 6 2020, 10:46 AM

May 5 2020

EricWF added a comment to D78200: [libc++] [test] Generate static_test_env on the fly.

Unless I'm mistaken, this change is racy. Tests in different files are run in parallel.
One test could be tearing down the static environment while another in trying to construct it.

May 5 2020, 7:26 PM · Restricted Project
EricWF reopened D78200: [libc++] [test] Generate static_test_env on the fly.

When I initially wrote the filesystem tests, I had divided them between "static" and "dynamic"; that is, tests which modified the target filesystem and those which did not.

May 5 2020, 6:54 PM · Restricted Project
EricWF requested changes to D72640: [libcxx] Qualify make_pair in searcher implementations to prevent ADL.

There are some simplifications suggested in the inline comments.
Once those are addressed, this LGTM.

May 5 2020, 6:22 PM · Restricted Project

Apr 23 2020

EricWF added inline comments to D78381: [libc++] Create a small DSL for defining Lit features and parameters.
Apr 23 2020, 1:00 PM · Restricted Project

Apr 20 2020

EricWF accepted D78390: [dfsan] Add "DataFlow" option to LLVM_USE_SANITIZER.
Apr 20 2020, 10:16 AM · Restricted Project, Restricted Project

Apr 16 2020

EricWF committed rGaf2968e37f4c: [clang] Fix invalid comparator in tablegen (authored by EricWF).
[clang] Fix invalid comparator in tablegen
Apr 16 2020, 3:40 PM
EricWF closed D78323: [clang] Fix invalid comparator in tablegen.

af2968e37f4c95846ffe287b64a4fcd72c765bee

Apr 16 2020, 3:39 PM · Restricted Project
EricWF added a comment to D78323: [clang] Fix invalid comparator in tablegen.

LGTM, did this comparison introduce any non-determinism?

Apr 16 2020, 3:37 PM · Restricted Project
EricWF created D78323: [clang] Fix invalid comparator in tablegen.
Apr 16 2020, 1:22 PM · Restricted Project
EricWF added a comment to D78200: [libc++] [test] Generate static_test_env on the fly.

The entire point of the static test environment is that it's /static/.
Meaning it's checked in and not created on the fly.

Apr 16 2020, 11:08 AM · Restricted Project
EricWF added a comment to D78245: [LIT] Make `%T` unique for every test.

Are you aware of https://llvm.org/docs/CommandGuide/lit.html#substitutions ("%T: parent directory of %t (not unique, deprecated, do not use)")? We've talked about this a bunch in the past, and the decision back then was to do mkdir %t in the tests that need a dir, and to remove %T over time.

Apr 16 2020, 9:27 AM · Restricted Project, Restricted Project, Restricted Project
EricWF requested changes to D77505: [libc++] Implement <numbers>.

This needs a bunch of additional tests. Specifically of the passing kind :-)

Apr 16 2020, 9:27 AM · Restricted Project

Apr 15 2020

EricWF added inline comments to D78245: [LIT] Make `%T` unique for every test.
Apr 15 2020, 2:55 PM · Restricted Project, Restricted Project, Restricted Project
EricWF created D78245: [LIT] Make `%T` unique for every test.
Apr 15 2020, 2:55 PM · Restricted Project, Restricted Project, Restricted Project
EricWF created D78223: [clang-tidy] performance-range-for-copy only for copy..
Apr 15 2020, 11:30 AM · Restricted Project

Apr 10 2020

EricWF accepted D77913: Make basic_string::operator=() tail call properly.
Apr 10 2020, 4:09 PM · Restricted Project

Apr 9 2020

EricWF committed rGc6eb584c6487: [libc++] Fix recursive instantiation in std::array. (authored by EricWF).
[libc++] Fix recursive instantiation in std::array.
Apr 9 2020, 2:43 PM

Apr 8 2020

EricWF committed rGbf90b8fc25ca: [libc++] Fix failing concepts tests (authored by EricWF).
[libc++] Fix failing concepts tests
Apr 8 2020, 3:47 PM
EricWF committed rG601f7631827a: [libcxx] Adds [concept.same] (authored by EricWF).
[libcxx] Adds [concept.same]
Apr 8 2020, 3:14 PM
EricWF added a comment to D74350: [libcxx][type_traits] Add C++20 changes to common_type.

I assume this is a behavioral change? Why are there no new tests?

Apr 8 2020, 3:13 PM · Restricted Project
EricWF closed D74291: [libcxx] Adds [concept.same].

Committed as 601f7631827ae6ac08117a282c83a62b67dedf48

Apr 8 2020, 3:13 PM

Apr 7 2020

EricWF accepted D77681: Reset more globalMemCounters..
Apr 7 2020, 4:22 PM · Restricted Project
EricWF accepted D76093: Don't expose unavailable cstdio functions..
Apr 7 2020, 3:16 PM · Restricted Project
EricWF accepted D74291: [libcxx] Adds [concept.same].

I have some concerns with how we harden our concepts,
but these can be addressed in follow up commits.

Apr 7 2020, 10:50 AM

Apr 4 2020

EricWF committed rG62f3a9650a9f: [libc++] Attempt to workaround module invalidation bug (authored by EricWF).
[libc++] Attempt to workaround module invalidation bug
Apr 4 2020, 12:19 AM

Apr 3 2020

EricWF accepted D77338: [libc++] Add an alternative Lit test format.

I like tests.
Assuming all of them pass;: LGTM.

Apr 3 2020, 2:07 AM · Restricted Project, Restricted Project

Mar 31 2020

EricWF accepted D76731: [libc++] Do not rely on the environment to run filesystem tests.
Mar 31 2020, 1:04 AM · Restricted Project

Mar 26 2020

EricWF added inline comments to D76311: [libc++] Do not force the use of -Werror in verify tests.
Mar 26 2020, 6:30 PM · Restricted Project
EricWF added a comment to D76798: [libc++] Make sure that temp_directory_path() doesn't return a path with a trailing slash.

I don't necessarily agree.

Mar 26 2020, 9:11 AM · Restricted Project
EricWF added a comment to D76798: [libc++] Make sure that temp_directory_path() doesn't return a path with a trailing slash.

Lets not start writing tests like this.

Mar 26 2020, 8:38 AM · Restricted Project
EricWF added inline comments to D76731: [libc++] Do not rely on the environment to run filesystem tests.
Mar 26 2020, 8:06 AM · Restricted Project
EricWF accepted D76785: [libc++] Set filesystem test flags in a lit.local.cfg.
Mar 26 2020, 8:06 AM · Restricted Project
EricWF added a reverting change for rGa32b94c6c3a1: [libc++] Run the builders Docker containers 'as 'buildbot instead of 'root': rG076773253ebf: Revert "[libc++] Run the builders Docker containers 'as 'buildbot instead of….
Mar 26 2020, 5:23 AM
EricWF committed rG076773253ebf: Revert "[libc++] Run the builders Docker containers 'as 'buildbot instead of… (authored by EricWF).
Revert "[libc++] Run the builders Docker containers 'as 'buildbot instead of…
Mar 26 2020, 5:23 AM

Mar 24 2020

EricWF committed rGd6fb02b196da: [libc++] Update a bad documentation link (authored by EricWF).
[libc++] Update a bad documentation link
Mar 24 2020, 6:37 PM
EricWF requested changes to D76636: [RFC] Added type trait to remove address space qualifier from type.

We don't implement non-standard extensions in libc++.
Only when this trait is proposed for standardization can we consider adding it to the library

Mar 24 2020, 1:28 PM

Mar 23 2020

EricWF accepted D76618: [libc++] Bump Clang support for Clang 4.

I would like us to be even more aggressive bumping our compiler support,
but that's a conversation for another time.

Mar 23 2020, 9:14 AM · Restricted Project
EricWF accepted D76256: [libc++] Require the use of clang-verify in .fail.cpp tests that don't fail without it.

These tests still require that "arbitrary" warning flag be enabled.

I don't understand? The only reason these tests were using that flag was to cause a compilation error (instead of a warning) on GCC. If we properly require that we're able to check the diagnostics, which is the goal, we can stop trying to make them fail arbitrarily on GCC.

Mar 23 2020, 8:09 AM · Restricted Project

Mar 21 2020

EricWF accepted D76104: Remove legacy CMake targets for libcxx and libcxxabi.
Mar 21 2020, 5:06 PM · Restricted Project, Restricted Project
EricWF added a comment to D76431: Introduce LIBUNWIND_LIT_ARGS, LIBCXXABI_LIT_ARGS and LIBCXX_LIT_AGRS.

Why do we need separate options for each project?
I think that's the wrong direction.

Mar 21 2020, 5:06 PM · Restricted Project, Restricted Project, Restricted Project
EricWF committed rG90c74435d366: [libc++] tolerate missing diagnostic with modules enabled (authored by EricWF).
[libc++] tolerate missing diagnostic with modules enabled
Mar 21 2020, 1:52 PM
EricWF committed rGc0e1135fb087: [libc++] Fix URL to llvm github (authored by EricWF).
[libc++] Fix URL to llvm github
Mar 21 2020, 7:30 AM
EricWF committed rGdeb510337840: [libc++] Rework buildbot configuration for the greater good. (authored by EricWF).
[libc++] Rework buildbot configuration for the greater good.
Mar 21 2020, 7:30 AM
EricWF committed rG05880fc9ae78: [libc++] fix some non-modular tests (authored by EricWF).
[libc++] fix some non-modular tests
Mar 21 2020, 7:30 AM

Mar 18 2020

EricWF added a comment to D76256: [libc++] Require the use of clang-verify in .fail.cpp tests that don't fail without it.

These tests still require that "arbitrary" warning flag be enabled.

Mar 18 2020, 8:04 PM · Restricted Project
EricWF accepted D76311: [libc++] Do not force the use of -Werror in verify tests.

I'm not sure I love this, but I'm OK demoting warnings to errors in the XFAIL tests.

Mar 18 2020, 8:04 PM · Restricted Project

Mar 16 2020

EricWF accepted D76150: Fix -Wdeprecated-copy-dtor and -Wdeprecated-dynamic-exception-spec warnings..
In D76150#1922104, @dim wrote:

Note: one thing that I am unsure about is the _NOEXCEPT after some of the copy constructors. I only applied those to classes that have other _NOEXCEPT constructors, but copying may involve other things that could throw.

Maybe it is safer to not use _NOEXCEPT at all for all of them, instead?

Mar 16 2020, 6:03 PM · Restricted Project

Mar 14 2020

EricWF removed a reviewer for D76177: Add support for unwrapping all contiguous iterators: Restricted Project.
Mar 14 2020, 10:43 AM
EricWF created D76177: Add support for unwrapping all contiguous iterators.
Mar 14 2020, 8:33 AM

Mar 13 2020

EricWF added a comment to D76093: Don't expose unavailable cstdio functions..

Could we add a .fail.cpp test that ensures when _LIBCPP_HAS_NO_FGETPOS is defined we don't actually have it?

Mar 13 2020, 1:29 PM · Restricted Project
EricWF reopened D76091: Move more tests to globalMemCounter and reset..

Some of the changes in this patch are not correct. Please revert them in a follow up commit.

Mar 13 2020, 1:29 PM · Restricted Project

Mar 11 2020

EricWF requested changes to D75905: [libc++][P1115][C++20] Improving the Return Value of Erase-Like Algorithms II: Free erase/erase if..

I think there is a bug in the paper, but I think the return type should be difference_type not size_type.
The paper says the return value is equivalent to:

Mar 11 2020, 8:23 PM · Restricted Project
EricWF requested changes to D75960: [libc++] Implement C++20's P0476r2: std::bit_cast.

LGTM other than the lack of tests.

Mar 11 2020, 8:21 PM · Restricted Project
EricWF requested changes to D75622: [FIX][libc++][Regex] Using regex_constants match_prev_avail | match_not_bol | match_not_bow.
Mar 11 2020, 8:21 PM · Restricted Project
EricWF requested changes to D71994: SFINAE span default constructor on Extent == 0.

Does this really need concepts?

Mar 11 2020, 8:21 PM · Restricted Project
EricWF requested changes to D74997: [libc++] Bugfix to std::binomial_distribution<int>.

Could you please upload this diff with more context?
(https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface)

Mar 11 2020, 8:21 PM · Restricted Project
EricWF requested changes to D75795: [libc++abi] Change __cxa_finalize return type to void.

Sorry about this -- I'll try to be more careful in the future about getting my changes vetted.

@rprichard has no commits to libc++/libc++abi

Pedantic: I do have a couple libcxxabi patches, but @EricWF committed them for me (D36446 and D36447).

Why are we declaring this function at all?

I don't know. __cxa_finalize and __cxa_atexit are part of the IA-64 C++ ABI, so I suppose the notice at the top of the file applies. They're implemented elsewhere, though, not in libcxxabi.

/*
 * This header provides the interface to the C++ ABI as defined at:
 *       https://itanium-cxx-abi.github.io/cxx-abi/
 */
Mar 11 2020, 8:21 PM · Restricted Project, Restricted Project, Restricted Project
EricWF accepted D75950: [libc++abi] NFC: Move AtomicInt to cxa_guard_impl.h.

Is this just a clean move, or is the class definition changed as well?
I can't tell with all the linter warnings in the way.

Mar 11 2020, 3:56 PM · Restricted Project, Restricted Project, Restricted Project
EricWF accepted D75950: [libc++abi] NFC: Move AtomicInt to cxa_guard_impl.h.
Mar 11 2020, 3:56 PM · Restricted Project, Restricted Project, Restricted Project
EricWF accepted D71894: [libcxxabi] Allow tests to link with static libc++abi/libc++ even if the shared version is present.

Your good to commit.

Mar 11 2020, 3:56 PM · Restricted Project, Restricted Project, Restricted Project
EricWF requested changes to D72282: [clang-tidy] Add `bugprone-unintended-adl`.
  • This check should suggest fixes. It knows what function is actually being resolved, and it has enough information to create a minimally qualified name that resolves it.
  • This check should ignore hidden friends, since their entire purpose is to be called via ADL.
  • This check should allow whitelisting namespaces that opt-out of ADL into their namespace. This makes it much easier to roll out the clang-tidy incrementally.
Mar 11 2020, 3:56 PM · Restricted Project, Restricted Project
EricWF added a comment to D72282: [clang-tidy] Add `bugprone-unintended-adl`.

I'll be picking this up seriously this week.
I'm currently getting an internal version of this clang-tidy reviewed,
and my plan was to submit it upstream after because I didn't see this
patch until now (bad email filters).

Mar 11 2020, 3:20 PM · Restricted Project, Restricted Project
EricWF requested changes to D73245: Avoid using std::max_align_t in pre-C++11 mode.

I've already stated my disapproval of this patch. Libc++ has never and will never provide nor value C++03 conformance.
Moving backwards to C++03 is inconsistent with the libraries general direction.

Mar 11 2020, 3:19 PM · Restricted Project
EricWF requested changes to D62778: [1/2] Add a benchmark for map operations..
Mar 11 2020, 2:41 PM · Restricted Project
EricWF accepted D75955: [libcxx] Enable C++17 for the benchmarks.
Mar 11 2020, 2:41 PM · Restricted Project
EricWF abandoned D70336: Explicitly enumerate std::string external instantiations..
Mar 11 2020, 2:41 PM
EricWF requested changes to D75795: [libc++abi] Change __cxa_finalize return type to void.

Why are we declaring this function at all?

Mar 11 2020, 2:41 PM · Restricted Project, Restricted Project, Restricted Project
EricWF accepted D76027: [libc++abi] NFC: Simplify extern C declaration.

Why was this file ever getting built as plain C anyway?
All of the libc++abi source files that use it should be C++.

Mar 11 2020, 2:41 PM · Restricted Project, Restricted Project
EricWF accepted D76022: [libc++] Add SHA for C++20 Synchronization Library in ABI changelog.
Mar 11 2020, 1:35 PM · Restricted Project

Mar 9 2020

EricWF abandoned D64298: Make ~mutex and ~condition_variable trivial with Apple pthreads.

Abandoning. We went in another direction.

Mar 9 2020, 11:52 AM