vsapsai (Volodymyr Sapsai)
User

Projects

User does not belong to any projects.

User Details

User Since
May 27 2014, 6:39 AM (216 w, 6 d)

Recent Activity

Wed, Jul 18

vsapsai added a comment to D49518: [VFS] Emit an error when a file isn't located in any directory..

Need to double check what tests we have when using relative path names at the root level. I'd like to make the behavior consistent because a file name is a specific case of relative paths. So far there are no assertions and no errors but file lookup doesn't seem to be working.

Wed, Jul 18, 6:59 PM
vsapsai added inline comments to D49518: [VFS] Emit an error when a file isn't located in any directory..
Wed, Jul 18, 4:27 PM
vsapsai created D49518: [VFS] Emit an error when a file isn't located in any directory..
Wed, Jul 18, 4:26 PM
vsapsai added inline comments to D48786: [Preprocessor] Stop entering included files after hitting a fatal error..
Wed, Jul 18, 4:14 PM

Tue, Jul 17

vsapsai added inline comments to D49317: Move __construct_forward (etc.) out of std::allocator_traits..
Tue, Jul 17, 11:18 AM

Mon, Jul 16

vsapsai added a comment to D49317: Move __construct_forward (etc.) out of std::allocator_traits..

My review is incomplete, especially I cannot say with confidence if the proposed change is entirely free from unintended consequences that might break code not covered by the test suite. So other reviewers are welcome to chime in.

Mon, Jul 16, 5:26 PM
vsapsai added a comment to D48786: [Preprocessor] Stop entering included files after hitting a fatal error..

Ping.

Mon, Jul 16, 4:01 PM
vsapsai added a comment to D49119: [Sema][ObjC] Issue a warning when a method declared in a protocol is non-escaping but the corresponding method in the implementation is escaping..

Overall looks good to me. Maybe add a test when a protocol is declared for an interface, not for a category. Something like

Mon, Jul 16, 2:57 PM · Restricted Project

Tue, Jul 10

vsapsai updated the diff for D48753: [libcxx] Use custom allocator's `construct` in C++03 when available..
  • In C++03 call allocator's destroy when available.
  • Rename _Args as it's not a variadic template pack.
Tue, Jul 10, 6:02 PM
vsapsai added a comment to D48753: [libcxx] Use custom allocator's `construct` in C++03 when available..

Why are we doing this?

I can't find the language in the C++03 specification that requires us to call an allocators construct method if it's present.

I think it's being proposed under "quality of implementation."

It has only just now occurred to me: a quality implementation should probably never mismatch calls to construct and destroy. Does libc++ currently call destroy in C++03? Does it call destroy in C++03 after this patch? If this patch is making libc++ call construct but not destroy, that's a bug. If before this patch libc++ would call destroy but not construct, this patch is a bugfix! (And please add a test case for matched pairs of construct and destroy.)

Tue, Jul 10, 3:47 PM
vsapsai added a comment to D48753: [libcxx] Use custom allocator's `construct` in C++03 when available..

Why are we doing this?

I can't find the language in the C++03 specification that requires us to call an allocators construct method if it's present.

Tue, Jul 10, 1:51 PM

Mon, Jul 9

vsapsai added inline comments to D48753: [libcxx] Use custom allocator's `construct` in C++03 when available..
Mon, Jul 9, 12:49 PM
vsapsai added a comment to D48786: [Preprocessor] Stop entering included files after hitting a fatal error..

Ping.

Mon, Jul 9, 12:41 PM
vsapsai updated the diff for D48753: [libcxx] Use custom allocator's `construct` in C++03 when available..
  • Allow allocator construct to return a value, not just have return type void.
Mon, Jul 9, 12:40 PM

Fri, Jul 6

vsapsai added inline comments to D48753: [libcxx] Use custom allocator's `construct` in C++03 when available..
Fri, Jul 6, 2:45 PM
vsapsai added a comment to D47687: [Sema] Missing -Wlogical-op-parentheses warnings in macros (PR18971).

Sorry, It seems a little bit difficult for me to add a proper fix-it hint for expressions in macros, because I cannot find the exact position of the last char of the token on right hand side of operator. Any suggestion?

Fri, Jul 6, 1:17 PM

Thu, Jul 5

vsapsai updated the diff for D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch..
  • Clean up tests according to review. We don't need a new test for custom allocators, parent patch covers that.
Thu, Jul 5, 3:06 PM
vsapsai updated the diff for D48753: [libcxx] Use custom allocator's `construct` in C++03 when available..
  • Use a better way to detect presence of construct with required signature. Clean up tests.
Thu, Jul 5, 3:03 PM

Wed, Jul 4

vsapsai added inline comments to D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch..
Wed, Jul 4, 5:27 PM
vsapsai added a comment to D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch..

Are there any tests which actually exercise the new behavior?

Wed, Jul 4, 4:45 PM
vsapsai added inline comments to D48753: [libcxx] Use custom allocator's `construct` in C++03 when available..
Wed, Jul 4, 4:16 PM

Tue, Jul 3

vsapsai added inline comments to D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch..
Tue, Jul 3, 5:06 PM
vsapsai updated the diff for D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch..
  • Proper support for custom allocators without construct.
Tue, Jul 3, 4:53 PM
vsapsai updated the diff for D48753: [libcxx] Use custom allocator's `construct` in C++03 when available..
  • Clean up functionality not required by the Standard.
Tue, Jul 3, 4:42 PM

Mon, Jul 2

vsapsai planned changes to D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch..
Mon, Jul 2, 12:21 PM
vsapsai added a comment to D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch..

I want to point out that this code (not -necessarily- this patch, but where it lives) needs to be rewritten.

There is no prohibition on users specializing allocator_traits for their allocators, and yet libc++'s vector depends on the existence of allocator_traits<Allocator>::__construct_range_forward (among other things).

Mon, Jul 2, 12:11 PM

Fri, Jun 29

vsapsai created D48786: [Preprocessor] Stop entering included files after hitting a fatal error..
Fri, Jun 29, 12:13 PM

Thu, Jun 28

vsapsai added a comment to D48753: [libcxx] Use custom allocator's `construct` in C++03 when available..

I'm pretty sure that the C++03 standard doesn't permit the implementation to call any construct method here, even if it wanted to. And this adds a couple hundred LOC to get that (non-standard) behavior. Is there a specific use-case that you're trying to enable by adding this behavior to libc++?

Thu, Jun 28, 6:18 PM
vsapsai added inline comments to D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch..
Thu, Jun 28, 5:40 PM
vsapsai updated the diff for D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch..

Try to exclude changes available in parent review from this review.

Thu, Jun 28, 5:30 PM
vsapsai added a dependent revision for D48753: [libcxx] Use custom allocator's `construct` in C++03 when available.: D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch..
Thu, Jun 28, 5:25 PM
vsapsai added a dependency for D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.: D48753: [libcxx] Use custom allocator's `construct` in C++03 when available..
Thu, Jun 28, 5:25 PM
vsapsai updated the diff for D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch..
  • Don't check !__has_construct for __construct_range_forward.
Thu, Jun 28, 5:24 PM
vsapsai created D48753: [libcxx] Use custom allocator's `construct` in C++03 when available..
Thu, Jun 28, 5:21 PM

Wed, Jun 27

vsapsai added inline comments to D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch..
Wed, Jun 27, 11:30 AM

Tue, Jun 26

vsapsai updated the diff for D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch..
  • Don't use memcpy specialization with custom allocators.
Tue, Jun 26, 4:45 PM
vsapsai planned changes to D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch..
Tue, Jun 26, 11:48 AM
vsapsai added a comment to D47341: [Sema] Fix infinite typo correction loop..

Thanks for the review, Richard.

Tue, Jun 26, 11:02 AM
vsapsai committed rL335638: [Sema] Fix infinite typo correction loop..
[Sema] Fix infinite typo correction loop.
Tue, Jun 26, 11:01 AM
vsapsai committed rC335638: [Sema] Fix infinite typo correction loop..
[Sema] Fix infinite typo correction loop.
Tue, Jun 26, 11:01 AM
vsapsai closed D47341: [Sema] Fix infinite typo correction loop..
Tue, Jun 26, 11:01 AM

Mon, Jun 25

vsapsai added a comment to D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch..

Ping.

Mon, Jun 25, 3:45 PM
vsapsai added a comment to D47341: [Sema] Fix infinite typo correction loop..

Ping.

Mon, Jun 25, 3:44 PM

Jun 22 2018

vsapsai accepted D47301: Warning for framework include violation from Headers to PrivateHeaders.

Looks good to me. The only problem is I'm not entirely sure this warning will interact with real code the way I expect it to. We'll need to keep an eye on it and tweak if necessary.

Jun 22 2018, 3:22 PM

Jun 19 2018

vsapsai added a comment to D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch..

Related change is https://reviews.llvm.org/D8109 For me the performance improvement was a twice faster execution on a dirty benchmark that doesn't exclude set up and tear down.

Jun 19 2018, 5:33 PM
vsapsai created D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch..
Jun 19 2018, 5:29 PM

Jun 18 2018

vsapsai added a comment to D47341: [Sema] Fix infinite typo correction loop..

Ping.

Jun 18 2018, 3:23 PM
vsapsai added inline comments to D48297: [Darwin] Add a warning for missing include path for libstdc++.
Jun 18 2018, 2:59 PM

Jun 11 2018

vsapsai added a comment to D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable..

With this change and the mentioned libc++ change the tests with old libc++ dylib are passing (didn't test all possible configurations though). Would like to get more feedback from other reviewers on this matter.

Jun 11 2018, 2:21 PM
vsapsai committed rCXX334431: Mark the test using <experimental/memory_resource> to require c++experimental..
Mark the test using <experimental/memory_resource> to require c++experimental.
Jun 11 2018, 1:01 PM
vsapsai committed rL334431: Mark the test using <experimental/memory_resource> to require c++experimental..
Mark the test using <experimental/memory_resource> to require c++experimental.
Jun 11 2018, 12:48 PM
vsapsai added a comment to D47341: [Sema] Fix infinite typo correction loop..

Ping.

Jun 11 2018, 9:58 AM
vsapsai retitled D47341: [Sema] Fix infinite typo correction loop. from [Sema] Disable creating new delayed typos while correcting existing. to [Sema] Fix infinite typo correction loop..
Jun 11 2018, 9:58 AM

Jun 8 2018

vsapsai added a comment to D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable..

Could you elaborate on what kind of changes you are planning to make in libc++ after committing this patch?

Libc++ shouldn't actually need any changes if this current patch lands. Currently libc++ is in a "incorrect" state where
it generates calls to __builtin_operator_new(size_t, align_val_t) when __cpp_aligned_new is defined but when aligned new/delete
are actually unavailable.

If we change __cpp_aligned_new to no longer be defined when aligned new is unavailable, then libc++ will start doing the right thing.
See r328180 for the relevent commits which made these libc++ changes.

Jun 8 2018, 4:51 PM
vsapsai added a comment to D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable..

Sorry for the churn but can you please take out -nostdinc++ part out of this change? After more thinking and discussion we think there is a chance developers can use -nostdinc++ not only for building the standard library. -nostdinc++ is a signal of building the standard library but it's not strong enough. Most likely developers will be unaware how -nostdinc++ affects aligned allocations and that can lead to linker failures at runtime. So if you deploy on older macOS versions it is safer to assume aligned allocation isn't available. But if you provide your own libc++ with aligned allocations, you can still use -faligned-allocation to make that work. For now we prefer to use that approach and if it proves to be too onerous for developers, we can reintroduce -nostdinc++ logic.

Jun 8 2018, 4:47 PM
vsapsai added inline comments to D47301: Warning for framework include violation from Headers to PrivateHeaders.
Jun 8 2018, 3:25 PM

May 30 2018

vsapsai added inline comments to D47557: Filesystem tests: un-confuse write time.
May 30 2018, 5:04 PM
vsapsai added inline comments to D47557: Filesystem tests: un-confuse write time.
May 30 2018, 4:46 PM

May 28 2018

vsapsai added a comment to D47341: [Sema] Fix infinite typo correction loop..

Some debugging details that can be useful but too specific for the commit message. When we have infinite loop, the steps are:

May 28 2018, 6:48 PM
vsapsai updated the diff for D47341: [Sema] Fix infinite typo correction loop..
  • Fix only infinite loop and don't touch the assertion failure.
May 28 2018, 6:22 PM
vsapsai accepted D44568: Fix emission of phony dependency targets when adding extra deps.

Looks good to me. Just watch the build bots in case some of them are strict with warnings and require (void)AddFilename(Filename).

May 28 2018, 1:11 PM

May 25 2018

vsapsai planned changes to D47341: [Sema] Fix infinite typo correction loop..

After looking into this more, I think there are 2 different bugs: one with infinite loop and another with DelayedTypos.empty() && "Uncorrected typos!" assertion. And disabling typo correction happened to fix both of them.

May 25 2018, 7:20 PM

May 24 2018

vsapsai added inline comments to D44568: Fix emission of phony dependency targets when adding extra deps.
May 24 2018, 3:28 PM
vsapsai added inline comments to D47301: Warning for framework include violation from Headers to PrivateHeaders.
May 24 2018, 12:42 PM
vsapsai created D47341: [Sema] Fix infinite typo correction loop..
May 24 2018, 11:07 AM

May 23 2018

vsapsai added a comment to D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable..

Somewhat tangential, in discussion with Duncan he mentioned that -nostdinc++ should turn off assumptions about old Darwin. So if you build libc++ yourself, you don't care what does the system stdlib support. I agree with that and think it doesn't interfere with the latest proposal but adds more to it.

May 23 2018, 11:05 AM

May 22 2018

vsapsai committed rL333011: [libcxx] [test] Mark the test as unsupported by apple-clang-8.1..
[libcxx] [test] Mark the test as unsupported by apple-clang-8.1.
May 22 2018, 11:50 AM
vsapsai committed rCXX333011: [libcxx] [test] Mark the test as unsupported by apple-clang-8.1..
[libcxx] [test] Mark the test as unsupported by apple-clang-8.1.
May 22 2018, 11:50 AM

May 18 2018

vsapsai added a comment to D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable..

That is: on old Darwin, we should not define __cpp_aligned_allocation (even in C++17), produce the "no aligned allocation support" warning in C++17 mode, and then not try to call the aligned allocation function. But if -faligned-allocation or -fno-aligned-allocation is specified explicitly, then the user knows what they're doing and they get no warning.

May 18 2018, 4:47 PM

May 17 2018

vsapsai added a comment to D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable..

Eric, do you have more thoughts on this issue?

May 17 2018, 5:03 PM

May 16 2018

vsapsai committed rC332509: [Sema] Fix assertion when constructor is disabled with partially specialized….
[Sema] Fix assertion when constructor is disabled with partially specialized…
May 16 2018, 11:32 AM
vsapsai committed rL332509: [Sema] Fix assertion when constructor is disabled with partially specialized….
[Sema] Fix assertion when constructor is disabled with partially specialized…
May 16 2018, 11:32 AM
vsapsai closed D46909: [Sema] Fix assertion when constructor is disabled with partially specialized template..
May 16 2018, 11:32 AM
vsapsai added a comment to D46909: [Sema] Fix assertion when constructor is disabled with partially specialized template..

Thanks for the quick review, Richard. I'll keep in mind the idea with comparing results of multiple lookups when I work on other partial specialization-related bugs.

May 16 2018, 11:30 AM

May 15 2018

vsapsai committed rCXX332413: Emit an error when include <atomic> after <stdatomic.h>.
Emit an error when include <atomic> after <stdatomic.h>
May 15 2018, 3:43 PM
vsapsai added a comment to D45470: Emit an error when include <atomic> after <stdatomic.h>.

Thanks for review.

May 15 2018, 3:42 PM
vsapsai committed rL332413: Emit an error when include <atomic> after <stdatomic.h>.
Emit an error when include <atomic> after <stdatomic.h>
May 15 2018, 3:42 PM
vsapsai closed D45470: Emit an error when include <atomic> after <stdatomic.h>.
May 15 2018, 3:42 PM
vsapsai updated the diff for D46909: [Sema] Fix assertion when constructor is disabled with partially specialized template..

Potentially, VarTemplateDecl is susceptible to the same bug as
ClassTemplateSpecializationDecl. But I wasn't able to trigger it. And based on
code I've convinced myself that the mentioned early exit in
Sema::CheckVarTemplateId is equivalent to reusing available partial
specialization. So seems like no changes for VarTemplateDecl are required.

May 15 2018, 3:39 PM
vsapsai added inline comments to D46909: [Sema] Fix assertion when constructor is disabled with partially specialized template..
May 15 2018, 3:34 PM
vsapsai created D46909: [Sema] Fix assertion when constructor is disabled with partially specialized template..
May 15 2018, 3:31 PM

May 14 2018

vsapsai added a comment to D46446: [c++17] Fix assertion on synthesizing deduction guides after a fatal error..

Thanks for the review.

May 14 2018, 3:57 PM
vsapsai committed rC332307: [c++17] Fix assertion on synthesizing deduction guides after a fatal error..
[c++17] Fix assertion on synthesizing deduction guides after a fatal error.
May 14 2018, 3:53 PM
vsapsai committed rL332307: [c++17] Fix assertion on synthesizing deduction guides after a fatal error..
[c++17] Fix assertion on synthesizing deduction guides after a fatal error.
May 14 2018, 3:53 PM
vsapsai closed D46446: [c++17] Fix assertion on synthesizing deduction guides after a fatal error..
May 14 2018, 3:53 PM
vsapsai added a comment to D46446: [c++17] Fix assertion on synthesizing deduction guides after a fatal error..

Ping.

May 14 2018, 2:29 PM
vsapsai committed rCXX332282: Update XFAIL so apple-clang-9.0 is the last version not implementing Core 2094..
Update XFAIL so apple-clang-9.0 is the last version not implementing Core 2094.
May 14 2018, 12:51 PM
vsapsai committed rL332282: Update XFAIL so apple-clang-9.0 is the last version not implementing Core 2094..
Update XFAIL so apple-clang-9.0 is the last version not implementing Core 2094.
May 14 2018, 12:49 PM

May 9 2018

vsapsai added a comment to D45470: Emit an error when include <atomic> after <stdatomic.h>.
In D45470#1092260, @jfb wrote:

Here is another approach that should emit an error only when mixing headers
causes compilation problems.

Have no ideas how to test the change. -verify doesn't work with fatal errors
and libcxx doesn't use FileCheck. Performed only manual testing.

This worked with <atomic> before <stdatomic.h> as well as with the order reversed?

May 9 2018, 11:51 AM

May 8 2018

vsapsai retitled D45470: Emit an error when include <atomic> after <stdatomic.h> from Emit an error when mixing <stdatomic.h> and <atomic> to Emit an error when include <atomic> after <stdatomic.h>.
May 8 2018, 5:09 PM
vsapsai requested review of D45470: Emit an error when include <atomic> after <stdatomic.h>.
May 8 2018, 5:08 PM
vsapsai updated the diff for D45470: Emit an error when include <atomic> after <stdatomic.h>.

Here is another approach that should emit an error only when mixing headers
causes compilation problems.

May 8 2018, 5:07 PM
vsapsai reopened D45470: Emit an error when include <atomic> after <stdatomic.h>.

__ALLOW_STDC_ATOMICS_IN_CXX__ approach didn't work in practice, I've reverted all changes.

May 8 2018, 5:07 PM
vsapsai committed rCXX331818: Revert "Emit an error when mixing <stdatomic.h> and <atomic>".
Revert "Emit an error when mixing <stdatomic.h> and <atomic>"
May 8 2018, 3:56 PM
vsapsai committed rL331818: Revert "Emit an error when mixing <stdatomic.h> and <atomic>".
Revert "Emit an error when mixing <stdatomic.h> and <atomic>"
May 8 2018, 3:54 PM

May 4 2018

vsapsai created D46446: [c++17] Fix assertion on synthesizing deduction guides after a fatal error..
May 4 2018, 12:23 PM

May 3 2018

vsapsai committed rL331491: Revert "Follow-up to r331378. Update tests to allow to use C atomics in C++.".
Revert "Follow-up to r331378. Update tests to allow to use C atomics in C++."
May 3 2018, 4:10 PM
vsapsai committed rCRT331491: Revert "Follow-up to r331378. Update tests to allow to use C atomics in C++.".
Revert "Follow-up to r331378. Update tests to allow to use C atomics in C++."
May 3 2018, 4:10 PM
vsapsai added a comment to D45470: Emit an error when include <atomic> after <stdatomic.h>.
In D45470#1087026, @jfb wrote:

This isn't bad, so I'd go with it, but separately I imagine that we could implement the suggestion in http://wg21.link/p0943 and expose it even before C++20? Not sure we do this much, but I'd argue that before that fix stdatomic.h is just useless anyways, so it's a fine breakage. I imagine that the stdatomic.h where it's implemented would be the one injected by clang, not the libc++ one?

May 3 2018, 3:00 PM
vsapsai committed rCRT331484: Follow-up to r331378. Update tests to allow to use C atomics in C++..
Follow-up to r331378. Update tests to allow to use C atomics in C++.
May 3 2018, 2:34 PM