Page MenuHomePhabricator

curdeius (Marek Kurdej)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 29 2013, 1:59 AM (390 w, 6 d)

Recent Activity

Fri, Jan 22

curdeius added a comment to D93912: [libc++][P1679] add string contains.

That's funny because https://en.cppreference.com/w/cpp/compiler_support is already updated!

Fri, Jan 22, 11:54 AM · Restricted Project
curdeius added inline comments to D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments.
Fri, Jan 22, 1:31 AM · Restricted Project, Restricted Project

Thu, Jan 21

curdeius added a comment to D95168: [clang-format] Add InsertBraces option.

True, I was aware of the presence of some of these options, thank you for indicating others. I'm not yet entirely convinced, especially that clang-tidy behaviour would be possibly different.

Thu, Jan 21, 11:31 PM · Restricted Project
curdeius added a reviewer for D95168: [clang-format] Add InsertBraces option: klimek.
Thu, Jan 21, 11:21 PM · Restricted Project
curdeius added inline comments to D94571: [libcxx] random_device, for OpenBSD specify optimal entropy properties.
Thu, Jan 21, 11:08 PM · Restricted Project
curdeius added inline comments to D94924: [libc++] first steps of a private modulemap.
Thu, Jan 21, 2:35 PM · Restricted Project
curdeius accepted D95081: [clang-format] [NFC] Restructure getLineCommentIndentPrefix.

Just an assert is ok IMO. We may fix it when LLVM will be compiled with C++20 but this code may change before it happens.

Thu, Jan 21, 2:28 PM · Restricted Project, Restricted Project
curdeius accepted D95128: [clang-format] [NFC] Remove unsued arguments.

LGTM. Thanks for tidying up!

Thu, Jan 21, 2:25 PM · Restricted Project, Restricted Project
curdeius added a comment to D95168: [clang-format] Add InsertBraces option.

There's a clang-tidy check for it.
And clang-format should not, IMO, add not remove tokens but only handle whitespace.

Thu, Jan 21, 2:21 PM · Restricted Project
curdeius committed rGf3b979b65e9f: [libc++] Use ioctl when available to get random_device entropy. (authored by curdeius).
[libc++] Use ioctl when available to get random_device entropy.
Thu, Jan 21, 9:01 AM
curdeius closed D94953: [libc++] Use ioctl when available to get random_device entropy..
Thu, Jan 21, 9:01 AM · Restricted Project

Wed, Jan 20

curdeius added inline comments to D95081: [clang-format] [NFC] Restructure getLineCommentIndentPrefix.
Wed, Jan 20, 2:36 PM · Restricted Project, Restricted Project
curdeius accepted D95078: [clang-format] [NFC] Use some constexpr StringRef instead of const char*..

LGTM.

Wed, Jan 20, 2:31 PM · Restricted Project, Restricted Project
curdeius updated the diff for D94953: [libc++] Use ioctl when available to get random_device entropy..

Fix has_include. Use result_type.

Wed, Jan 20, 1:40 AM · Restricted Project

Tue, Jan 19

curdeius updated the summary of D94955: [clang-format] Treat ForEachMacros as loops.
Tue, Jan 19, 10:45 PM · Restricted Project
curdeius requested changes to D94955: [clang-format] Treat ForEachMacros as loops.

LGTM per se. But, as it's a somehow a breaking change, I'd rather wait for release 12 branch before landing to main, so that folks have time to veto before release 13.
Please update release notes.
Also given that @MyDeveloperDay was involved in the bug discussion, please wait for his review too (if he has anything to add).

Tue, Jan 19, 10:43 PM · Restricted Project
curdeius added inline comments to D94953: [libc++] Use ioctl when available to get random_device entropy..
Tue, Jan 19, 12:49 PM · Restricted Project
curdeius added inline comments to D94953: [libc++] Use ioctl when available to get random_device entropy..
Tue, Jan 19, 5:24 AM · Restricted Project
curdeius updated the summary of D94953: [libc++] Use ioctl when available to get random_device entropy..
Tue, Jan 19, 1:03 AM · Restricted Project
curdeius requested review of D94953: [libc++] Use ioctl when available to get random_device entropy..
Tue, Jan 19, 1:01 AM · Restricted Project

Mon, Jan 18

curdeius committed rGa11f8b1ad66d: [libc++] [P0935] [C++20] Eradicating unnecessarily explicit default… (authored by curdeius).
[libc++] [P0935] [C++20] Eradicating unnecessarily explicit default…
Mon, Jan 18, 11:22 PM
curdeius closed D91292: [libc++] [P0935] [C++20] Eradicating unnecessarily explicit default constructors from the standard library..
Mon, Jan 18, 11:22 PM · Restricted Project
curdeius updated the diff for D91292: [libc++] [P0935] [C++20] Eradicating unnecessarily explicit default constructors from the standard library..

Rebase to trigger CI to reverify.

Mon, Jan 18, 12:54 PM · Restricted Project
curdeius accepted D94906: [clang-format] Apply Allman style to lambdas.

LGTM.
As a note, I agree with @MyDeveloperDay.
It's more user-friendly and IMO less surprising to set the base wrapping style in BreakBeforeBraces and then customize it changing in BraceWrapping.
I'd argue that it won't be a breaking change for .clang-format configs that are "correct" and would just make the examples you brought above work as intended.
Current behaviour is even strange in one case, what values are set to elements *omitted* in BraceWrapping when BreakBeforeBraces is Custom? Those from LLVM style?
Anyway, that goes way beyond this patch. I'd think about implementing this time permitting.

Mon, Jan 18, 12:23 PM · Restricted Project, Restricted Project

Sat, Jan 16

curdeius accepted D93776: [clang-format] Add StatementAttributeLikeMacros option.

LGTM apart from the minor comment.
But I'd like @MyDeveloperDay to have a look too.

Sat, Jan 16, 12:02 AM · Restricted Project, Restricted Project

Fri, Jan 15

curdeius added a comment to D94511: Implement p0586r2.

FWIW, you can take a look at the stub I've started writing at https://github.com/mkurdej/llvm-project/commit/0b7eccea2fbab13e897502c6d44ff958a0d550a9.
You might have a look at tests and other stuff like extended integer types.
It's just a WIP and far from ready though, so use with caution.

Fri, Jan 15, 12:38 AM · Restricted Project
curdeius added inline comments to D94718: [libc++] Unbreak the debug mode.
Fri, Jan 15, 12:12 AM · Restricted Project

Thu, Jan 14

curdeius added a comment to D94661: [clang-format] [PR19056] Add support for access modifiers indentation.

That looks like a strange style option to me. Is there any bigger codebase formatting the code this way?

Thu, Jan 14, 8:09 AM · Restricted Project, Restricted Project
curdeius accepted D93846: [clang-format] PR16518 Add flag to suppress empty line insertion before access modifier.

One last nit, otherwise LGTM.

Thu, Jan 14, 4:50 AM · Restricted Project, Restricted Project
curdeius added a comment to D56692: More calendar bits for <chrono>.

Shouldn't this patch implement also the renames of leap and link?
https://wg21.link/P1981 Rename leap to leap_second
https://wg21.link/P1982 Rename link to time_zone_link

Thu, Jan 14, 2:13 AM · Restricted Project

Wed, Jan 13

curdeius added inline comments to D75960: [libc++] Implement C++20's P0476r2: std::bit_cast.
Wed, Jan 13, 2:54 PM · Restricted Project
curdeius added inline comments to D75960: [libc++] Implement C++20's P0476r2: std::bit_cast.
Wed, Jan 13, 2:45 PM · Restricted Project

Tue, Jan 12

curdeius added a comment to D94530: [libc++] improve feature test macro script.

Concerning Python version, I don't know of any official policy but given that Python 3.5 is already end-of-life, requiring Python 3.6 seems OK.

Tue, Jan 12, 11:17 AM · Restricted Project
curdeius requested changes to D94511: Implement p0586r2.

The implementation looks okay at the first glance but you need to rework the tests.

  • Test noexcept using ASSERT_NOEXCEPT
  • Add tests in non-constexpr context. Look at other recently added commits and add a test function with asserts that gets called twice, once normally (runtime) and once in static_assert.
  • Split tests for each added function. Please follow the naming of test files according to the standard paragraphs.
  • Add more tests with all sorted integer types and extended integer types.
  • Test various values and their combinations: 0 limits::min/max +-1, min/2, max/2 and so on. Test interesting combinations of different types
Tue, Jan 12, 10:04 AM · Restricted Project
curdeius committed rG1f1250151f22: [libc++] [C++2b] [P1048] Add is_scoped_enum and is_scoped_enum_v. (authored by curdeius).
[libc++] [C++2b] [P1048] Add is_scoped_enum and is_scoped_enum_v.
Tue, Jan 12, 8:08 AM
curdeius closed D94409: [libc++] [C++2b] [P1048] Add is_scoped_enum and is_scoped_enum_v..
Tue, Jan 12, 8:08 AM · Restricted Project
curdeius added a comment to D91292: [libc++] [P0935] [C++20] Eradicating unnecessarily explicit default constructors from the standard library..

Friendly ping.

Tue, Jan 12, 4:58 AM · Restricted Project
curdeius accepted D93912: [libc++][P1679] add string contains.

LGTM.

Tue, Jan 12, 12:40 AM · Restricted Project
curdeius updated the diff for D94409: [libc++] [C++2b] [P1048] Add is_scoped_enum and is_scoped_enum_v..

Address comments.

Tue, Jan 12, 12:14 AM · Restricted Project

Mon, Jan 11

curdeius added a comment to D56997: Fix implementation of P0966 - string::reserve Should Not Shrink.

This review should probably be abandoned, as it was implemented in D91778.

Mon, Jan 11, 11:37 PM · Restricted Project
curdeius added inline comments to D94409: [libc++] [C++2b] [P1048] Add is_scoped_enum and is_scoped_enum_v..
Mon, Jan 11, 12:33 PM · Restricted Project
curdeius updated the diff for D94409: [libc++] [C++2b] [P1048] Add is_scoped_enum and is_scoped_enum_v..

Address review comments.

Mon, Jan 11, 12:32 PM · Restricted Project
curdeius committed rG30a7d430e869: [libc++] Turn off auto-formatting of generated files. NFC. (authored by curdeius).
[libc++] Turn off auto-formatting of generated files. NFC.
Mon, Jan 11, 11:49 AM
curdeius closed D94410: [libc++] Turn off auto-formatting of generated files. NFC..
Mon, Jan 11, 11:49 AM · Restricted Project
curdeius added inline comments to D94409: [libc++] [C++2b] [P1048] Add is_scoped_enum and is_scoped_enum_v..
Mon, Jan 11, 11:02 AM · Restricted Project
curdeius planned changes to D94409: [libc++] [C++2b] [P1048] Add is_scoped_enum and is_scoped_enum_v..
Mon, Jan 11, 11:00 AM · Restricted Project
curdeius requested review of D94410: [libc++] Turn off auto-formatting of generated files. NFC..
Mon, Jan 11, 6:27 AM · Restricted Project
curdeius requested review of D94409: [libc++] [C++2b] [P1048] Add is_scoped_enum and is_scoped_enum_v..
Mon, Jan 11, 6:21 AM · Restricted Project
curdeius added inline comments to D93912: [libc++][P1679] add string contains.
Mon, Jan 11, 5:27 AM · Restricted Project
curdeius requested changes to D93912: [libc++][P1679] add string contains.
Mon, Jan 11, 5:24 AM · Restricted Project
curdeius committed rG89878e8c966a: [clang-format] Find main include after block ended with #pragma hdrstop (authored by rjelonek).
[clang-format] Find main include after block ended with #pragma hdrstop
Mon, Jan 11, 12:50 AM
curdeius committed rG7473940bae0f: [clang-format] turn on formatting after "clang-format on" while sorting includes (authored by rjelonek).
[clang-format] turn on formatting after "clang-format on" while sorting includes
Mon, Jan 11, 12:41 AM
curdeius committed rGee27c767bd20: [clang-format] Skip UTF8 Byte Order Mark while sorting includes (authored by rjelonek).
[clang-format] Skip UTF8 Byte Order Mark while sorting includes
Mon, Jan 11, 12:33 AM

Sun, Jan 10

curdeius accepted D93912: [libc++][P1679] add string contains.

Great.
LGTM if CI passes. But please wait for an approval from somebody from libc++ group, e.g. @ldionne.

Sun, Jan 10, 6:03 AM · Restricted Project
curdeius added a comment to D93912: [libc++][P1679] add string contains.

Please reupload the diff setting the repo to rG LLVM Monorepo so that the CI gets triggered. You might need to do it twice to make it work. Otherwise use arcanist and it should do the job.
Otherwise looks good!

Sun, Jan 10, 5:54 AM · Restricted Project

Fri, Jan 8

curdeius accepted D91176: [20/N] [libcxx] Implement parsing of root_name for paths on windows.

LGTM. I'm looking forward to seeing all your 50+ patches landed :).

Fri, Jan 8, 2:00 PM · Restricted Project
curdeius added inline comments to D91176: [20/N] [libcxx] Implement parsing of root_name for paths on windows.
Fri, Jan 8, 1:44 PM · Restricted Project
curdeius added a comment to D91201: [libc++] LWG2070: Use Allocator construction for objects created with allocate_shared.

@ldionne, shouldn't the related bug be updated?
https://llvm.org/PR41900

Fri, Jan 8, 12:57 PM · Restricted Project
curdeius added inline comments to D93912: [libc++][P1679] add string contains.
Fri, Jan 8, 11:28 AM · Restricted Project
curdeius committed rG95729f95d803: [libc++] Add basic support for -std=c++2b. (authored by curdeius).
[libc++] Add basic support for -std=c++2b.
Fri, Jan 8, 10:03 AM
curdeius closed D94227: [libc++] Add basic support for -std=c++2b..
Fri, Jan 8, 10:03 AM · Restricted Project
curdeius added a comment to D91176: [20/N] [libcxx] Implement parsing of root_name for paths on windows.

I may be repeating myself, but my biggest remark is that there are no tests. I know there's no libcxx CI on Windows, but still, that will make reviewing easier, looking what are the new test cases and what has been forgotten.

Fri, Jan 8, 7:00 AM · Restricted Project
curdeius added a comment to D93912: [libc++][P1679] add string contains.

There is already D94227. You might want to wait for it and rebase when it's merged.

Fri, Jan 8, 4:11 AM · Restricted Project
curdeius updated the diff for D94227: [libc++] Add basic support for -std=c++2b..

Rebase and resolve conflicts.

Fri, Jan 8, 3:05 AM · Restricted Project

Thu, Jan 7

curdeius added a comment to D94287: [clang-format] Fix include sorting bug.

Seems to be a duplicate of D94206.

Thu, Jan 7, 11:31 PM · Restricted Project, Restricted Project
curdeius requested review of D94227: [libc++] Add basic support for -std=c++2b..
Thu, Jan 7, 4:41 AM · Restricted Project
curdeius committed rG044b892c79b3: [libc++] Use c++20 instead of c++2a consistently. (authored by curdeius).
[libc++] Use c++20 instead of c++2a consistently.
Thu, Jan 7, 4:11 AM
curdeius closed D93383: [libc++] Use c++20 instead of c++2a consistently..
Thu, Jan 7, 4:11 AM · Restricted Project
curdeius committed rGb6fb0209b6d4: [libc++] [CI] Install Tip-of-Trunk clang. (authored by curdeius).
[libc++] [CI] Install Tip-of-Trunk clang.
Thu, Jan 7, 3:04 AM
curdeius closed D93520: [libc++] [CI] Install Tip-of-Trunk clang..
Thu, Jan 7, 3:04 AM · Restricted Project
curdeius added a comment to D93520: [libc++] [CI] Install Tip-of-Trunk clang..

Yes, built and tested. Works fine. Will land it soon.

Thu, Jan 7, 1:40 AM · Restricted Project

Sun, Jan 3

curdeius added inline comments to D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments.
Sun, Jan 3, 12:39 PM · Restricted Project, Restricted Project

Sat, Jan 2

curdeius added a comment to D93938: [clang-format] Fixed AfterEnum handling.

I think we are missing some clarity in this bug as to what the actual problem is, I do agree the test looks wrong,

I agree on this. If like to see a more exhaustive test suite for all the combinations of AfterEnum and AllowShortEnumsOnASingleLine, not only fixing the single wrong test.

Sat, Jan 2, 6:14 AM · Restricted Project, Restricted Project

Wed, Dec 30

curdeius added inline comments to D93383: [libc++] Use c++20 instead of c++2a consistently..
Wed, Dec 30, 5:55 AM · Restricted Project
curdeius committed rG3f0b637d6b3e: [libc++] [docs] Mark contract-related papers as removed from C++20. (authored by curdeius).
[libc++] [docs] Mark contract-related papers as removed from C++20.
Wed, Dec 30, 5:25 AM
curdeius accepted D93839: [clang-format] PR48594 BraceWrapping: SplitEmptyRecord ignored for templates.

LGTM. Thank you!

Wed, Dec 30, 2:56 AM · Restricted Project, Restricted Project
curdeius added a comment to D93830: [libc++] Update generate_feature_test_macro_components.py to match SD-6.

You should add ["UNSUPPORTED: libcpp-has-no-threads"] to barrier, latch and semaphore.

Wed, Dec 30, 2:49 AM · Restricted Project
curdeius requested changes to D93938: [clang-format] Fixed AfterEnum handling.

Please add and fix tests.

Wed, Dec 30, 2:42 AM · Restricted Project, Restricted Project

Tue, Dec 29

curdeius added a comment to D93846: [clang-format] PR16518 Add flag to suppress empty line insertion before access modifier.

Nit.

Tue, Dec 29, 3:12 PM · Restricted Project, Restricted Project
curdeius requested changes to D93839: [clang-format] PR48594 BraceWrapping: SplitEmptyRecord ignored for templates.
Tue, Dec 29, 2:56 PM · Restricted Project, Restricted Project
curdeius added a comment to D93912: [libc++][P1679] add string contains.

You might want to wait for D93383 to land, so that it updates C++2a to C++20.
Then, we need to have a way to test C++2b. I have a patch D93520 that adds Tip of the Trunk clang supporting -std=c++2b.
When this is done and the builders are updated, we need to add C++2b to the tested configurations.

Tue, Dec 29, 2:33 PM · Restricted Project

Mon, Dec 28

curdeius updated the summary of D93846: [clang-format] PR16518 Add flag to suppress empty line insertion before access modifier.
Mon, Dec 28, 6:15 AM · Restricted Project, Restricted Project
curdeius added a comment to D93846: [clang-format] PR16518 Add flag to suppress empty line insertion before access modifier.

My 2 cents. Otherwise I agree with @MyDeveloperDay's comments.

Mon, Dec 28, 6:12 AM · Restricted Project, Restricted Project

Sat, Dec 26

curdeius added a comment to D93806: [clang-format] PR48569 clang-format fails to align case label with `switch` with Whitesmith Indentation.

This is the closest I could find thus far http://bitsavers.informatik.uni-stuttgart.de/pdf/chromatics/CGC_7900_C_Programmers_Manual_Mar82.pdf

and the very few examples from the manual show the case is aligned with the switch

Sat, Dec 26, 4:49 AM · Restricted Project, Restricted Project

Fri, Dec 25

curdeius accepted D93806: [clang-format] PR48569 clang-format fails to align case label with `switch` with Whitesmith Indentation.

It would be great if we had some spec for this style, it gets a bit hard to verify if the formatting is actually what is expected.
Having said that, LGTM if the bug reporter confirms the behaviour is ok.

Fri, Dec 25, 11:04 AM · Restricted Project, Restricted Project

Dec 23 2020

curdeius added a comment to D93776: [clang-format] Add StatementAttributeLikeMacros option.

Seems okay at the first glance. I'm wondering if we can find a better name though.

Dec 23 2020, 1:30 PM · Restricted Project, Restricted Project

Dec 22 2020

curdeius added a comment to D93626: [clang-format] PR48535 clang-format Incorrectly Removes Space After C Style Cast When Type Is Not a Pointer.

LGTM now. I tried to find other cases where this change may change the behaviour but couldn't. Have you tried applying to some bigger repo and see what you get? The best would be a repo with SpaceAfterCStyleCast=true.

Only my own internally, which I preconverted to using this style using an older clang-format then ran with this version. This seems to be fixing more cases than creating false positives:

Here are just 2 additional cases I found

int a = (int16)(a + (b << 2) + (c << 4));
if ((size_t)(a - b) <= c)

which become

int a = (int16) (a + (b << 2) + (c << 4));
if ((size_t) (a - b) <= c) {
Dec 22 2020, 6:53 AM · Restricted Project, Restricted Project
curdeius accepted D93626: [clang-format] PR48535 clang-format Incorrectly Removes Space After C Style Cast When Type Is Not a Pointer.

LGTM now. I tried to find other cases where this change may change the behaviour but couldn't. Have you tried applying to some bigger repo and see what you get? The best would be a repo with SpaceAfterCStyleCast=true.

Dec 22 2020, 4:49 AM · Restricted Project, Restricted Project
curdeius added inline comments to D93582: [libc++] [libc++abi] Correctly quote source and target names in tests. NFC..
Dec 22 2020, 3:23 AM · Restricted Project, Restricted Project

Dec 21 2020

curdeius added inline comments to D87587: [clang-format][PR47290] Add ShortNamespaceLines format option.
Dec 21 2020, 2:14 PM · Restricted Project, Restricted Project
curdeius updated the diff for D93582: [libc++] [libc++abi] Correctly quote source and target names in tests. NFC..
  • Quote {gdb}.
Dec 21 2020, 12:42 PM · Restricted Project, Restricted Project
curdeius added inline comments to D87587: [clang-format][PR47290] Add ShortNamespaceLines format option.
Dec 21 2020, 2:48 AM · Restricted Project, Restricted Project
curdeius added a comment to D93562: [libc++] Add a missing `<_Compare>` template argument..

Hmm, just one thought, it could be tested using non-copyable comparators passed by (non-reduced) lvalue ref, nope? How did you find this "missed optimization"?

Dec 21 2020, 1:03 AM · Restricted Project
curdeius accepted D93562: [libc++] Add a missing `<_Compare>` template argument..

LGTM.

Dec 21 2020, 1:00 AM · Restricted Project

Dec 20 2020

curdeius added a comment to D93562: [libc++] Add a missing `<_Compare>` template argument..

Excuse my ignorance, but could you please explain why letting _Compare get deduced is wrong/suboptimal?
IIUC the case you mention is when e.g. __buffered_inplace_merge has _Compare being some non-ref type T, so then calling __half_inplace_merge will deduce its _Compare to be an lvalue ref T &. So far so good. But why/when is it bad?

Dec 20 2020, 12:54 PM · Restricted Project
curdeius planned changes to D93582: [libc++] [libc++abi] Correctly quote source and target names in tests. NFC..
Dec 20 2020, 9:00 AM · Restricted Project, Restricted Project
curdeius added a comment to D93582: [libc++] [libc++abi] Correctly quote source and target names in tests. NFC..

Ok. For the moment I just fixed what was failing with my current setup but will fix cxx and gdb too.

Dec 20 2020, 9:00 AM · Restricted Project, Restricted Project
curdeius added a comment to D93590: [libc++] Implemented make_unique_for_overwrite.

There's already an ongoing effort to implement this: D90111.
You might want to sync with the author of the other patch.

Dec 20 2020, 12:43 AM · Restricted Project

Dec 19 2020

curdeius retitled D93582: [libc++] [libc++abi] Correctly quote source and target names in tests. NFC. from [libc++] Correctly quote source and target names in tests. NFC. to [libc++] [libc++abi] Correctly quote source and target names in tests. NFC..
Dec 19 2020, 12:26 PM · Restricted Project, Restricted Project
curdeius requested review of D93582: [libc++] [libc++abi] Correctly quote source and target names in tests. NFC..
Dec 19 2020, 6:35 AM · Restricted Project, Restricted Project