Re: gfortran using libcpp/internal.h (was Re: [C++ Patch] PR 50956)
On Thu, Nov 3, 2011 at 4:03 PM, Daniel Franke wrote: > 2011/11/3 Richard Guenther : >> On Thu, Nov 3, 2011 at 3:46 PM, Jason Merrill wrote: >>> On 11/03/2011 09:51 AM, Tom Tromey wrote: > > "Paolo" == Paolo Carlini writes: Paolo> By the way, I have no idea why I didn't see it, only Fortran includes Paolo> [internal.h] in the *libcpp* directory? Anyway, I'm proceeding to Paolo> bootstrap the attached. It really isn't ok to do this. That header is supposedly private to libcpp. >>> >>> That include seems to have been there since cpp.c was first checked in by >>> Daniel Franke. >> >> Most of the errors go away with replacing it with #include "cpplib.h", >> remaining: >> >> ,,, >> k/gcc/fortran/cpp.c -o fortran/cpp.o >> /space/rguenther/src/svn/trunk/gcc/fortran/cpp.c: In function >> ‘scan_translation_unit_trad’: >> /space/rguenther/src/svn/trunk/gcc/fortran/cpp.c:769: warning: >> implicit declaration of function ‘_cpp_read_logical_line_trad’ >> >> but then we dereference struct cpp_reader: >> >> /space/rguenther/src/svn/trunk/gcc/fortran/cpp.c:771: error: >> dereferencing pointer to incomplete type >> /space/rguenther/src/svn/trunk/gcc/fortran/cpp.c:771: error: >> dereferencing pointer to incomplete type >> /space/rguenther/src/svn/trunk/gcc/fortran/cpp.c:772: error: >> dereferencing pointer to incomplete type > > Hi all. > > This was then mimicking (actually mostly duplicating), the > implementation of the C frontend. In particular around the usage of > scan_translation_unit_trad and associated friends. I wasn't happy to > do this, but to get at least some functionality done without > rebuilding everything, it was required. > > If you go back to the 4.6 branch you will find similar code in > c-family/c-ppoutput.c. It still includes internal.h on trunk it seems ... Richard. > Regards > > Daniel >
Re: gfortran using libcpp/internal.h (was Re: [C++ Patch] PR 50956)
On Thu, Nov 3, 2011 at 3:46 PM, Jason Merrill wrote: > On 11/03/2011 09:51 AM, Tom Tromey wrote: >>> >>> "Paolo" == Paolo Carlini writes: >> >> Paolo> By the way, I have no idea why I didn't see it, only Fortran >> includes >> Paolo> [internal.h] in the *libcpp* directory? Anyway, I'm proceeding to >> Paolo> bootstrap the attached. >> >> It really isn't ok to do this. That header is supposedly private to >> libcpp. > > That include seems to have been there since cpp.c was first checked in by > Daniel Franke. Most of the errors go away with replacing it with #include "cpplib.h", remaining: ,,, k/gcc/fortran/cpp.c -o fortran/cpp.o /space/rguenther/src/svn/trunk/gcc/fortran/cpp.c: In function ‘scan_translation_unit_trad’: /space/rguenther/src/svn/trunk/gcc/fortran/cpp.c:769: warning: implicit declaration of function ‘_cpp_read_logical_line_trad’ but then we dereference struct cpp_reader: /space/rguenther/src/svn/trunk/gcc/fortran/cpp.c:771: error: dereferencing pointer to incomplete type /space/rguenther/src/svn/trunk/gcc/fortran/cpp.c:771: error: dereferencing pointer to incomplete type /space/rguenther/src/svn/trunk/gcc/fortran/cpp.c:772: error: dereferencing pointer to incomplete type ... Richard. > Jason > >
gfortran using libcpp/internal.h (was Re: [C++ Patch] PR 50956)
On 11/03/2011 09:51 AM, Tom Tromey wrote: "Paolo" == Paolo Carlini writes: Paolo> By the way, I have no idea why I didn't see it, only Fortran includes Paolo> [internal.h] in the *libcpp* directory? Anyway, I'm proceeding to Paolo> bootstrap the attached. It really isn't ok to do this. That header is supposedly private to libcpp. That include seems to have been there since cpp.c was first checked in by Daniel Franke. Jason
Re: [C++ Patch] PR 50956
Hi, > It really isn't ok to do this. That header is supposedly private to > libcpp. Eh, eh looks like somebody wandered around and found a few handy helper functions ;) > Paolo
Re: [C++ Patch] PR 50956
> "Paolo" == Paolo Carlini writes: Paolo> By the way, I have no idea why I didn't see it, only Fortran includes Paolo> that header in the *libcpp* directory? Anyway, I'm proceeding to Paolo> bootstrap the attached. It really isn't ok to do this. That header is supposedly private to libcpp. Tom
Re: [C++ Patch] PR 50956
this actually.
Paolo.
/
Index: internal.h
===
--- internal.h (revision 180785)
+++ internal.h (working copy)
@@ -739,8 +739,8 @@
static inline int ustrncmp (const unsigned char *, const unsigned char *,
size_t);
static inline size_t ustrlen (const unsigned char *);
-static inline unsigned char *uxstrdup (const unsigned char *);
-static inline unsigned char *ustrchr (const unsigned char *, int);
+static inline const unsigned char *uxstrdup (const unsigned char *);
+static inline const unsigned char *ustrchr (const unsigned char *, int);
static inline int ufputs (const unsigned char *, FILE *);
/* Use a const char for the second parameter since it is usually a literal. */
@@ -770,16 +770,16 @@
return strlen ((const char *)s1);
}
-static inline unsigned char *
+static inline const unsigned char *
uxstrdup (const unsigned char *s1)
{
- return (unsigned char *) xstrdup ((const char *)s1);
+ return (const unsigned char *) xstrdup ((const char *)s1);
}
-static inline unsigned char *
+static inline const unsigned char *
ustrchr (const unsigned char *s1, int c)
{
- return (unsigned char *) strchr ((const char *)s1, c);
+ return (const unsigned char *) strchr ((const char *)s1, c);
}
static inline int
Re: [C++ Patch] PR 50956
On 11/02/2011 08:44 PM, Paolo Carlini wrote:
On 11/02/2011 08:12 PM, Jason Merrill wrote:
Another bootstrap issue:
In file included from /home/jason/src/trunk/gcc/fortran/cpp.c:35:0:
/home/jason/src/trunk/gcc/fortran/../../libcpp/internal.h: In
function ‘unsigned char* ustrchr(const unsigned char*, int)’:
/home/jason/src/trunk/gcc/fortran/../../libcpp/internal.h:782:55:
error: cast from type ‘const char*’ to type ‘unsigned char*’ casts
away qualifiers [-Werror=cast-qual]
Argh, I'll fix it momentarily.
By the way, I have no idea why I didn't see it, only Fortran includes
that header in the *libcpp* directory? Anyway, I'm proceeding to
bootstrap the attached.
Paolo.
/
Index: internal.h
===
--- internal.h (revision 180785)
+++ internal.h (working copy)
@@ -770,16 +770,16 @@ ustrlen (const unsigned char *s1)
return strlen ((const char *)s1);
}
-static inline unsigned char *
+static inline const unsigned char *
uxstrdup (const unsigned char *s1)
{
- return (unsigned char *) xstrdup ((const char *)s1);
+ return (const unsigned char *) xstrdup ((const char *)s1);
}
-static inline unsigned char *
+static inline const unsigned char *
ustrchr (const unsigned char *s1, int c)
{
- return (unsigned char *) strchr ((const char *)s1, c);
+ return (const unsigned char *) strchr ((const char *)s1, c);
}
static inline int
Re: [C++ Patch] PR 50956
On 11/02/2011 08:12 PM, Jason Merrill wrote: Another bootstrap issue: In file included from /home/jason/src/trunk/gcc/fortran/cpp.c:35:0: /home/jason/src/trunk/gcc/fortran/../../libcpp/internal.h: In function ‘unsigned char* ustrchr(const unsigned char*, int)’: /home/jason/src/trunk/gcc/fortran/../../libcpp/internal.h:782:55: error: cast from type ‘const char*’ to type ‘unsigned char*’ casts away qualifiers [-Werror=cast-qual] Argh, I'll fix it momentarily. Paolo.
Re: [C++ Patch] PR 50956
Another bootstrap issue: In file included from /home/jason/src/trunk/gcc/fortran/cpp.c:35:0: /home/jason/src/trunk/gcc/fortran/../../libcpp/internal.h: In function ‘unsigned char* ustrchr(const unsigned char*, int)’: /home/jason/src/trunk/gcc/fortran/../../libcpp/internal.h:782:55: error: cast from type ‘const char*’ to type ‘unsigned char*’ casts away qualifiers [-Werror=cast-qual]
Re: [C++ Patch] PR 50956
On 11/02/2011 07:26 PM, Jason Merrill wrote: On 11/02/2011 01:45 PM, Paolo Carlini wrote: ... ehm, we use -Wcast-qual during the bootstrap, thus if I don't want to break it again, better doing the below too. Seems obvious, by itself. Yep. Great, all done. Note for the accidental reader: it's indeed obvious, but only if you are used to the C++ memchr, the two overloads. For the targets (many outside Linux) not providing those (via libc hooks) I would not have broken the bootstrap ;) Paolo.
Re: [C++ Patch] PR 50956
On 11/02/2011 01:45 PM, Paolo Carlini wrote: ... ehm, we use -Wcast-qual during the bootstrap, thus if I don't want to break it again, better doing the below too. Seems obvious, by itself. Yep. Jason
Re: [C++ Patch] PR 50956
... ehm, we use -Wcast-qual during the bootstrap, thus if I don't want to break it again, better doing the below too. Seems obvious, by itself. Paolo. // 2011-11-02 Paolo Carlini PR c++/50956 * builtins.c (fold_builtin_memchr): Fix cast. Index: builtins.c === --- builtins.c (revision 180778) +++ builtins.c (working copy) @@ -8427,7 +8427,7 @@ fold_builtin_memchr (location_t loc, tree arg1, tr if (target_char_cast (arg2, &c)) return NULL_TREE; - r = (char *) memchr (p1, c, tree_low_cst (len, 1)); + r = (const char *) memchr (p1, c, tree_low_cst (len, 1)); if (r == NULL) return build_int_cst (TREE_TYPE (arg1), 0);
Re: [C++ Patch] PR 50956
OK. Jason
[C++ Patch] PR 50956
Hi,
this should restore -Wcast-qual to the 3.4.x (!) behavior, more or less.
The diff seems large but the new code is essentially in the last 5
lines. When fixing this issue the most subtle existing testcase to get
right, IMHO, is c-c++-common/Wcast-qual-1.c. Duplicate warnings should
not be an issue because, in input, the valid_p parameter of
build_const_cast_1 essentially plays the role of c_cast_p afterwards and
all the following check_for_casting_away_constness calls are protected
by !c_cast_p.
Tested x86_64-linux.
Ok for mainline?
Thanks,
Paolo.
/
/cp
2011-11-02 Paolo Carlini
PR c++/50956
* typeck.c (build_const_cast_1): Fix -Wcast-qual for false
comp_ptr_ttypes_const.
/testsuite
2011-11-02 Paolo Carlini
PR c++/50956
* g++.dg/warn/Wcast-qual2.C: New.
Index: testsuite/g++.dg/warn/Wcast-qual2.C
===
--- testsuite/g++.dg/warn/Wcast-qual2.C (revision 0)
+++ testsuite/g++.dg/warn/Wcast-qual2.C (revision 0)
@@ -0,0 +1,4 @@
+// PR c++/50956
+// { dg-options "-Wcast-qual" }
+
+void* p = (void*)"txt"; // { dg-warning "cast" }
Index: cp/typeck.c
===
--- cp/typeck.c (revision 180778)
+++ cp/typeck.c (working copy)
@@ -6345,34 +6345,41 @@ build_const_cast_1 (tree dst_type, tree expr, tsub
return error_mark_node;
}
- if ((TYPE_PTR_P (src_type) || TYPE_PTRMEM_P (src_type))
- && comp_ptr_ttypes_const (dst_type, src_type))
+ if (TYPE_PTR_P (src_type) || TYPE_PTRMEM_P (src_type))
{
- if (valid_p)
+ if (comp_ptr_ttypes_const (dst_type, src_type))
{
- *valid_p = true;
- /* This cast is actually a C-style cast. Issue a warning if
-the user is making a potentially unsafe cast. */
- check_for_casting_away_constness (src_type, dst_type, CAST_EXPR,
- complain);
+ if (valid_p)
+ {
+ *valid_p = true;
+ /* This cast is actually a C-style cast. Issue a warning if
+the user is making a potentially unsafe cast. */
+ check_for_casting_away_constness (src_type, dst_type,
+ CAST_EXPR, complain);
+ }
+ if (reference_type)
+ {
+ expr = cp_build_addr_expr (expr, complain);
+ expr = build_nop (reference_type, expr);
+ return convert_from_reference (expr);
+ }
+ else
+ {
+ expr = decay_conversion (expr);
+ /* build_c_cast puts on a NOP_EXPR to make the result not an
+lvalue. Strip such NOP_EXPRs if VALUE is being used in
+non-lvalue context. */
+ if (TREE_CODE (expr) == NOP_EXPR
+ && TREE_TYPE (expr) == TREE_TYPE (TREE_OPERAND (expr, 0)))
+ expr = TREE_OPERAND (expr, 0);
+ return build_nop (dst_type, expr);
+ }
}
- if (reference_type)
- {
- expr = cp_build_addr_expr (expr, complain);
- expr = build_nop (reference_type, expr);
- return convert_from_reference (expr);
- }
- else
- {
- expr = decay_conversion (expr);
- /* build_c_cast puts on a NOP_EXPR to make the result not an
-lvalue. Strip such NOP_EXPRs if VALUE is being used in
-non-lvalue context. */
- if (TREE_CODE (expr) == NOP_EXPR
- && TREE_TYPE (expr) == TREE_TYPE (TREE_OPERAND (expr, 0)))
- expr = TREE_OPERAND (expr, 0);
- return build_nop (dst_type, expr);
- }
+ else if (valid_p
+ && !at_least_as_qualified_p (TREE_TYPE (dst_type),
+ TREE_TYPE (src_type)))
+ check_for_casting_away_constness (src_type, dst_type, CAST_EXPR,
+ complain);
}
if (complain & tf_error)
