I'm not entirely sure what to make of this series. There seem to be good
bits in there but also some things I find questionable. I'll add some
comments on things that occur to me.
On 10/06/2015 01:28 AM, Mikhail Maltsev wrote:
* include/line-map.h: Fix use of ENABLE_CHECKING.
Fix how? What's wrong with it?
/* Sanity-checks are dependent on command-line options, so it is
called as a subroutine of cpp_read_main_file (). */
-#if ENABLE_CHECKING
+#if CHECKING_P
static void sanity_checks (cpp_reader *);
static void sanity_checks (cpp_reader *pfile)
{
Ok, this one seems to be a real problem (should have been #ifdef), but...
-#ifdef ENABLE_CHECKING
+#if CHECKING_P
I fail to see the point of this change.
-#ifdef ENABLE_CHECKING
- if (kind == MACRO_ARG_TOKEN_STRINGIFIED
- || !track_macro_exp_p)
- /* We can't set the location of a stringified argument
- token and we can't set any location if we aren't tracking
- macro expansion locations. */
- abort ();
-#endif
+ /* We can't set the location of a stringified argument
+ token and we can't set any location if we aren't tracking
+ macro expansion locations. */
+ gcc_checking_assert (kind != MACRO_ARG_TOKEN_STRINGIFIED
+ && track_macro_exp_p);
This kind of change seems good. I think the patch series would benefit
if it was separated thematically rather than by sets of files. I.e.,
merge all changes like this one into one patch or maybe a few if it
grows too large.
+/* Redefine abort to report an internal error w/o coredump, and
+ reporting the location of the error in the source file. */
+extern void fancy_abort (const char *, int, const char *) ATTRIBUTE_NORETURN;
+#define abort() fancy_abort (__FILE__, __LINE__, __FUNCTION__)
+
+/* Use gcc_assert(EXPR) to test invariants. */
+#if ENABLE_ASSERT_CHECKING
+#define gcc_assert(EXPR) \
+ ((void)(!(EXPR) ? fancy_abort (__FILE__, __LINE__, __FUNCTION__), 0 : 0))
+#elif (GCC_VERSION >= 4005)
+#define gcc_assert(EXPR) \
+ ((void)(__builtin_expect (!(EXPR), 0) ? __builtin_unreachable (), 0 : 0))
+#else
+/* Include EXPR, so that unused variable warnings do not occur. */
+#define gcc_assert(EXPR) ((void)(0 && (EXPR)))
+#endif
Probably a good thing, but it looks like libcpp has grown its own
variant linemap_assert; we should check whether that can be replaced.
Also, the previous patch already introduces a use of gcc_assert, or at
least a reference to it, and it's only defined here. The two
modifications of libcpp/system.h should probably be merged into one.
Bernd
