diff mbox

[2/9] ENABLE_CHECKING refactoring: libcpp

Message ID 56243FBF.7000303@gmail.com
State New
Headers show

Commit Message

Mikhail Maltsev Oct. 19, 2015, 12:56 a.m. UTC 
On 10/12/2015 11:57 PM, Jeff Law wrote:
> On 10/06/2015 06:40 AM, Bernd Schmidt wrote:
>> 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.
> Maybe we should start pulling out the bits that we think are ready & good and
> start installing them independently.
> 
(snip)

>> 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.
> Agreed.
> 
> jeff

I moved the 'libcpp/system.h' parts into the first patch of the series. As for
replacing linemap_assert, I hope it can be done separately.

Comments

Jeff Law Oct. 21, 2015, 10:22 p.m. UTC | #1 
On 10/18/2015 06:56 PM, Mikhail Maltsev wrote:
> On 10/12/2015 11:57 PM, Jeff Law wrote:
>> >On 10/06/2015 06:40 AM, Bernd Schmidt wrote:
>>> >>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.
>> >Maybe we should start pulling out the bits that we think are ready & good and
>> >start installing them independently.
>> >
> (snip)
>
>>> >>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.
>> >Agreed.
>> >
>> >jeff
> I moved the 'libcpp/system.h' parts into the first patch of the series. As for
> replacing linemap_assert, I hope it can be done separately.
Thanks.  I went ahead and tested, then installed patch #2.

And yes, I think replacing linemap_assert can be done as a follow-up.


jeff
diff mbox

Patch

From 5bb52360b2a065015cb081b4528f2f9295c326d6 Mon Sep 17 00:00:00 2001
From: Mikhail Maltsev <maltsevm@gmail.com>
Date: Tue, 22 Sep 2015 02:51:31 +0300
Subject: [PATCH 2/7] libcpp - v2

---
 libcpp/include/line-map.h |  2 +-
 libcpp/init.c             |  2 +-
 libcpp/macro.c            | 38 +++++++++++++++-----------------------
 3 files changed, 17 insertions(+), 25 deletions(-)

diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index bc747c1..e718fc2 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -272,7 +272,7 @@  struct GTY((tag ("2"))) line_map_macro : public line_map {
   source_location expansion;
 };
 
-#if defined ENABLE_CHECKING && (GCC_VERSION >= 2007)
+#if CHECKING_P && (GCC_VERSION >= 2007)
 
 /* Assertion macro to be used in line-map code.  */
 #define linemap_assert(EXPR)                  \
diff --git a/libcpp/init.c b/libcpp/init.c
index 2d5626f..0419e95 100644
--- a/libcpp/init.c
+++ b/libcpp/init.c
@@ -535,7 +535,7 @@  cpp_init_builtins (cpp_reader *pfile, int hosted)
 
 /* 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)
 {
diff --git a/libcpp/macro.c b/libcpp/macro.c
index 786c21b..60a0753 100644
--- a/libcpp/macro.c
+++ b/libcpp/macro.c
@@ -73,7 +73,7 @@  struct macro_arg_token_iter
      -ftrack-macro-expansion is used this location tracks loci across
      macro expansion.  */
   const source_location *location_ptr;
-#ifdef ENABLE_CHECKING
+#if CHECKING_P
   /* The number of times the iterator went forward. This useful only
      when checking is enabled.  */
   size_t num_forwards;
@@ -1310,14 +1310,11 @@  set_arg_token (macro_arg *arg, const cpp_token *token,
 
   if (loc != NULL)
     {
-#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);
       *loc = location;
     }
 }
@@ -1403,7 +1400,7 @@  macro_arg_token_iter_init (macro_arg_token_iter *iter,
   iter->location_ptr = NULL;
   if (track_macro_exp_p)
     iter->location_ptr = get_arg_token_location (arg, kind);
-#ifdef ENABLE_CHECKING
+#if CHECKING_P
   iter->num_forwards = 0;
   if (track_macro_exp_p
       && token_ptr != NULL
@@ -1428,14 +1425,14 @@  macro_arg_token_iter_forward (macro_arg_token_iter *it)
 	it->location_ptr++;
       break;
     case MACRO_ARG_TOKEN_STRINGIFIED:
-#ifdef ENABLE_CHECKING
+#if CHECKING_P
       if (it->num_forwards > 0)
 	abort ();
 #endif
       break;
     }
 
-#ifdef ENABLE_CHECKING
+#if CHECKING_P
   it->num_forwards++;
 #endif
 }
@@ -1444,7 +1441,7 @@  macro_arg_token_iter_forward (macro_arg_token_iter *it)
 static const cpp_token *
 macro_arg_token_iter_get_token (const macro_arg_token_iter *it)
 {
-#ifdef ENABLE_CHECKING
+#if CHECKING_P
   if (it->kind == MACRO_ARG_TOKEN_STRINGIFIED
       && it->num_forwards > 0)
     abort ();
@@ -1458,7 +1455,7 @@  macro_arg_token_iter_get_token (const macro_arg_token_iter *it)
 static source_location
 macro_arg_token_iter_get_location (const macro_arg_token_iter *it)
 {
-#ifdef ENABLE_CHECKING
+#if CHECKING_P
   if (it->kind == MACRO_ARG_TOKEN_STRINGIFIED
       && it->num_forwards > 0)
     abort ();
@@ -2144,11 +2141,9 @@  tokens_buff_add_token (_cpp_buff *buffer,
 static void
 alloc_expanded_arg_mem (cpp_reader *pfile, macro_arg *arg, size_t capacity)
 {
-#ifdef ENABLE_CHECKING
-  if (arg->expanded != NULL
-      || arg->expanded_virt_locs != NULL)
-    abort ();
-#endif
+  gcc_checking_assert (arg->expanded == NULL
+		       && arg->expanded_virt_locs == NULL);
+
   arg->expanded = XNEWVEC (const cpp_token *, capacity);
   if (CPP_OPTION (pfile, track_macro_expansion))
     arg->expanded_virt_locs = XNEWVEC (source_location, capacity);
@@ -2709,10 +2704,7 @@  _cpp_backup_tokens (cpp_reader *pfile, unsigned int count)
 	    {
 	      macro_context *m = pfile->context->c.mc;
 	      m->cur_virt_loc--;
-#ifdef ENABLE_CHECKING
-	      if (m->cur_virt_loc < m->virt_locs)
-		abort ();
-#endif
+	      gcc_checking_assert (m->cur_virt_loc >= m->virt_locs);
 	    }
 	  else
 	    abort ();
-- 
2.1.4