On Thu, Jul 12, 2012 at 2:07 PM, Richard Trieu <[email protected]> wrote:
> Change diagnostic messages in DiagnosticsSemaKinds.td to use %diff when > two QualType's are required. This will enable template type diffing for > all Sema messages. The default text of the diff matches the old message so > no test cases need to be updated. Some tests have been added for the > modified messages. > > Patch attached and also available at: > http://llvm-reviews.chandlerc.com/D6 > This looks awesome! I have a few questions: 1) A few of the diagnostics contain multiple %diff{...}s, for instance, err_init_conversion_failed. How well does that work in tree mode? 2) How should we handle diagnostics which refer only to built-in types? (Examples: the warn_impcast_* and most of the err_typecheck_* diagnostics.) Is the diff valuable here? Are there cases where this will cause worse diagnostics? More generally, how hesitant should we be before adding %diff to a diagnostic, and are there any rules we should be following? I'm not overly fond of the "between types" wording in the tree case for the warn_impcast_* set (it seems redundant to say that a cast is between types, although I see that you need some way to introduce the type tree). I wonder if something like this would read more naturally: 'implicit conversion changes value from %2 to %3; types are: <tree>' 3) How should we handle diagnostics are just naming two types which weren't expected to be the same, rather than comparing them to each other? (Examples: most of the cases where the diagnostic concerns an explicit cast.) In such a case, I would expect the way that the types differ would be uninteresting. My concern in such cases is mainly that adding a diff might make the diagnostic less clear, in the case where the diff either performs some desugaring or produces a tree. This also applies to err_ovl_ambiguous_oper_binary and err_typecheck_invalid_operands. Those are a little more interesting, because I suppose it will frequently be the case that the operands to a binary operator were supposed to be the same. However, I think it could be confusing to produce a type tree for these, since the diagnostic is fundamentally trying to list two types, not point out the difference between two types. 4) There's a typo in the text for warn_init_list_type_narrowing. (OK, that one's not a question...)
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
