On 10/25/2017 6:44 PM, Mukesh Kapoor wrote:
On 10/25/2017 4:20 AM, Nathan Sidwell wrote:
On 10/25/2017 12:03 AM, Mukesh Kapoor wrote:
Thanks for pointing this out. Checking in the front end will be
difficult because the front end gets tokens after macro expansion. I
think the difficulty of fixing this bug comes because of the
requirement to maintain backward compatibility with the option
-Wliteral-suffix for -std=c++11.
IIUC the warning's intent is to catch cases of:
printf ("some format"PRIx64 ..., ...);
where there's no space between the string literals and the PRIx64
macro. I suspect it's very common for there to be a following
string-literal, so perhaps the preprocessor could detect:
<string-literal>NON-FN-MACRO<maybe-space><string-literal>
and warn on that sequence?
Yes, this can be done easily and this is also the usage mentioned in
the man page. I made this change in the compiler, bootstrapped it and
ran the tests. The following two tests fail after the fix:
g++.dg/cpp0x/Wliteral-suffix.C
g++.dg/cpp0x/warn_cxx0x4.C
Both tests have code similar to the following (from Wliteral-suffix.C):
#define BAR "bar"
#define PLUS_ONE + 1
char c = '3'PLUS_ONE; // { dg-warning "invalid suffix on literal" }
char s[] = "foo"BAR; // { dg-warning "invalid suffix on literal" }
Other compilers don't accept this code. Maybe I should just modify
these tests to have error messages instead of warnings and submit my
revised fix?
Actually, according to the man page for -Wliteral-suffix, only macro
names that don't start with an underscore should be considered when
issuing a warning:
-Wliteral-suffix (C++ and Objective-C++ only)
Warn when a string or character literal is followed by a
ud-suffix
which does not begin with an underscore...
So the fix is simply to check if the macro name in is_macro() starts
with an underscore. The function is_macro() is called only at three
places. At two places it's used to check for the warning related to
-Wliteral-suffix and the check for underscore should be made for these
two cases; at one place it is used to check for the warning related to
-Wc++11-compat and there is no need to check for underscore for this case.
The fix is simply to pass a bool flag as an additional argument to
is_macro() to decide whether the macro name starts with an underscore or
not. I have tested the attached patch on x86_64-linux. Thanks.
Mukesh
/libcpp
2017-10-31 Mukesh Kapoor <mukesh.kap...@oracle.com>
PR c++/80955
* lex.c (lex_string): Add an argument, 'bool no_underscore', to
is_macro(). If no_underscore is true, check if the macro name
starts with an underscore and return false if it does.
If no_underscore is false, don't check for underscore.
is_macro() is called at three places. At two places it's used to
check for the warning related to -Wliteral-suffix and the check for
underscore is made for these two cases. At one place it's used to
check for the warning related to -Wc++11-compat and the check for
underscore is not made for this case.
/testsuite
2017-10-31 Mukesh Kapoor <mukesh.kap...@oracle.com>
PR c++/80955
* g++.dg/cpp0x/udlit-macros.C: New.
Index: gcc/testsuite/g++.dg/cpp0x/udlit-macros.C
===================================================================
--- gcc/testsuite/g++.dg/cpp0x/udlit-macros.C (revision 0)
+++ gcc/testsuite/g++.dg/cpp0x/udlit-macros.C (working copy)
@@ -0,0 +1,31 @@
+// PR c++/80955
+// { dg-do compile { target c++11 } }
+
+#define __STDC_FORMAT_MACROS
+#include <inttypes.h>
+#include <stdio.h>
+#include <string.h>
+
+using size_t = decltype(sizeof(0));
+#define _zero
+#define _ID _xx
+int operator""_zero(const char*, size_t) { return 0; }
+int operator""_ID(const char*, size_t) { return 0; }
+
+int main()
+{
+ int64_t i64 = 123;
+ char buf[100];
+ sprintf(buf, "%"PRId64"abc", i64); // { dg-warning "invalid suffix on
literal" }
+ return strcmp(buf, "123abc")
+ + ""_zero
+ + "bob"_zero
+ + R"#(raw
+ string)#"_zero
+ + "xx"_ID
+ + ""_ID
+ + R"AA(another
+ raw
+ string)AA"_ID;
+}
+
Index: libcpp/lex.c
===================================================================
--- libcpp/lex.c (revision 254048)
+++ libcpp/lex.c (working copy)
@@ -1576,14 +1576,17 @@
/* Returns true if a macro has been defined.
+ If no_underscore is true, check that the macro
+ name does not start with underscore.
This might not work if compile with -save-temps,
or preprocess separately from compilation. */
static bool
-is_macro(cpp_reader *pfile, const uchar *base)
+is_macro(cpp_reader *pfile, const uchar *base, bool no_underscore)
{
const uchar *cur = base;
- if (! ISIDST (*cur))
+ bool invalid_ident = (no_underscore ? ! ISALPHA (*cur) : ! ISIDST (*cur));
+ if (invalid_ident)
return false;
unsigned int hash = HT_HASHSTEP (0, *cur);
++cur;
@@ -1872,7 +1875,7 @@
a string literal it could be parsed as a C++11 user-defined string
literal thus breaking the program.
Try to identify macros with is_macro. A warning is issued. */
- if (is_macro (pfile, cur))
+ if (is_macro (pfile, cur, true))
{
/* Raise a warning, but do not consume subsequent tokens. */
if (CPP_OPTION (pfile, warn_literal_suffix) && !pfile->state.skipping)
@@ -2002,7 +2005,7 @@
a string literal it could be parsed as a C++11 user-defined string
literal thus breaking the program.
Try to identify macros with is_macro. A warning is issued. */
- if (is_macro (pfile, cur))
+ if (is_macro (pfile, cur, true))
{
/* Raise a warning, but do not consume subsequent tokens. */
if (CPP_OPTION (pfile, warn_literal_suffix) && !pfile->state.skipping)
@@ -2023,7 +2026,7 @@
}
}
else if (CPP_OPTION (pfile, cpp_warn_cxx11_compat)
- && is_macro (pfile, cur)
+ && is_macro (pfile, cur, false)
&& !pfile->state.skipping)
cpp_warning_with_line (pfile, CPP_W_CXX11_COMPAT,
token->src_loc, 0, "C++11 requires a space "
