diff mbox

[jit] Use ISALPHA and ISALNUM rather than writing our own

Message ID 1415202488-1461-1-git-send-email-dmalcolm@redhat.com
State New
Headers show

Commit Message

David Malcolm Nov. 5, 2014, 3:48 p.m. UTC 
On Tue, 2014-11-04 at 14:39 -0700, Jeff Law wrote:
> On 11/04/14 09:57, David Malcolm wrote:
> >>> +#define IS_ASCII_DIGIT(CHAR) \
> >>> +  ((CHAR) >= '0' && (CHAR) <='9')
> >>> +
> >>> +#define IS_ASCII_ALNUM(CHAR) \
> >>> +  (IS_ASCII_ALPHA (CHAR) || IS_ASCII_DIGIT (CHAR))
> >> Can't we rely on the C library to give us equivalents?
> >
> > I've been burned in the past by the C library using locales, in
> > particular the two lowercase "i" variants in Turkish.
> >
> > These macros are used by gcc_jit_context_new_function to enforce C's
> > naming restrictions, to avoid errors from the assembler.  The comment I
> > put there was:
> >
> >    /* The assembler can only handle certain names, so for now, enforce
> >       C's rules for identifiers upon the name.
> >       Eventually we'll need some way to interact with e.g. C++ name mangling.  */
> >
> > Am I right in thinking that for the assembler we need to enforce the C
> > naming rules specifically on *ASCII*.
> >
> > (clearly another comment is needed here).
> I guess you've got to do it somewhere.  Presumably there isn't something 
> already in GCC that enforces an input character set?  I guess I just 
> dislike seeing something that feels like it ought to already be available.

It turns out that locale-independent tests for this did already exist in
libiberty, in safe-ctype.h, so I've committed this to the jit branch:

gcc/jit/ChangeLog.jit:
	* libgccjit.c: Include safe-ctype.h from libiberty.
	(IS_ASCII_ALPHA): Delete.
	(IS_ASCII_DIGIT): Delete.
	(IS_ASCII_ALNUM): Delete.
	(gcc_jit_context_new_function): Replace use of IS_ASCII_ALPHA and
	IS_ASCII_ALNUM with ISALPHA and ISALNUM respectively, from
	libiberty.
---
 gcc/jit/ChangeLog.jit | 10 ++++++++++
 gcc/jit/libgccjit.c   | 24 +++++++-----------------
 2 files changed, 17 insertions(+), 17 deletions(-)

Comments

Jeff Law Nov. 5, 2014, 8:01 p.m. UTC | #1 
On 11/05/14 08:48, David Malcolm wrote:
> On Tue, 2014-11-04 at 14:39 -0700, Jeff Law wrote:
>> On 11/04/14 09:57, David Malcolm wrote:
>>>>> +#define IS_ASCII_DIGIT(CHAR) \
>>>>> +  ((CHAR) >= '0' && (CHAR) <='9')
>>>>> +
>>>>> +#define IS_ASCII_ALNUM(CHAR) \
>>>>> +  (IS_ASCII_ALPHA (CHAR) || IS_ASCII_DIGIT (CHAR))
>>>> Can't we rely on the C library to give us equivalents?
>>>
>>> I've been burned in the past by the C library using locales, in
>>> particular the two lowercase "i" variants in Turkish.
>>>
>>> These macros are used by gcc_jit_context_new_function to enforce C's
>>> naming restrictions, to avoid errors from the assembler.  The comment I
>>> put there was:
>>>
>>>     /* The assembler can only handle certain names, so for now, enforce
>>>        C's rules for identifiers upon the name.
>>>        Eventually we'll need some way to interact with e.g. C++ name mangling.  */
>>>
>>> Am I right in thinking that for the assembler we need to enforce the C
>>> naming rules specifically on *ASCII*.
>>>
>>> (clearly another comment is needed here).
>> I guess you've got to do it somewhere.  Presumably there isn't something
>> already in GCC that enforces an input character set?  I guess I just
>> dislike seeing something that feels like it ought to already be available.
>
> It turns out that locale-independent tests for this did already exist in
> libiberty, in safe-ctype.h, so I've committed this to the jit branch:
>
> gcc/jit/ChangeLog.jit:
> 	* libgccjit.c: Include safe-ctype.h from libiberty.
> 	(IS_ASCII_ALPHA): Delete.
> 	(IS_ASCII_DIGIT): Delete.
> 	(IS_ASCII_ALNUM): Delete.
> 	(gcc_jit_context_new_function): Replace use of IS_ASCII_ALPHA and
> 	IS_ASCII_ALNUM with ISALPHA and ISALNUM respectively, from
> 	libiberty.
Excellent.  Thanks for the cleanup.

Jeff
diff mbox

Patch

diff --git a/gcc/jit/ChangeLog.jit b/gcc/jit/ChangeLog.jit
index 90fccdb..3d6361c 100644
--- a/gcc/jit/ChangeLog.jit
+++ b/gcc/jit/ChangeLog.jit
@@ -1,3 +1,13 @@ 
+2014-11-05  David Malcolm  <dmalcolm@redhat.com>
+
+	* libgccjit.c: Include safe-ctype.h from libiberty.
+	(IS_ASCII_ALPHA): Delete.
+	(IS_ASCII_DIGIT): Delete.
+	(IS_ASCII_ALNUM): Delete.
+	(gcc_jit_context_new_function): Replace use of IS_ASCII_ALPHA and
+	IS_ASCII_ALNUM with ISALPHA and ISALNUM respectively, from
+	libiberty.
+
 2014-10-30  David Malcolm  <dmalcolm@redhat.com>
 
 	* dummy-frontend.c (jit_langhook_init): Remove some dead code.
diff --git a/gcc/jit/libgccjit.c b/gcc/jit/libgccjit.c
index 286a85e..d9f259e 100644
--- a/gcc/jit/libgccjit.c
+++ b/gcc/jit/libgccjit.c
@@ -22,24 +22,12 @@  along with GCC; see the file COPYING3.  If not see
 #include "system.h"
 #include "coretypes.h"
 #include "opts.h"
+#include "safe-ctype.h"
 
 #include "libgccjit.h"
 #include "jit-common.h"
 #include "jit-recording.h"
 
-#define IS_ASCII_ALPHA(CHAR) \
-  (					\
-    ((CHAR) >= 'a' && (CHAR) <='z')	\
-    ||					\
-    ((CHAR) >= 'A' && (CHAR) <= 'Z')	\
-  )
-
-#define IS_ASCII_DIGIT(CHAR) \
-  ((CHAR) >= '0' && (CHAR) <='9')
-
-#define IS_ASCII_ALNUM(CHAR) \
-  (IS_ASCII_ALPHA (CHAR) || IS_ASCII_DIGIT (CHAR))
-
 struct gcc_jit_context : public gcc::jit::recording::context
 {
   gcc_jit_context (gcc_jit_context *parent_ctxt) :
@@ -589,13 +577,15 @@  gcc_jit_context_new_function (gcc_jit_context *ctxt,
   RETURN_NULL_IF_FAIL (return_type, ctxt, loc, "NULL return_type");
   RETURN_NULL_IF_FAIL (name, ctxt, loc, "NULL name");
   /* The assembler can only handle certain names, so for now, enforce
-     C's rules for identiers upon the name.
-     Eventually we'll need some way to interact with e.g. C++ name mangling.  */
+     C's rules for identiers upon the name, using ISALPHA and ISALNUM
+     from safe-ctype.h to ignore the current locale.
+     Eventually we'll need some way to interact with e.g. C++ name
+     mangling.  */
   {
     /* Leading char: */
     char ch = *name;
     RETURN_NULL_IF_FAIL_PRINTF2 (
-	IS_ASCII_ALPHA (ch) || ch == '_',
+	ISALPHA (ch) || ch == '_',
 	ctxt, loc,
 	"name \"%s\" contains invalid character: '%c'",
 	name, ch);
@@ -603,7 +593,7 @@  gcc_jit_context_new_function (gcc_jit_context *ctxt,
     for (const char *ptr = name + 1; (ch = *ptr); ptr++)
       {
 	RETURN_NULL_IF_FAIL_PRINTF2 (
-	  IS_ASCII_ALNUM (ch) || ch == '_',
+	  ISALNUM (ch) || ch == '_',
 	  ctxt, loc,
 	  "name \"%s\" contains invalid character: '%c'",
 	  name, ch);