mizvekov added a comment.

In D134453#3880729 <https://reviews.llvm.org/D134453#3880729>, @dblaikie wrote:
> I think we should restrict the discussion in this review to being only about 
> the case where we have the canonical type (acknowledging that there's broader 
> work to reduce the number of cases where that is what we have to deal with).

I agree that we can leave that aside for this patch.

> When you say "for reasonably small objects" are you suggesting this patch/an 
> nearish-term solution to the original problem presented here (`t1<{}>` with 
> no info about the type of the NTTP) should include some heuristic to judge 
> "reasonably small objects" and choose a different rendering in that case?

I don't think we necessarily have to do something about that from the start, 
but it's probably a problem that is going to come up at some point.
We are trying to render the whole structure of a class type in one line of 
diagnostic, seems like this breaks new ground.

> OK, I'm not sure what to make of this, though - "the expression that 
> initializes the NTTP" is something we don't have, right? Just the value, I 
> think, at this point? So we're trying to pick some kind of rendering for that 
> value - is it just the outer type and then braces and values (Using the 
> "bad"/difficult case of nested subobjects - `t1{{3}, {4}}`) or with nested 
> types (`t1{t2{3}, t3{4]}`) or with member names (`t1{.v1 = {.v2 = 3}, .v3 = 
> {.v4 = 4}}`) or both (`t1{.v1 = t2{.v2 = 3}, .v3 = t3{.v4 = 4}}`).
>
> I guess you meant the first, `t1{{3}, {4}}`? OK, so I think you're agreeing 
> with what I would prefer/outlined in https://reviews.llvm.org/D134453#3871251 
> (which isn't where this review was going when @aaron.ballman roped you in, it 
> was going towards fully explicit (`t1{t2{3}, t3{4}}`))

I am a bit torn actually, something like `t1{.name = "Bar"}` does look better 
ideally, but it's probably more prone to bad corner cases. Will probably not 
work well for tuples.

The second option, `t1{{3}, {4}}`, looks more well-rounded.

The fully explicit `t1{t2{3}, t3{4}}` option seems like it would be both less 
readable and also even more prone to bad corner cases than the first (field 
names) option.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134453/new/

https://reviews.llvm.org/D134453

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to