r290417 - Add an assert to catch improperly constructed %diff sequences in

[Richard Smith via cfe-commits]

On 22 December 2016 at 21:19, Chandler Carruth via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Author: chandlerc
> Date: Thu Dec 22 23:19:47 2016
> New Revision: 290417
>
> URL: http://llvm.org/viewvc/llvm-project?rev=290417&view=rev
> Log:
> Add an assert to catch improperly constructed %diff sequences in
> diagnostics and fix one such diagnostic.
>
> Sadly, this assert doesn't catch this bug because we have no tests that
> emit this diagnostic! Doh! I'm following up on the commit that
> introduces it to get that fixed. Then this assert will help in a more
> direct way.
>
> Modified:
>     cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>     cfe/trunk/lib/Basic/Diagnostic.cpp
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/
> DiagnosticSemaKinds.td?rev=290417&r1=290416&r2=290417&view=diff
> ============================================================
> ==================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Dec 22
> 23:19:47 2016
> @@ -3346,7 +3346,7 @@ def note_ovl_candidate_inconsistent_dedu
>  def note_ovl_candidate_inconsistent_deduction_types : Note<
>      "candidate template ignored: deduced values %diff{"
>      "of conflicting types for parameter %0 (%1 of type $ vs. %3 of type
> $)|"
> -    "%1 and %3 of conflicting types for parameter %0|}2,4">;
> +    "%1 and %3 of conflicting types for parameter %0}2,4">;
>  def note_ovl_candidate_explicit_arg_mismatch_named : Note<
>      "candidate template ignored: invalid explicitly-specified argument "
>      "for template parameter %0">;
>
> Modified: cfe/trunk/lib/Basic/Diagnostic.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/
> Diagnostic.cpp?rev=290417&r1=290416&r2=290417&view=diff
> ============================================================
> ==================
> --- cfe/trunk/lib/Basic/Diagnostic.cpp (original)
> +++ cfe/trunk/lib/Basic/Diagnostic.cpp Thu Dec 22 23:19:47 2016
> @@ -742,7 +742,10 @@ FormatDiagnostic(const char *DiagStr, co
>          //   "%diff{compare $ to $|other text}1,2"
>          // treat it as:
>          //   "compare %1 to %2"
> -        const char *Pipe = ScanFormat(Argument, Argument + ArgumentLen,
> '|');
> +        const char *ArgumentEnd = Argument + ArgumentLen;
> +        const char *Pipe = ScanFormat(Argument, ArgumentEnd, '|');
> +        assert(ScanFormat(Pipe + 1, ArgumentEnd, '|') == ArgumentEnd &&
> +               "Found too many '|'s in a %diff modifier!");
>

This is the fallback path for the case where the types being diffed turn
out to not both be types after all; your assert is unreachable for
properly-emitted diagnostics. Perhaps we should instead replace this whole
block with an llvm_unreachable and fix any cases where we're passing
non-types into a %diff...

         const char *FirstDollar = ScanFormat(Argument, Pipe, '$');
>          const char *SecondDollar = ScanFormat(FirstDollar + 1, Pipe, '$');
>          const char ArgStr1[] = { '%', static_cast<char>('0' + ArgNo) };
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20161223/40f4952a/attachment.html>