riscv: fix -Wformat-diag errors.
Commit Message
Pushed as pre-approved by Jeff. The patch fixes -Wformat-diag warnings.
Martin
gcc/ChangeLog:
* common/config/riscv/riscv-common.cc (riscv_subset_list::add):
Wrap keywords with quotes and remove trailing dots.
(riscv_subset_list::parsing_subset_version): Likewise.
(riscv_subset_list::parse_std_ext): Likewise.
(riscv_subset_list::parse_multiletter_ext): Likewise.
* config/riscv/riscv.cc (riscv_handle_type_attribute): Likewise.
---
gcc/common/config/riscv/riscv-common.cc | 16 ++++++++--------
gcc/config/riscv/riscv.cc | 4 ++--
2 files changed, 10 insertions(+), 10 deletions(-)
Comments
Thanks Martin!
On Wed, Jan 19, 2022 at 12:23 AM Martin Liška <mliska@suse.cz> wrote:
>
> Pushed as pre-approved by Jeff. The patch fixes -Wformat-diag warnings.
>
> Martin
>
> gcc/ChangeLog:
>
> * common/config/riscv/riscv-common.cc (riscv_subset_list::add):
> Wrap keywords with quotes and remove trailing dots.
> (riscv_subset_list::parsing_subset_version): Likewise.
> (riscv_subset_list::parse_std_ext): Likewise.
> (riscv_subset_list::parse_multiletter_ext): Likewise.
> * config/riscv/riscv.cc (riscv_handle_type_attribute): Likewise.
> ---
> gcc/common/config/riscv/riscv-common.cc | 16 ++++++++--------
> gcc/config/riscv/riscv.cc | 4 ++--
> 2 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
> index c1d8431c1fa..004822bfe6c 100644
> --- a/gcc/common/config/riscv/riscv-common.cc
> +++ b/gcc/common/config/riscv/riscv-common.cc
> @@ -375,7 +375,7 @@ riscv_subset_list::add (const char *subset, int major_version,
> else
> error_at (
> m_loc,
> - "%<-march=%s%>: Extension `%s' appear more than one time.",
> + "%<-march=%s%>: extension %qs appear more than one time",
> m_arch,
> subset);
>
> @@ -613,14 +613,14 @@ riscv_subset_list::parsing_subset_version (const char *ext,
> {
> if (!ISDIGIT (*(p+1)))
> {
> - error_at (m_loc, "%<-march=%s%>: Expect number "
> - "after %<%dp%>.", m_arch, version);
> + error_at (m_loc, "%<-march=%s%>: expect number "
> + "after %<%dp%>", m_arch, version);
> return NULL;
> }
> if (!major_p)
> {
> - error_at (m_loc, "%<-march=%s%>: For %<%s%dp%dp?%>, version "
> - "number with more than 2 level is not supported.",
> + error_at (m_loc, "%<-march=%s%>: for %<%s%dp%dp?%>, version "
> + "number with more than 2 level is not supported",
> m_arch, ext, major, version);
> return NULL;
> }
> @@ -701,8 +701,8 @@ riscv_subset_list::parse_std_ext (const char *p)
> /* std_ext_p= */ true, &explicit_version_p);
> if (major_version != 0 || minor_version != 0)
> {
> - warning_at (m_loc, 0, "version of `g` will be omitted, please "
> - "specify version for individual extension.");
> + warning_at (m_loc, 0, "version of %<g%> will be omitted, please "
> + "specify version for individual extension");
> }
>
> /* We have special rule for G, we disallow rv32gm2p but allow rv32g_zicsr
> @@ -906,7 +906,7 @@ riscv_subset_list::parse_multiletter_ext (const char *p,
>
> if (*p != '\0' && *p != '_')
> {
> - error_at (m_loc, "%<-march=%s%>: %s must separate with _",
> + error_at (m_loc, "%<-march=%s%>: %s must separate with %<_%>",
> m_arch, ext_type_str);
> return NULL;
> }
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 7c806780883..8314864d5e7 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -3309,8 +3309,8 @@ riscv_handle_type_attribute (tree *node ATTRIBUTE_UNUSED, tree name, tree args,
> && strcmp (string, "machine"))
> {
> warning (OPT_Wattributes,
> - "argument to %qE attribute is not \"user\", \"supervisor\", or \"machine\"",
> - name);
> + "argument to %qE attribute is not %<user%>, %<supervisor%>, "
> + "or %<machine%>", name);
> *no_add_attrs = true;
> }
> }
> --
> 2.34.1
>
>
On Tue, 18 Jan 2022 08:31:12 PST (-0800), gcc-patches@gcc.gnu.org wrote:
> Thanks Martin!
Yep. Seeing this go by, though, I think there's some English issues
with the original messages. I'd write it more like this, but I'm never
100% sure on these things:
diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
index 004822bfe6c..2f83303ca51 100644
--- a/gcc/common/config/riscv/riscv-common.cc
+++ b/gcc/common/config/riscv/riscv-common.cc
@@ -375,7 +375,7 @@ riscv_subset_list::add (const char *subset, int major_version,
else
error_at (
m_loc,
- "%<-march=%s%>: extension %qs appear more than one time",
+ "%<-march=%s%>: extension %qs appears more than one time",
m_arch,
subset);
@@ -620,7 +620,7 @@ riscv_subset_list::parsing_subset_version (const char *ext,
if (!major_p)
{
error_at (m_loc, "%<-march=%s%>: for %<%s%dp%dp?%>, version "
- "number with more than 2 level is not supported",
+ "numbers with more than 2 levels are not supported",
m_arch, ext, major, version);
return NULL;
}
@@ -701,8 +701,9 @@ riscv_subset_list::parse_std_ext (const char *p)
/* std_ext_p= */ true, &explicit_version_p);
if (major_version != 0 || minor_version != 0)
{
- warning_at (m_loc, 0, "version of %<g%> will be omitted, please "
- "specify version for individual extension");
+ warning_at (m_loc, 0, "version of %<g%> will be ignored, please "
+ "specify versions for each individual "
+ "extension");
}
/* We have special rule for G, we disallow rv32gm2p but allow rv32g_zicsr
>
> On Wed, Jan 19, 2022 at 12:23 AM Martin Liška <mliska@suse.cz> wrote:
>>
>> Pushed as pre-approved by Jeff. The patch fixes -Wformat-diag warnings.
>>
>> Martin
>>
>> gcc/ChangeLog:
>>
>> * common/config/riscv/riscv-common.cc (riscv_subset_list::add):
>> Wrap keywords with quotes and remove trailing dots.
>> (riscv_subset_list::parsing_subset_version): Likewise.
>> (riscv_subset_list::parse_std_ext): Likewise.
>> (riscv_subset_list::parse_multiletter_ext): Likewise.
>> * config/riscv/riscv.cc (riscv_handle_type_attribute): Likewise.
>> ---
>> gcc/common/config/riscv/riscv-common.cc | 16 ++++++++--------
>> gcc/config/riscv/riscv.cc | 4 ++--
>> 2 files changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
>> index c1d8431c1fa..004822bfe6c 100644
>> --- a/gcc/common/config/riscv/riscv-common.cc
>> +++ b/gcc/common/config/riscv/riscv-common.cc
>> @@ -375,7 +375,7 @@ riscv_subset_list::add (const char *subset, int major_version,
>> else
>> error_at (
>> m_loc,
>> - "%<-march=%s%>: Extension `%s' appear more than one time.",
>> + "%<-march=%s%>: extension %qs appear more than one time",
>> m_arch,
>> subset);
>>
>> @@ -613,14 +613,14 @@ riscv_subset_list::parsing_subset_version (const char *ext,
>> {
>> if (!ISDIGIT (*(p+1)))
>> {
>> - error_at (m_loc, "%<-march=%s%>: Expect number "
>> - "after %<%dp%>.", m_arch, version);
>> + error_at (m_loc, "%<-march=%s%>: expect number "
>> + "after %<%dp%>", m_arch, version);
>> return NULL;
>> }
>> if (!major_p)
>> {
>> - error_at (m_loc, "%<-march=%s%>: For %<%s%dp%dp?%>, version "
>> - "number with more than 2 level is not supported.",
>> + error_at (m_loc, "%<-march=%s%>: for %<%s%dp%dp?%>, version "
>> + "number with more than 2 level is not supported",
>> m_arch, ext, major, version);
>> return NULL;
>> }
>> @@ -701,8 +701,8 @@ riscv_subset_list::parse_std_ext (const char *p)
>> /* std_ext_p= */ true, &explicit_version_p);
>> if (major_version != 0 || minor_version != 0)
>> {
>> - warning_at (m_loc, 0, "version of `g` will be omitted, please "
>> - "specify version for individual extension.");
>> + warning_at (m_loc, 0, "version of %<g%> will be omitted, please "
>> + "specify version for individual extension");
>> }
>>
>> /* We have special rule for G, we disallow rv32gm2p but allow rv32g_zicsr
>> @@ -906,7 +906,7 @@ riscv_subset_list::parse_multiletter_ext (const char *p,
>>
>> if (*p != '\0' && *p != '_')
>> {
>> - error_at (m_loc, "%<-march=%s%>: %s must separate with _",
>> + error_at (m_loc, "%<-march=%s%>: %s must separate with %<_%>",
>> m_arch, ext_type_str);
>> return NULL;
>> }
>> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
>> index 7c806780883..8314864d5e7 100644
>> --- a/gcc/config/riscv/riscv.cc
>> +++ b/gcc/config/riscv/riscv.cc
>> @@ -3309,8 +3309,8 @@ riscv_handle_type_attribute (tree *node ATTRIBUTE_UNUSED, tree name, tree args,
>> && strcmp (string, "machine"))
>> {
>> warning (OPT_Wattributes,
>> - "argument to %qE attribute is not \"user\", \"supervisor\", or \"machine\"",
>> - name);
>> + "argument to %qE attribute is not %<user%>, %<supervisor%>, "
>> + "or %<machine%>", name);
>> *no_add_attrs = true;
>> }
>> }
>> --
>> 2.34.1
>>
>>
On Tue, 18 Jan 2022, Martin Liška wrote:
> @@ -3309,8 +3309,8 @@ riscv_handle_type_attribute (tree *node
> ATTRIBUTE_UNUSED, tree name, tree args,
> && strcmp (string, "machine"))
> {
> warning (OPT_Wattributes,
> - "argument to %qE attribute is not \"user\",
> \"supervisor\", or \"machine\"",
> - name);
> + "argument to %qE attribute is not %<user%>,
> %<supervisor%>, "
> + "or %<machine%>", name);
> *no_add_attrs = true;
My reading is that the attribute arguments here are string constants, not
identifiers - that is, the ASCII double quotes are correct in the
diagnostic output, because those double quotes are part of the literal
text that's supposed to appear in the program. (Maybe %<\"user\"%> is the
right way of marking it up to indicate that the double quotes are part of
the literal program text, not English-level quoting.)
On 1/18/22 23:00, Joseph Myers wrote:
> On Tue, 18 Jan 2022, Martin Liška wrote:
>
>> @@ -3309,8 +3309,8 @@ riscv_handle_type_attribute (tree *node
>> ATTRIBUTE_UNUSED, tree name, tree args,
>> && strcmp (string, "machine"))
>> {
>> warning (OPT_Wattributes,
>> - "argument to %qE attribute is not \"user\",
>> \"supervisor\", or \"machine\"",
>> - name);
>> + "argument to %qE attribute is not %<user%>,
>> %<supervisor%>, "
>> + "or %<machine%>", name);
>> *no_add_attrs = true;
>
> My reading is that the attribute arguments here are string constants, not
> identifiers - that is, the ASCII double quotes are correct in the
> diagnostic output, because those double quotes are part of the literal
> text that's supposed to appear in the program. (Maybe %<\"user\"%> is the
> right way of marking it up to indicate that the double quotes are part of
> the literal program text, not English-level quoting.)
Makes sense, I' going to install the following patch.
Martin
On Tue, 18 Jan 2022, Palmer Dabbelt wrote:
> Yep. Seeing this go by, though, I think there's some English issues with the
> original messages. I'd write it more like this, but I'm never 100% sure on
> these things:
>
> diff --git a/gcc/common/config/riscv/riscv-common.cc
> b/gcc/common/config/riscv/riscv-common.cc
> index 004822bfe6c..2f83303ca51 100644
> --- a/gcc/common/config/riscv/riscv-common.cc
> +++ b/gcc/common/config/riscv/riscv-common.cc
> @@ -375,7 +375,7 @@ riscv_subset_list::add (const char *subset, int
> major_version,
> else
> error_at (
> m_loc,
> - "%<-march=%s%>: extension %qs appear more than one time",
> + "%<-march=%s%>: extension %qs appears more than one time",
Or "... more than once" even.
Maciej
@@ -375,7 +375,7 @@ riscv_subset_list::add (const char *subset, int major_version,
else
error_at (
m_loc,
- "%<-march=%s%>: Extension `%s' appear more than one time.",
+ "%<-march=%s%>: extension %qs appear more than one time",
m_arch,
subset);
@@ -613,14 +613,14 @@ riscv_subset_list::parsing_subset_version (const char *ext,
{
if (!ISDIGIT (*(p+1)))
{
- error_at (m_loc, "%<-march=%s%>: Expect number "
- "after %<%dp%>.", m_arch, version);
+ error_at (m_loc, "%<-march=%s%>: expect number "
+ "after %<%dp%>", m_arch, version);
return NULL;
}
if (!major_p)
{
- error_at (m_loc, "%<-march=%s%>: For %<%s%dp%dp?%>, version "
- "number with more than 2 level is not supported.",
+ error_at (m_loc, "%<-march=%s%>: for %<%s%dp%dp?%>, version "
+ "number with more than 2 level is not supported",
m_arch, ext, major, version);
return NULL;
}
@@ -701,8 +701,8 @@ riscv_subset_list::parse_std_ext (const char *p)
/* std_ext_p= */ true, &explicit_version_p);
if (major_version != 0 || minor_version != 0)
{
- warning_at (m_loc, 0, "version of `g` will be omitted, please "
- "specify version for individual extension.");
+ warning_at (m_loc, 0, "version of %<g%> will be omitted, please "
+ "specify version for individual extension");
}
/* We have special rule for G, we disallow rv32gm2p but allow rv32g_zicsr
@@ -906,7 +906,7 @@ riscv_subset_list::parse_multiletter_ext (const char *p,
if (*p != '\0' && *p != '_')
{
- error_at (m_loc, "%<-march=%s%>: %s must separate with _",
+ error_at (m_loc, "%<-march=%s%>: %s must separate with %<_%>",
m_arch, ext_type_str);
return NULL;
}
@@ -3309,8 +3309,8 @@ riscv_handle_type_attribute (tree *node ATTRIBUTE_UNUSED, tree name, tree args,
&& strcmp (string, "machine"))
{
warning (OPT_Wattributes,
- "argument to %qE attribute is not \"user\", \"supervisor\", or \"machine\"",
- name);
+ "argument to %qE attribute is not %<user%>, %<supervisor%>, "
+ "or %<machine%>", name);
*no_add_attrs = true;
}
}