aaron.ballman added a comment.
In D134453#3834659 <https://reviews.llvm.org/D134453#3834659>, @dblaikie wrote:
>>> OK, so it sounds like the printing behavior change (not necessarily with a
>>> policy flag) necessary for diagnostics (make it non-ambiguous/not invalid
>>> code which is what is currently emitted) would be a good bug fix both for
>>> diagnostics and for ast-print? I'm not sure we need anything other than
>>> that.
>>
>> Agreed, fixing up diagnostics and ast print to not print invalid types would
>> be a good fix to have. It doesn't necessitate a new printing policy.
>>
>>> So that what is currently printing as `t1<{}>` (ambiguous if the NTTP is of
>>> type `auto`, non-ambiguous, but invalid C++ if the NTTP has the specific
>>> type already) prints out as `t1<t2{}>` (correct C++ and disambiguates in
>>> the case of `auto` ambiguity).
>>>
>>> Or would you want the non-auto case to print in diagnostics as `t1<{}>`
>>> since there's no ambiguity, despite that not being valid C++? And then have
>>> a separate policy for ast-print that still prints it as `t1<t2{}>` so it's
>>> valid code?
>>
>> Personally (and others are welcome to disagree with me here!), I think we
>> would want to print `t1<t2{}>` even in the `auto` case for diagnostics and
>> ast print.
>
> (sorry to nitpick, but this conversation's been a bit confusing so trying to
> be extra clear) - I guess you meant "even in the *non*-`auto`" case? (the
> auto case I think is clearer that it should always have the type - otherwise
> it's ambiguous) - but yeah, I'm with you - I think the language requires the
> type even in the non-auto case and it seems reasonable to side with that for
> explicitness/clarity even if it could arguably be omitted without loss of
> information, technically.
No apologies necessary, sorry for the confusion! Yes, I meant the non-auto case
(practically, I mean both cases).
>> For diagnostics, we sometimes print type information reasonably far away
>> from where the declaration of a type is (and use a note to shift the user's
>> gaze to the related type), and so I think having the extra type information
>> is useful for that situation. But even when we print the type information
>> reasonably close to the declaration of the type, it can be helpful because
>> the code context might be easy to lose -- consider using Clang from an IDE
>> where diagnostics are added to a listbox; if you copy the diagnostic from
>> the listbox to paste into an email to a coworker to ask about it, the
>> related code isn't going to come along without extra intervention.
>
> Ok, so sounds like we're on the same page that `t1<{}>` is a diagnostic
> quality bug and we'd love to see a fix for it to include the top level type
> of the NTTP, as in `t1<t2{}>`, and that shouldn't need a new printing policy
> - because we never want to print `t1<{}>` and anywhere we do is a bug to be
> fixed.
I think we're on the same page, yes, thanks!
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
