On 26/09/17 00:25, Steve Ellcey wrote:
> This is a new version of my patch to fix PR target/79868, where some
> error messages are impossible to translate correctly due to how the
> strings are dynamically constructed. It also includes some format
> changes in the error messags to make the messages more consistent with
> each other and with other GCC errors. This was worked out with help
> from Martin Sebor. I also had to fix some tests to match the new error
> string formats.
>
> Tested on Aarch64 with no regressions, OK to checkin?
I can't help feeling that all this logic is somewhat excessive and
changing the wording of each message to include "pragma or attribute"
would solve it equally well. With the new context highlighting it's
trivial to tell which subcase of usage is being referred to.
R.
>
> Steve Ellcey
> sell...@cavium.com
>
>
> 2017-09-25 Steve Ellcey <sell...@cavium.com>
>
> PR target/79868
> * config/aarch64/aarch64-c.c (aarch64_pragma_target_parse):
> Change argument type on aarch64_process_target_attr call.
> * config/aarch64/aarch64-protos.h (aarch64_process_target_attr):
> Change argument type.
> * config/aarch64/aarch64.c (aarch64_attribute_info): Change
> field type.
> (aarch64_handle_attr_arch): Change argument type, use boolean
> argument to use different strings in error calls.
> (aarch64_handle_attr_cpu): Ditto.
> (aarch64_handle_attr_tune): Ditto.
> (aarch64_handle_attr_isa_flags): Ditto.
> (aarch64_process_one_target_attr): Ditto.
> (aarch64_process_target_attr): Ditto.
> (aarch64_option_valid_attribute_p): Change argument type on
> aarch64_process_target_attr call.
>
>
> 2017-09-25 Steve Ellcey <sell...@cavium.com>
>
> PR target/79868
> * gcc.target/aarch64/spellcheck_1.c: Update dg-error string to match
> new format.
> * gcc.target/aarch64/spellcheck_2.c: Ditto.
> * gcc.target/aarch64/spellcheck_3.c: Ditto.
> * gcc.target/aarch64/target_attr_11.c: Ditto.
> * gcc.target/aarch64/target_attr_12.c: Ditto.
> * gcc.target/aarch64/target_attr_17.c: Ditto.
>
>
> pr79868.patch
>
>
> diff --git a/gcc/config/aarch64/aarch64-c.c b/gcc/config/aarch64/aarch64-c.c
> index 177e638..c9945db 100644
> --- a/gcc/config/aarch64/aarch64-c.c
> +++ b/gcc/config/aarch64/aarch64-c.c
> @@ -165,7 +165,7 @@ aarch64_pragma_target_parse (tree args, tree pop_target)
> information that it specifies. */
> if (args)
> {
> - if (!aarch64_process_target_attr (args, "pragma"))
> + if (!aarch64_process_target_attr (args, true))
> return false;
>
> aarch64_override_options_internal (&global_options);
> diff --git a/gcc/config/aarch64/aarch64-protos.h
> b/gcc/config/aarch64/aarch64-protos.h
> index e67c2ed..4323e9e 100644
> --- a/gcc/config/aarch64/aarch64-protos.h
> +++ b/gcc/config/aarch64/aarch64-protos.h
> @@ -445,7 +445,7 @@ bool aarch64_gen_adjusted_ldpstp (rtx *, bool,
> scalar_mode, RTX_CODE);
>
> void aarch64_init_builtins (void);
>
> -bool aarch64_process_target_attr (tree, const char*);
> +bool aarch64_process_target_attr (tree, bool);
> void aarch64_override_options_internal (struct gcc_options *);
>
> rtx aarch64_expand_builtin (tree exp,
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 1c14008..122ed5e 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -67,6 +67,7 @@
> #include "common/common-target.h"
> #include "selftest.h"
> #include "selftest-rtl.h"
> +#include "intl.h"
>
> /* This file should be included last. */
> #include "target-def.h"
> @@ -9554,15 +9555,15 @@ struct aarch64_attribute_info
> const char *name;
> enum aarch64_attr_opt_type attr_type;
> bool allow_neg;
> - bool (*handler) (const char *, const char *);
> + bool (*handler) (const char *, bool);
> enum opt_code opt_num;
> };
>
> /* Handle the ARCH_STR argument to the arch= target attribute.
> - PRAGMA_OR_ATTR is used in potential error messages. */
> + IS_PRAGMA is used in potential error messages. */
>
> static bool
> -aarch64_handle_attr_arch (const char *str, const char *pragma_or_attr)
> +aarch64_handle_attr_arch (const char *str, bool is_pragma)
> {
> const struct processor *tmp_arch = NULL;
> enum aarch64_parse_opt_result parse_res
> @@ -9579,15 +9580,22 @@ aarch64_handle_attr_arch (const char *str, const char
> *pragma_or_attr)
> switch (parse_res)
> {
> case AARCH64_PARSE_MISSING_ARG:
> - error ("missing architecture name in 'arch' target %s", pragma_or_attr);
> + error (is_pragma
> + ? G_("missing name in %<target(\"arch=\")%> pragma")
> + : G_("missing name in %<target(\"arch=\")%> attribute"));
> break;
> case AARCH64_PARSE_INVALID_ARG:
> - error ("unknown value %qs for 'arch' target %s", str, pragma_or_attr);
> + error (is_pragma
> + ? G_("invalid name (\"%s\") in %<target(\"arch=\")%> pragma")
> + : G_("invalid name (\"%s\") in %<target(\"arch=\")%> attribute"),
> + str);
> aarch64_print_hint_for_arch (str);
> break;
> case AARCH64_PARSE_INVALID_FEATURE:
> - error ("invalid feature modifier %qs for 'arch' target %s",
> - str, pragma_or_attr);
> + error (is_pragma
> + ? G_("invalid value (\"%s\") in %<target()%> pragma")
> + : G_("invalid value (\"%s\") in %<target()%> attribute"),
> + str);
> break;
> default:
> gcc_unreachable ();
> @@ -9597,10 +9605,10 @@ aarch64_handle_attr_arch (const char *str, const char
> *pragma_or_attr)
> }
>
> /* Handle the argument CPU_STR to the cpu= target attribute.
> - PRAGMA_OR_ATTR is used in potential error messages. */
> + IS_PRAGMA is used in potential error messages. */
>
> static bool
> -aarch64_handle_attr_cpu (const char *str, const char *pragma_or_attr)
> +aarch64_handle_attr_cpu (const char *str, bool is_pragma)
> {
> const struct processor *tmp_cpu = NULL;
> enum aarch64_parse_opt_result parse_res
> @@ -9620,15 +9628,22 @@ aarch64_handle_attr_cpu (const char *str, const char
> *pragma_or_attr)
> switch (parse_res)
> {
> case AARCH64_PARSE_MISSING_ARG:
> - error ("missing cpu name in 'cpu' target %s", pragma_or_attr);
> + error (is_pragma
> + ? G_("missing name in %<target(\"cpu=\")%> pragma")
> + : G_("missing name in %<target(\"cpu=\")%> attribute"));
> break;
> case AARCH64_PARSE_INVALID_ARG:
> - error ("unknown value %qs for 'cpu' target %s", str, pragma_or_attr);
> + error (is_pragma
> + ? G_("invalid name (\"%s\") in %<target(\"cpu=\")%> pragma")
> + : G_("invalid name (\"%s\") in %<target(\"cpu=\")%> attribute"),
> + str);
> aarch64_print_hint_for_core (str);
> break;
> case AARCH64_PARSE_INVALID_FEATURE:
> - error ("invalid feature modifier %qs for 'cpu' target %s",
> - str, pragma_or_attr);
> + error (is_pragma
> + ? G_("invalid value (\"%s\") in %<target()%> pragma")
> + : G_("invalid value (\"%s\") in %<target()%> attribute"),
> + str);
> break;
> default:
> gcc_unreachable ();
> @@ -9638,10 +9653,10 @@ aarch64_handle_attr_cpu (const char *str, const char
> *pragma_or_attr)
> }
>
> /* Handle the argument STR to the tune= target attribute.
> - PRAGMA_OR_ATTR is used in potential error messages. */
> + IS_PRAGMA is used in potential error messages. */
>
> static bool
> -aarch64_handle_attr_tune (const char *str, const char *pragma_or_attr)
> +aarch64_handle_attr_tune (const char *str, bool is_pragma)
> {
> const struct processor *tmp_tune = NULL;
> enum aarch64_parse_opt_result parse_res
> @@ -9658,7 +9673,10 @@ aarch64_handle_attr_tune (const char *str, const char
> *pragma_or_attr)
> switch (parse_res)
> {
> case AARCH64_PARSE_INVALID_ARG:
> - error ("unknown value %qs for 'tune' target %s", str, pragma_or_attr);
> + error (is_pragma
> + ? G_("invalid name (\"%s\") in %<target(\"tune=\")%> pragma")
> + : G_("invalid name (\"%s\") in %<target(\"tune=\")%> attribute"),
> + str);
> aarch64_print_hint_for_core (str);
> break;
> default:
> @@ -9672,10 +9690,10 @@ aarch64_handle_attr_tune (const char *str, const char
> *pragma_or_attr)
> For example "+fp+nosimd". Show any errors if needed. Return TRUE
> if successful. Update aarch64_isa_flags to reflect the ISA features
> modified.
> - PRAGMA_OR_ATTR is used in potential error messages. */
> + IS_PRAGMA is used in potential error messages. */
>
> static bool
> -aarch64_handle_attr_isa_flags (char *str, const char *pragma_or_attr)
> +aarch64_handle_attr_isa_flags (char *str, bool is_pragma)
> {
> enum aarch64_parse_opt_result parse_res;
> unsigned long isa_flags = aarch64_isa_flags;
> @@ -9699,13 +9717,16 @@ aarch64_handle_attr_isa_flags (char *str, const char
> *pragma_or_attr)
> switch (parse_res)
> {
> case AARCH64_PARSE_MISSING_ARG:
> - error ("missing feature modifier in target %s %qs",
> - pragma_or_attr, str);
> + error (is_pragma
> + ? G_("missing value in %<target()%> pragma")
> + : G_("missing value in %<target()%> attribute"));
> break;
>
> case AARCH64_PARSE_INVALID_FEATURE:
> - error ("invalid feature modifier in target %s %qs",
> - pragma_or_attr, str);
> + error (is_pragma
> + ? G_("invalid value (\"%s\") in %<target()%> pragma")
> + : G_("invalid value (\"%s\") in %<target()%> attribute"),
> + str);
> break;
>
> default:
> @@ -9744,11 +9765,11 @@ static const struct aarch64_attribute_info
> aarch64_attributes[] =
>
> /* Parse ARG_STR which contains the definition of one target attribute.
> Show appropriate errors if any or return true if the attribute is valid.
> - PRAGMA_OR_ATTR holds the string to use in error messages about whether
> + IS_PRAGMA holds the boolean to tell error messages about whether
> we're processing a target attribute or pragma. */
>
> static bool
> -aarch64_process_one_target_attr (char *arg_str, const char* pragma_or_attr)
> +aarch64_process_one_target_attr (char *arg_str, bool is_pragma)
> {
> bool invert = false;
>
> @@ -9756,7 +9777,9 @@ aarch64_process_one_target_attr (char *arg_str, const
> char* pragma_or_attr)
>
> if (len == 0)
> {
> - error ("malformed target %s", pragma_or_attr);
> + error (is_pragma
> + ? G_("malformed %<target()%> pragma")
> + : G_("malformed %<target()%> attribute"));
> return false;
> }
>
> @@ -9772,7 +9795,7 @@ aarch64_process_one_target_attr (char *arg_str, const
> char* pragma_or_attr)
> through the machinery for the rest of the target attributes in this
> function. */
> if (*str_to_check == '+')
> - return aarch64_handle_attr_isa_flags (str_to_check, pragma_or_attr);
> + return aarch64_handle_attr_isa_flags (str_to_check, is_pragma);
>
> if (len > 3 && strncmp (str_to_check, "no-", 3) == 0)
> {
> @@ -9804,8 +9827,10 @@ aarch64_process_one_target_attr (char *arg_str, const
> char* pragma_or_attr)
>
> if (attr_need_arg_p ^ (arg != NULL))
> {
> - error ("target %s %qs does not accept an argument",
> - pragma_or_attr, str_to_check);
> + error (is_pragma
> + ? G_("pragma %<target(\"%s\")%> does not accept an argument")
> + : G_("attribute %<target(\"%s\")%> does not accept an
> argument"),
> + str_to_check);
> return false;
> }
>
> @@ -9813,8 +9838,10 @@ aarch64_process_one_target_attr (char *arg_str, const
> char* pragma_or_attr)
> then we can't match. */
> if (invert && !p_attr->allow_neg)
> {
> - error ("target %s %qs does not allow a negated form",
> - pragma_or_attr, str_to_check);
> + error (is_pragma
> + ? G_("pragma %<target(\"%s\")%> does not allow a negated form")
> + : G_("attribute %<target(\"%s\")%> does not allow a negated
> form"),
> + str_to_check);
> return false;
> }
>
> @@ -9824,7 +9851,7 @@ aarch64_process_one_target_attr (char *arg_str, const
> char* pragma_or_attr)
> For example, cpu=, arch=, tune=. */
> case aarch64_attr_custom:
> gcc_assert (p_attr->handler);
> - if (!p_attr->handler (arg, pragma_or_attr))
> + if (!p_attr->handler (arg, is_pragma))
> return false;
> break;
>
> @@ -9868,8 +9895,10 @@ aarch64_process_one_target_attr (char *arg_str, const
> char* pragma_or_attr)
> }
> else
> {
> - error ("target %s %s=%s is not valid",
> - pragma_or_attr, str_to_check, arg);
> + error (is_pragma
> + ? G_("pragma %<target(\"%s=%s\")%> is not valid")
> + : G_("attribute %<target(\"%s=%s\")%> is not valid"),
> + str_to_check, arg);
> }
> break;
> }
> @@ -9903,12 +9932,12 @@ num_occurences_in_str (char c, char *str)
> }
>
> /* Parse the tree in ARGS that contains the target attribute information
> - and update the global target options space. PRAGMA_OR_ATTR is a string
> - to be used in error messages, specifying whether this is processing
> + and update the global target options space. IS_PRAGMA is a boolean
> + to be used by error messages, specifying whether this is processing
> a target attribute or a target pragma. */
>
> bool
> -aarch64_process_target_attr (tree args, const char* pragma_or_attr)
> +aarch64_process_target_attr (tree args, bool is_pragma)
> {
> if (TREE_CODE (args) == TREE_LIST)
> {
> @@ -9917,7 +9946,7 @@ aarch64_process_target_attr (tree args, const char*
> pragma_or_attr)
> tree head = TREE_VALUE (args);
> if (head)
> {
> - if (!aarch64_process_target_attr (head, pragma_or_attr))
> + if (!aarch64_process_target_attr (head, is_pragma))
> return false;
> }
> args = TREE_CHAIN (args);
> @@ -9938,7 +9967,9 @@ aarch64_process_target_attr (tree args, const char*
> pragma_or_attr)
>
> if (len == 0)
> {
> - error ("malformed target %s value", pragma_or_attr);
> + error (is_pragma
> + ? G_("malformed %<target()%> pragma")
> + : G_("malformed %<target()%> attribute"));
> return false;
> }
>
> @@ -9953,9 +9984,12 @@ aarch64_process_target_attr (tree args, const char*
> pragma_or_attr)
> while (token)
> {
> num_attrs++;
> - if (!aarch64_process_one_target_attr (token, pragma_or_attr))
> + if (!aarch64_process_one_target_attr (token, is_pragma))
> {
> - error ("target %s %qs is invalid", pragma_or_attr, token);
> + error (is_pragma
> + ? G_("pragma %<target(\"%s\")%> is not valid")
> + : G_("attribute %<target(\"%s\")%> is not valid"),
> + token);
> return false;
> }
>
> @@ -9964,8 +9998,10 @@ aarch64_process_target_attr (tree args, const char*
> pragma_or_attr)
>
> if (num_attrs != num_commas + 1)
> {
> - error ("malformed target %s list %qs",
> - pragma_or_attr, TREE_STRING_POINTER (args));
> + error (is_pragma
> + ? G_("malformed %<target(\"%s\")%> pragma")
> + : G_("malformed %<target(\"%s\")%> attribute"),
> + TREE_STRING_POINTER (args));
> return false;
> }
>
> @@ -10025,7 +10061,7 @@ aarch64_option_valid_attribute_p (tree fndecl, tree,
> tree args, int)
> TREE_TARGET_OPTION (target_option_current_node));
>
>
> - ret = aarch64_process_target_attr (args, "attribute");
> + ret = aarch64_process_target_attr (args, false);
>
> /* Set up any additional state. */
> if (ret)
>
>
> pr79868-tests.patch
>
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_1.c
> b/gcc/testsuite/gcc.target/aarch64/spellcheck_1.c
> index ccfe417..33ea312 100644
> --- a/gcc/testsuite/gcc.target/aarch64/spellcheck_1.c
> +++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_1.c
> @@ -4,6 +4,6 @@ __attribute__((target ("arch=armv8-a-typo"))) void
> foo ()
> {
> /* { dg-message "valid arguments are: \[^\n\r]*; did you mean 'armv8-a'?"
> "" { target *-*-* } .-1 } */
> - /* { dg-error "unknown value 'armv8-a-typo' for 'arch' target attribute"
> "" { target *-*-* } .-2 } */
> - /* { dg-error "target attribute 'arch=armv8-a-typo' is invalid" "" {
> target *-*-* } .-3 } */
> + /* { dg-error "invalid name \\(\"armv8-a-typo\"\\) in
> 'target\\(\"arch=\"\\)' attribute" "" { target *-*-* } .-2 } */
> + /* { dg-error "attribute 'target\\(\"arch=armv8-a-typo\"\\)' is not valid"
> "" { target *-*-* } .-3 } */
> }
> diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_2.c
> b/gcc/testsuite/gcc.target/aarch64/spellcheck_2.c
> index 42ba51a..533b949 100644
> --- a/gcc/testsuite/gcc.target/aarch64/spellcheck_2.c
> +++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_2.c
> @@ -3,7 +3,7 @@
> __attribute__((target ("cpu=cortex-a57-typo"))) void
> foo ()
> {
> - /* { dg-message "valid arguments are: \[^\n\r]*; did you mean
> 'cortex-a57?" "" { target *-*-* } .-1 } */
> - /* { dg-error "unknown value 'cortex-a57-typo' for 'cpu' target attribute"
> "" { target *-*-* } .-2 } */
> - /* { dg-error "target attribute 'cpu=cortex-a57-typo' is invalid" "" {
> target *-*-* } .-3 } */
> + /* { dg-message "valid arguments are: \[^\n\r]*; did you mean
> 'cortex-a57'?" "" { target *-*-* } .-1 } */
> + /* { dg-error "invalid name \\(\"cortex-a57-typo\"\\) in
> 'target\\(\"cpu=\"\\)' attribute" "" { target *-*-* } .-2 } */
> + /* { dg-error "attribute 'target\\(\"cpu=cortex-a57-typo\"\\)' is not
> valid" "" { target *-*-* } .-3 } */
> }
> diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_3.c
> b/gcc/testsuite/gcc.target/aarch64/spellcheck_3.c
> index 03d2bbf..017f110 100644
> --- a/gcc/testsuite/gcc.target/aarch64/spellcheck_3.c
> +++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_3.c
> @@ -3,7 +3,7 @@
> __attribute__((target ("tune=cortex-a57-typo"))) void
> foo ()
> {
> - /* { dg-message "valid arguments are: \[^\n\r]*; did you mean
> 'cortex-a57?" "" { target *-*-* } .-1 } */
> - /* { dg-error "unknown value 'cortex-a57-typo' for 'tune' target
> attribute" "" { target *-*-* } .-2 } */
> - /* { dg-error "target attribute 'tune=cortex-a57-typo' is invalid" "" {
> target *-*-* } .-3 } */
> + /* { dg-message "valid arguments are: \[^\n\r]*; did you mean
> 'cortex-a57'?" "" { target *-*-* } .-1 } */
> + /* { dg-error "invalid name \\(\"cortex-a57-typo\"\\) in
> 'target\\(\"tune=\"\\)' attribute" "" { target *-*-* } .-2 } */
> + /* { dg-error "attribute 'target\\(\"tune=cortex-a57-typo\"\\)' is not
> valid" "" { target *-*-* } .-3 } */
> }
> diff --git a/gcc/testsuite/gcc.target/aarch64/target_attr_11.c
> b/gcc/testsuite/gcc.target/aarch64/target_attr_11.c
> index 7cfb826..a3df438 100644
> --- a/gcc/testsuite/gcc.target/aarch64/target_attr_11.c
> +++ b/gcc/testsuite/gcc.target/aarch64/target_attr_11.c
> @@ -10,4 +10,4 @@ foo (int a)
> }
>
> /* { dg-error "does not allow a negated form" "" { target *-*-* } 0 } */
> -/* { dg-error "is invalid" "" { target *-*-* } 0 } */
> +/* { dg-error "is not valid" "" { target *-*-* } 0 } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/target_attr_12.c
> b/gcc/testsuite/gcc.target/aarch64/target_attr_12.c
> index 39cb996..8a3a25b 100644
> --- a/gcc/testsuite/gcc.target/aarch64/target_attr_12.c
> +++ b/gcc/testsuite/gcc.target/aarch64/target_attr_12.c
> @@ -10,4 +10,4 @@ foo (int a)
> }
>
> /* { dg-error "does not accept an argument" "" { target *-*-* } 0 } */
> -/* { dg-error "is invalid" "" { target *-*-* } 0 } */
> +/* { dg-error "is not valid" "" { target *-*-* } 0 } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/target_attr_17.c
> b/gcc/testsuite/gcc.target/aarch64/target_attr_17.c
> index 483cc6d..2a7a751 100644
> --- a/gcc/testsuite/gcc.target/aarch64/target_attr_17.c
> +++ b/gcc/testsuite/gcc.target/aarch64/target_attr_17.c
> @@ -5,4 +5,4 @@ foo (int a)
> return a + 5;
> }
>
> -/* { dg-error "target attribute.*is invalid" "" { target *-*-* } 0 } */
> \ No newline at end of file
> +/* { dg-error "attribute 'target\\(\"invalid-attr-string\"\\)' is not valid"
> "" { target *-*-* } 0 } */
>
