On Fri, Nov 11, 2016 at 03:43:02PM -0800, Cesar Philippidis wrote:
> +         error_at (OMP_CLAUSE_LOCATION (c),
> +                   "%qs specifies a conflicting level of parallelism",
> +                   omp_clause_code_name[OMP_CLAUSE_CODE (c)]);
> +         inform (OMP_CLAUSE_LOCATION (c_level),
> +                 "... to the previous %qs clause here",

I think the '... ' part is unnecessary.
Perhaps word it better like we word errors/warnings for mismatched
attributes etc.?

> +    incompatible:
> +      if (c_diag != NULL_TREE)
> +     error_at (OMP_CLAUSE_LOCATION (c_diag),
> +               "incompatible %qs clause when applying"
> +               " %<%s%> to %qD, which has already been"
> +               " marked as an accelerator routine",
> +               omp_clause_code_name[OMP_CLAUSE_CODE (c_diag)],
> +               routine_str, fndecl);
> +      else if (c_diag_p != NULL_TREE)
> +     error_at (loc,
> +               "missing %qs clause when applying"
> +               " %<%s%> to %qD, which has already been"
> +               " marked as an accelerator routine",
> +               omp_clause_code_name[OMP_CLAUSE_CODE (c_diag_p)],
> +               routine_str, fndecl);
> +      else
> +     gcc_unreachable ();
> +      if (c_diag_p != NULL_TREE)
> +     inform (OMP_CLAUSE_LOCATION (c_diag_p),
> +             "... with %qs clause here",
> +             omp_clause_code_name[OMP_CLAUSE_CODE (c_diag_p)]);

Again, I think this usually would be something like "previous %qs clause"
or similar in the inform.  Generally, I think the error message should
be self-contained and infom should be just extra information, rather than
error containing first half of the diagnostic message and inform the second
one.  E.g. for translations, while such a sentence crossing the two
diagnostic routines might make sense in english, it might look terrible in
other languages.

> +      else
> +     {
> +       /* In the front ends, we don't preserve location information for the
> +          OpenACC routine directive itself.  However, that of c_level_p
> +          should be close.  */
> +       location_t loc_routine = OMP_CLAUSE_LOCATION (c_level_p);
> +       inform (loc_routine, "... without %qs clause near to here",
> +               omp_clause_code_name[OMP_CLAUSE_CODE (c_diag)]);
> +     }
> +      /* Incompatible.  */
> +      return -1;
> +    }
> +
> +  return 0;

        Jakub

Reply via email to