Page MenuHomePhabricator

ilya-biryukov (Ilya Biryukov)
UserAdministrator

Projects

User does not belong to any projects.

User Details

User Since
Apr 6 2017, 1:42 AM (311 w, 3 d)
Roles
Administrator

Recent Activity

Tue, Mar 21

ilya-biryukov added a comment to D146426: [Sema] Fix crash on __fp16 parameters in template instantiations.

I landed the fix to unbreak our crashes and explored a bit of the alternative solution.

Tue, Mar 21, 6:14 AM · Restricted Project, Restricted Project
ilya-biryukov committed rG282cae0b9a60: [Sema] Fix crash on __fp16 parameters in template instantiations (authored by ilya-biryukov).
[Sema] Fix crash on __fp16 parameters in template instantiations
Tue, Mar 21, 6:09 AM · Restricted Project, Restricted Project
ilya-biryukov closed D146426: [Sema] Fix crash on __fp16 parameters in template instantiations.
Tue, Mar 21, 6:09 AM · Restricted Project, Restricted Project
ilya-biryukov updated the diff for D146426: [Sema] Fix crash on __fp16 parameters in template instantiations.
  • Add a test for block pointers
Tue, Mar 21, 6:04 AM · Restricted Project, Restricted Project
ilya-biryukov added a comment to D146426: [Sema] Fix crash on __fp16 parameters in template instantiations.

As I noted in the bug report not doing D.setInvalidType(); does fix this bug and seems harmless since the error diagnostic should prevent us from getting to codegen but it is not clear to me if this has other negative impacts. Reading your replies it is not obvious you looked at this path or not.

Tue, Mar 21, 2:31 AM · Restricted Project, Restricted Project

Mon, Mar 20

ilya-biryukov added a comment to D146426: [Sema] Fix crash on __fp16 parameters in template instantiations.

This feels like it's heading in the wrong direction -- the AST should not have holes in it. An invalid type should be replaced by a valid type (after diagnosing the invalid type, of course) so that we can keep as much of the AST around as possible (for example, we typically stub in int and continue compilation, as in: https://godbolt.org/z/MvxjGovGh), which should then result in a non-null ParmVarDecl. This way, we don't need to sprinkle nullptr checks all over the compiler when inspecting a function's parameters.

Mon, Mar 20, 12:21 PM · Restricted Project, Restricted Project
ilya-biryukov added inline comments to D146426: [Sema] Fix crash on __fp16 parameters in template instantiations.
Mon, Mar 20, 9:30 AM · Restricted Project, Restricted Project
ilya-biryukov updated the diff for D146426: [Sema] Fix crash on __fp16 parameters in template instantiations.
  • rename test file to GH61441.cpp, add a comment this checks for no crash
  • Update outdated comment
Mon, Mar 20, 9:30 AM · Restricted Project, Restricted Project
ilya-biryukov added a comment to D146426: [Sema] Fix crash on __fp16 parameters in template instantiations.

Please edit the summary to indicate this fixes: https://github.com/llvm/llvm-project/issues/61441
I am happy someone had more time to dig into this problem after my initial investigation.

Mon, Mar 20, 9:06 AM · Restricted Project, Restricted Project
ilya-biryukov updated the summary of D146426: [Sema] Fix crash on __fp16 parameters in template instantiations.
Mon, Mar 20, 9:05 AM · Restricted Project, Restricted Project
ilya-biryukov requested review of D146426: [Sema] Fix crash on __fp16 parameters in template instantiations.
Mon, Mar 20, 7:54 AM · Restricted Project, Restricted Project

Fri, Mar 17

ilya-biryukov added a reverting change for rG54225c457a33: [Coroutines] Fix premature conversion of return object: rG4361ba791cd6: Revert "[Coroutines] Fix premature conversion of return object".
Fri, Mar 17, 9:02 AM · Restricted Project, Restricted Project
ilya-biryukov committed rG4361ba791cd6: Revert "[Coroutines] Fix premature conversion of return object" (authored by ilya-biryukov).
Revert "[Coroutines] Fix premature conversion of return object"
Fri, Mar 17, 9:02 AM · Restricted Project, Restricted Project
ilya-biryukov added a reverting change for D145639: [Coroutines] Fix premature conversion of return object: rG4361ba791cd6: Revert "[Coroutines] Fix premature conversion of return object".
Fri, Mar 17, 9:02 AM · Restricted Project, Restricted Project
ilya-biryukov added a comment to D145639: [Coroutines] Fix premature conversion of return object.

I suspect D145641 won't land today in the EU working hours.
My suggestion would be to revert this patch and reland together with D145641.
This should be in line with @ChuanqiXu's suggestions to wait for a response today.

Fri, Mar 17, 4:40 AM · Restricted Project, Restricted Project

Thu, Mar 16

ilya-biryukov added a comment to D145639: [Coroutines] Fix premature conversion of return object.

Reverting is common in Clang/LLVM. But let's wait for the response from @bruno temporarily. I think it makes sense to revert this one if @bruno don't response tomorrow. And we can commit this one and D145641 quickly the next time. Recommit is common too.

@bgraur, FYI, hope waiting for a day works for this case.

Thu, Mar 16, 2:55 AM · Restricted Project, Restricted Project
ilya-biryukov added a comment to D145639: [Coroutines] Fix premature conversion of return object.

Reverting the RVO breaks some coroutine code we have. The short repro is https://gcc.godbolt.org/z/1543qc8Ee (code at the end of the post, very similar to the deleted constructor in warn-throw-out-noexcept-coro.cpp):
The RVO seems to be mandated by the standard and D145641 is in the works to fix this.

Thu, Mar 16, 2:41 AM · Restricted Project, Restricted Project

Thu, Mar 9

ilya-biryukov added a comment to D145369: Emit const globals with constexpr destructor as constant LLVM values.

I don't have a lot of experience in codegen, so will let Aaron and Richard do the review.

Thu, Mar 9, 5:52 AM · Restricted Project, Restricted Project

Feb 7 2023

ilya-biryukov added a reviewer for D143436: [clangd] Move standard options adaptor to CommandMangler: kadircet.
Feb 7 2023, 1:57 AM · Restricted Project, Restricted Project, Restricted Project

Feb 3 2023

ilya-biryukov added a comment to D142384: [C++20] Fix a crash with modules..

It should be good to prevent crashes. But it looks not good that it doesn't have a test. Do you have plans to add a test case for this soon?

Feb 3 2023, 3:31 AM · Restricted Project, Restricted Project

Feb 2 2023

ilya-biryukov accepted D142384: [C++20] Fix a crash with modules..

I think this is fine, and we should just use the definition when it is available without asking the callers to request fields only when definition is available.

Feb 2 2023, 3:17 AM · Restricted Project, Restricted Project

Feb 1 2023

ilya-biryukov added a comment to D142384: [C++20] Fix a crash with modules..

I don't see any issues with this, so happy to LGTM. @rsmith any concerns on your side?
@usaxena95 could you give an example of the code that fails the assertion? Is it some of the tests?

Feb 1 2023, 8:33 AM · Restricted Project, Restricted Project

Jan 31 2023

ilya-biryukov added a reviewer for D142384: [C++20] Fix a crash with modules.: rsmith.

This fix looks very sensible to me and @rsmith confirmed this pattern that we are seeing might happen (seeing members of something that was demoted from declaration to a definition).
@rsmith could you confirm the update version looks good to you?

Jan 31 2023, 5:13 AM · Restricted Project, Restricted Project

Jan 25 2023

ilya-biryukov added a comment to D142446: [MC] Disable copying and moving of MCInstrDescs the C++20 way.

Alternatively, how bad would it be if aggregate initialization were removed and these objects were initialized with ctor calls? (bad for an -O0 build, no doubt... (how bad?) calling all those ctors to build the tables, but does clang optimize all the ctor calls away to the equivalent of aggregate init when using optimizations?)

Jan 25 2023, 3:22 AM · Restricted Project, Restricted Project

Jan 24 2023

ilya-biryukov committed rZORGa6d73d7d2faf: Fix a typo and disable more spammy warnings in C++20 buildbot (authored by ilya-biryukov).
Fix a typo and disable more spammy warnings in C++20 buildbot
Jan 24 2023, 4:18 AM · Restricted Project
ilya-biryukov added a comment to D142446: [MC] Disable copying and moving of MCInstrDescs the C++20 way.

Ah, I keep forgetting that the unknown attributes are fine. My experiments with MSVC suggest that it will increase the struct size: https://gcc.godbolt.org/z/offYrhn4K.
libc++ uses msvc::no_unique_address, which seems to work, but is MSVC-specific.

Jan 24 2023, 3:13 AM · Restricted Project, Restricted Project
ilya-biryukov added a comment to D142214: [MC] Do not copy MCInstrDescs. NFC..

For the record, the corresponding standard change is P1008R1.

Jan 24 2023, 2:56 AM · Restricted Project, Restricted Project
ilya-biryukov added a comment to D142214: [MC] Do not copy MCInstrDescs. NFC..

To describe the problem, the following code breaks in C++20:

struct WantAggregate {
  WantAggregate(const WantAggregate&) = delete;
  int a;
  int b;
};
Jan 24 2023, 2:53 AM · Restricted Project, Restricted Project
ilya-biryukov added a comment to D142214: [MC] Do not copy MCInstrDescs. NFC..

Interesting. Is there any easy way I can test building in C++20 mode on my own machine?

Jan 24 2023, 2:47 AM · Restricted Project, Restricted Project
ilya-biryukov committed rGf3d64644d8d7: [MC] Temporarily remove the deleted constructors, they break C++20 build (authored by ilya-biryukov).
[MC] Temporarily remove the deleted constructors, they break C++20 build
Jan 24 2023, 2:41 AM · Restricted Project, Restricted Project

Jan 23 2023

ilya-biryukov committed rZORGbcc90002ba91: Move C++20 buildbot to production (authored by ilya-biryukov).
Move C++20 buildbot to production
Jan 23 2023, 3:13 AM · Restricted Project

Jan 20 2023

ilya-biryukov added inline comments to D142187: [clang] Fix typos in member initializers.
Jan 20 2023, 7:43 AM · Restricted Project, Restricted Project
ilya-biryukov added a comment to D142187: [clang] Fix typos in member initializers.

Randomly chiming in here.
I never had a good model of where CorrectDelayedTyposInExpr, but wanted to note that ActOnFullExpr also calls it. This may be fine, I just wanted to mention it as it stood out.

Jan 20 2023, 1:51 AM · Restricted Project, Restricted Project

Jan 17 2023

ilya-biryukov added inline comments to D141690: [clang] fix consteval ctor code generation assert.
Jan 17 2023, 7:07 AM · Restricted Project, Restricted Project
ilya-biryukov committed rZORGb576693e40ce: Record the new version of clang-debian-cpp20 deployment (authored by ilya-biryukov).
Record the new version of clang-debian-cpp20 deployment
Jan 17 2023, 5:25 AM · Restricted Project
ilya-biryukov committed rZORG3eeb0300465d: Disable spammy deprecation warnings in C++20 buildbot (authored by ilya-biryukov).
Disable spammy deprecation warnings in C++20 buildbot
Jan 17 2023, 5:25 AM · Restricted Project

Jan 16 2023

ilya-biryukov accepted D141818: Add test for an invalid requirement in requires expr..

LGTM

Jan 16 2023, 2:48 AM · Restricted Project, Restricted Project

Jan 13 2023

ilya-biryukov added a comment to D141587: Add default constructurs to `filter_iterator_impl` and `filter_iterator_impl`..

Slight NIT: we might want to initialize End and iterator_adaptor_base::I with WrappedIteratorT End = WrappedIteratorT() to avoid uninitialized data members when WrappedIteratorT is a pointer.
This does not happen in practice right now, but if someone actually starts using this constructor in the future, we will at least not have UB.

Jan 13 2023, 9:15 AM · Restricted Project, Restricted Project
ilya-biryukov updated subscribers of D141690: [clang] fix consteval ctor code generation assert.

Sorry, didn't see your comment when submitting mine.

Jan 13 2023, 7:40 AM · Restricted Project, Restricted Project
ilya-biryukov added a comment to D141690: [clang] fix consteval ctor code generation assert.

The change seems reasonable. Could you add a test with a crasher repro and some validation of the generated IR?

Jan 13 2023, 7:02 AM · Restricted Project, Restricted Project
ilya-biryukov accepted D141671: Move definitions to prevent incomplete types..

LGTM! Looks much cleaner now, BTW.

Jan 13 2023, 6:57 AM · Restricted Project, Restricted Project
ilya-biryukov accepted D141663: Add notifications for clangd and cpp20 buildbot status.

LGTM, would be happy to get those emails.

Jan 13 2023, 3:04 AM · Restricted Project
ilya-biryukov added a comment to D140547: Perform access checking to private members in simple requirement..

Anyone else see this?

I have not checked, but I would not be surprised if we hit the stack size limits with asan enabled
@usaxena95, maybe reduce the number of instantiations from 10001 to 1001 or 101? It should not change the intention of the test and fix this failure.

Jan 13 2023, 3:01 AM · Restricted Project, Restricted Project
ilya-biryukov added a reviewer for D141671: Move definitions to prevent incomplete types.: kadircet.

There is potentially a way to move less code by keeping all declarations at place and only moving bodies of definitions of constructors and destructors to the .cpp file.
Not sure what's preferable (less code moves vs more functions inline in the header), @kadircet do you have an opinion?

Jan 13 2023, 2:54 AM · Restricted Project, Restricted Project

Jan 12 2023

ilya-biryukov committed rZORGde1f1b221d8a: Use Debian unstable for C++20 buildbot (authored by ilya-biryukov).
Use Debian unstable for C++20 buildbot
Jan 12 2023, 6:10 AM · Restricted Project
ilya-biryukov accepted D141546: [clang] Reland parenthesized aggregate init patches.

LGTM, mostly based on the previous review.
I have not noticed any significant changes, so have not looked too cautiously. Let me know if there are any particular places that you someone to take a look at before landing.

Jan 12 2023, 5:55 AM · Restricted Project, Restricted Project

Jan 11 2023

ilya-biryukov accepted D141392: Avoid u8"" literals in tests, their type changes in C++20.

LGTM, but please address the NITs before submitting.

Jan 11 2023, 9:28 AM · Restricted Project, Restricted Project
ilya-biryukov accepted D140547: Perform access checking to private members in simple requirement..

LGTM, thanks for fixing this!

Jan 11 2023, 2:00 AM · Restricted Project, Restricted Project

Jan 10 2023

ilya-biryukov added a comment to D141392: Avoid u8"" literals in tests, their type changes in C++20.

Are there any uses of u8 string literals with formatted_raw_ostream outside tests? What was the effect on the final behavior here?

Jan 10 2023, 8:22 AM · Restricted Project, Restricted Project
ilya-biryukov accepted D141368: Avoid u8"" literals in tests, their type changes in C++20.

LGTM. The presubmit failures are definitely unrelated as they are in a lit test for a different tool.

Jan 10 2023, 4:35 AM · Restricted Project, Restricted Project
ilya-biryukov added a comment to D141358: Remove unnecessary template specifiers from truct constructors in tests..
Jan 10 2023, 3:21 AM · Restricted Project, Restricted Project
ilya-biryukov accepted D141358: Remove unnecessary template specifiers from truct constructors in tests..

LGTM to unbreak the buildbots, this change definitely does not change semantics.

Jan 10 2023, 2:40 AM · Restricted Project, Restricted Project

Jan 9 2023

ilya-biryukov added a comment to D140547: Perform access checking to private members in simple requirement..

I think the only major problem is not checking for error case when accessing TransReq, the rest are NITs.
Thanks for the change! Will be happy to LGTM it as soon as the access to TransReq is fixed.

Jan 9 2023, 10:15 AM · Restricted Project, Restricted Project

Jan 3 2023

ilya-biryukov added a comment to D140876: [clang][C++20] Non-dependent access checks in requires expression..

Should access checks should happen in the context where concept is written or where it's used? Is there a standard wording around it?
If access-checking should happen where concept is defined, having a hard error seems fine because of the wording you quoted:

If the substitution of template arguments into a requirement would always result in a substitution failure, the program is ill-formed; no diagnostic required.

The program is ill-formed and we show the diagnostic even though it's not required.

Jan 3 2023, 8:16 AM · Restricted Project, Restricted Project
ilya-biryukov added inline comments to D140547: Perform access checking to private members in simple requirement..
Jan 3 2023, 8:00 AM · Restricted Project, Restricted Project

Dec 23 2022

ilya-biryukov accepted D140587: [clang] Fix a clang crash on invalid code in C++20 mode..

LGTM to unbreak clangd, this seems to pop up a lot for Chrome developers.

Dec 23 2022, 2:42 AM · Restricted Project, Restricted Project
ilya-biryukov updated subscribers of D140587: [clang] Fix a clang crash on invalid code in C++20 mode..
Dec 23 2022, 2:38 AM · Restricted Project, Restricted Project

Dec 22 2022

ilya-biryukov committed rZORG3e674cfc128c: Add a missing semicolon in C++20 build configuration (authored by ilya-biryukov).
Add a missing semicolon in C++20 build configuration
Dec 22 2022, 1:58 AM · Restricted Project
ilya-biryukov committed rZORGaf0b3ec05fac: Deploy new VERSION for clang-debian-cpp20 (authored by ilya-biryukov).
Deploy new VERSION for clang-debian-cpp20
Dec 22 2022, 1:58 AM · Restricted Project
ilya-biryukov committed rZORG09fb54904634: Do not log commands in docker images (authored by ilya-biryukov).
Do not log commands in docker images
Dec 22 2022, 1:51 AM · Restricted Project

Dec 21 2022

ilya-biryukov committed rZORG765dfd01bc42: Fix error LD env var in C++20 buildbot (authored by ilya-biryukov).
Fix error LD env var in C++20 buildbot
Dec 21 2022, 11:39 AM · Restricted Project
ilya-biryukov committed rZORGb8f129118fa5: Update VERSION file for the deployed docker image (authored by ilya-biryukov).
Update VERSION file for the deployed docker image
Dec 21 2022, 11:25 AM · Restricted Project
ilya-biryukov closed D140317: Add buildbot with C++20 configuration.
Dec 21 2022, 11:25 AM · Restricted Project
ilya-biryukov committed rZORG0aa13d7d8dbe: Add buildbot with C++20 configuration (authored by ilya-biryukov).
Add buildbot with C++20 configuration
Dec 21 2022, 11:25 AM · Restricted Project
ilya-biryukov updated the diff for D140317: Add buildbot with C++20 configuration.
  • Use kubernetes resource YAML for deployment instead of terraform config. I had problems updating our terraform deployment, it seems broken. I will follow up by moving existing clangd buildbot out of terrafrom too, the kubectl-based workflow seems prefereable.
  • Set version of the new docker image to 0 (script updates it on build).
Dec 21 2022, 11:05 AM · Restricted Project
ilya-biryukov accepted D140327: [clang] Remove overly restrictive aggregate paren init logic.

LGTM

Dec 21 2022, 8:19 AM · Restricted Project, Restricted Project
ilya-biryukov committed rZORG8c5bda98c41c: Fix a typo in /vol/ccache for clangd builder (authored by ilya-biryukov).
Fix a typo in /vol/ccache for clangd builder
Dec 21 2022, 3:31 AM · Restricted Project
ilya-biryukov closed D140390: Fix a typo in /vol/ccache for clangd builder.
Dec 21 2022, 3:31 AM · Restricted Project

Dec 20 2022

ilya-biryukov requested review of D140390: Fix a typo in /vol/ccache for clangd builder.
Dec 20 2022, 5:25 AM · Restricted Project
ilya-biryukov added inline comments to D140317: Add buildbot with C++20 configuration.
Dec 20 2022, 5:18 AM · Restricted Project
ilya-biryukov added inline comments to D140317: Add buildbot with C++20 configuration.
Dec 20 2022, 5:11 AM · Restricted Project
ilya-biryukov updated the diff for D140317: Add buildbot with C++20 configuration.
  • fix a typo in /vol/ccache
  • use password from clangd worker (approved by Galina in our email exchange)
  • leave out the targets and checks, use defaults that build and test everything
  • Use $CC and $LD instead of the wrong compilers in run.sh
Dec 20 2022, 5:11 AM · Restricted Project

Dec 19 2022

ilya-biryukov added a comment to D140317: Add buildbot with C++20 configuration.

Sending this out early to have it reviewed by the time I get the required credentials.
This will also require the corresponding configuration changes in our GCP changes before being submitted.

Dec 19 2022, 9:45 AM · Restricted Project
ilya-biryukov requested review of D140317: Add buildbot with C++20 configuration.
Dec 19 2022, 9:41 AM · Restricted Project

Dec 15 2022

ilya-biryukov added inline comments to D140044: [CodeComplete] Complete members of dependent `auto` variables.
Dec 15 2022, 3:12 AM · Restricted Project, Restricted Project

Dec 14 2022

ilya-biryukov accepted D129531: [clang][C++20] P0960R3 and P1975R0: Allow initializing aggregates from a parenthesized list of values.

LGTM! Thanks a lot for driving this to conclusion! Please note that there is one small NIT you may want to fix before landing this.
UB definitely explains why we saw the bugs. It's good that we caught it before this landed.

Dec 14 2022, 7:08 AM · Restricted Project, Restricted Project

Dec 12 2022

ilya-biryukov added a comment to D129531: [clang][C++20] P0960R3 and P1975R0: Allow initializing aggregates from a parenthesized list of values.

Since the errors only shows up in modular builds, I would look closely for bugs inside ASTReader/ASTWriter.
Also, it seems that ArrayFiller is not taken in to account in computeDependence and maybe it should be. I am not 100% sure, though: if ArrayFiller is only used for non-dependent code, it should not case this bug. It also does not explain the variation between modular and non-modular builds.

Dec 12 2022, 9:22 AM · Restricted Project, Restricted Project
ilya-biryukov added a comment to D129531: [clang][C++20] P0960R3 and P1975R0: Allow initializing aggregates from a parenthesized list of values.

This LG, but not accepting as I believe libc++ is still broken if built with modules, right?
I have run check-cxx locally and it seems to work, but I suspect that's not using modules by default. I have clicked through the links in phrabricator and the errors seem to come from the latest uploaded diff.
Let me know if I'm missing something.

Dec 12 2022, 9:17 AM · Restricted Project, Restricted Project

Dec 9 2022

ilya-biryukov added inline comments to D129531: [clang][C++20] P0960R3 and P1975R0: Allow initializing aggregates from a parenthesized list of values.
Dec 9 2022, 8:09 AM · Restricted Project, Restricted Project
ilya-biryukov committed rGedd5d777e981: [clangd] NFC. Add a newline at the end of the file (authored by ilya-biryukov).
[clangd] NFC. Add a newline at the end of the file
Dec 9 2022, 3:33 AM · Restricted Project, Restricted Project

Dec 7 2022

ilya-biryukov accepted D139541: Fix parameter name in Sema::addInitCapture to ByRef..

LGTM. BTW, feel free to send small changes like these without review. The common jargon for them in LLVM is NFC (non-functional change)

Dec 7 2022, 8:20 AM · Restricted Project, Restricted Project
ilya-biryukov added a comment to D139107: [clangd] Allow to build Clangd without decision forest.

I have sent a follow-up NFC change to fix a typo in the documentation of --ranking-model=heuristics. Note that I decided to avoid changing the order of options for --ranking-model in the documentation, this didn't seem important.

Dec 7 2022, 4:55 AM · Restricted Project, Restricted Project
ilya-biryukov committed rGcdfce10b28b1: [clangd] Fix a typo in -ranking-model documentation. NFC (authored by ilya-biryukov).
[clangd] Fix a typo in -ranking-model documentation. NFC
Dec 7 2022, 4:54 AM · Restricted Project, Restricted Project
ilya-biryukov committed rGc9b325088d14: [clangd] Allow to build Clangd without decision forest (authored by ilya-biryukov).
[clangd] Allow to build Clangd without decision forest
Dec 7 2022, 4:52 AM · Restricted Project, Restricted Project
ilya-biryukov closed D139107: [clangd] Allow to build Clangd without decision forest.
Dec 7 2022, 4:52 AM · Restricted Project, Restricted Project
ilya-biryukov updated the diff for D139107: [clangd] Allow to build Clangd without decision forest.
  • Add a comment to CMakelists.txt explaining motivation for the option
  • Add #include "Feature.h"
  • Revert changes to the order of compiler flags to keep this change more focused
  • clang-format
  • replace #ifndef with #if ! in ClangdMain.cpp
Dec 7 2022, 4:50 AM · Restricted Project, Restricted Project
ilya-biryukov accepted D139125: [clang] Correctly handle by-reference capture with an initializer that is a pack expansion in lambdas..

Thanks, LGTM!

Dec 7 2022, 2:02 AM · Restricted Project, Restricted Project

Dec 5 2022

ilya-biryukov added a comment to D129531: [clang][C++20] P0960R3 and P1975R0: Allow initializing aggregates from a parenthesized list of values.

I have two major comments and also inline NITs. Not sure if we should block on those, just wanted to hear your opinions:

  • InitListExpr and CXXParenInitListExpr have some code in common. Code duplication is substantial and I think sharing the common implementation is warranted. E.g. if some code would want to change something with ArrayFiller or make a use of it, the work will probably need to be duplicated. To reiterate what I mentioned earlier, I believe deriving CXXParenInitListExpr from InitListExpr is not a very good option, but we might explore other possibilities: a base class or factoring out common code in a separate struct. I believe this could be done with a follow-up change, though, as the amount of changes required does not seem too big, I would be happy with first landing this patch and then cleaning up with a follow-up change.
  • Did we explore the possibility of modelling this differently and somehow avoiding making CXXParenInitListExpr an expression. This seems to borrow from InitListExpr, but maybe for the wrong reasons. Braced initializers can actually occur in expressions positions, e.g. int a = {123}. However, CXXParenInistListExpr cannot occur in expression positions outside initializations, e.g. having CXXFunctionalCastExpr for aggregate_struct(123) feels somewhat wrong to me and seems to lead to confusing error messages too. Maybe we should model it as a new expression that contains both the type and the argument list? I.e. more similar to CXXConstructExpr?
Dec 5 2022, 9:31 AM · Restricted Project, Restricted Project
ilya-biryukov added inline comments to D139107: [clangd] Allow to build Clangd without decision forest.
Dec 5 2022, 2:56 AM · Restricted Project, Restricted Project
ilya-biryukov updated the diff for D139107: [clangd] Allow to build Clangd without decision forest.
  • Merge DecisionForest.cpp and *_disabled.cpp, change #ifndef to #if
  • Add decision_forest to the feature string
  • Set Heuristics as the default code completion model in tests
  • Do not compile decision forest tests when it's disabled
  • Remove the use of preprocessor for defining the default value of -ranking-model flag to clangd
Dec 5 2022, 2:55 AM · Restricted Project, Restricted Project

Dec 2 2022

ilya-biryukov added inline comments to D139125: [clang] Correctly handle by-reference capture with an initializer that is a pack expansion in lambdas..
Dec 2 2022, 4:47 AM · Restricted Project, Restricted Project

Dec 1 2022

ilya-biryukov added a comment to D139107: [clangd] Allow to build Clangd without decision forest.

This seems to build in both modes, but I still need to figure out what to do with tests with decision forest off.
Also, maybe there is a way to minimize the use of preprocessor even more.

Dec 1 2022, 6:18 AM · Restricted Project, Restricted Project
ilya-biryukov requested review of D139107: [clangd] Allow to build Clangd without decision forest.
Dec 1 2022, 6:17 AM · Restricted Project, Restricted Project

Nov 28 2022

ilya-biryukov accepted D138727: [clang] Skip defaulted functions in zero-as-null-pointer-constant..

LGTM with a few NITs. Thanks!

Nov 28 2022, 9:03 AM · Restricted Project, Restricted Project
ilya-biryukov accepted D138210: [clang] Require parameter pack to be last argument in concepts..
Nov 28 2022, 8:48 AM · Restricted Project, Restricted Project
ilya-biryukov added a comment to D138210: [clang] Require parameter pack to be last argument in concepts..

Thanks for the explanation. LGTM! And thanks for adding an assert.

Nov 28 2022, 8:42 AM · Restricted Project, Restricted Project
ilya-biryukov added inline comments to D129531: [clang][C++20] P0960R3 and P1975R0: Allow initializing aggregates from a parenthesized list of values.
Nov 28 2022, 8:05 AM · Restricted Project, Restricted Project
ilya-biryukov added a comment to D129531: [clang][C++20] P0960R3 and P1975R0: Allow initializing aggregates from a parenthesized list of values.

Thanks for addressing the comments and sorry for a wait before the comments.
Please see the comment about syntactic form, I might be holding it wrong, but it looks like we actually loose the syntactic form completely in this case.

Nov 28 2022, 7:52 AM · Restricted Project, Restricted Project

Nov 25 2022

ilya-biryukov committed rGdb335d02a5e7: [clang-tidy] Ignore cxxRewrittenBinaryOperator in defaulted function decls in… (authored by massberg).
[clang-tidy] Ignore cxxRewrittenBinaryOperator in defaulted function decls in…
Nov 25 2022, 6:46 AM · Restricted Project, Restricted Project
ilya-biryukov closed D138701: [clang-tidy] Ignore cxxRewrittenBinaryOperator in defaulted function decls in modernize-use-nullptr..
Nov 25 2022, 6:46 AM · Restricted Project, Restricted Project