tinyos / nesc

Master nesc repository
GNU General Public License v2.0
100 stars 53 forks source link

Add unescaped restrict keyword #42

Closed dgay42 closed 6 years ago

dgay42 commented 6 years ago

There's already a __restrict keyword...

tgtakaoka commented 6 years ago

Because restrict is the official keyword defined in C99, changing a definition in qualifiers.h to restrict and adding __restrict to c-parse.gperf seems more reasonable.

cire831 commented 6 years ago

how hard is the change?

simple text substitution?

I can try it both ways but don't understand myself what is happening.

On Wed, May 2, 2018 at 11:12 PM, Tadashi G. Takaoka < notifications@github.com> wrote:

Because restrict is the official keyword defined in C99 http://en.cppreference.com/w/c/language/restrict, changing a definition in qualifiers.h https://github.com/tinyos/nesc/blob/b99bddcb0219df15a10f4db3e3dafe48852c2776/src/qualifiers.h#L3 to restrict and adding __restrict to c-parse.gperf https://github.com/tinyos/nesc/blob/master/src/c-parse.gperf seems more reasonable.

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/tinyos/nesc/pull/42#issuecomment-386199754, or mute the thread https://github.com/notifications/unsubscribe-auth/AAY46axb9D52nnhkCQ1Iku2Pd2WVJ0f8ks5tup-5gaJpZM4TC_fw .

-- Eric B. Decker Senior (over 50 :-) Researcher

tgtakaoka commented 6 years ago

What I tried is a simple text substitution just like you mentioned. I confirmed that modified nesc with gcc 7.3 based TI msp430 compiler can handle restrict keyword correctly.

diff --git a/src/c-parse.gperf b/src/c-parse.gperf
index 1789383..8639c65 100644
--- a/src/c-parse.gperf
+++ b/src/c-parse.gperf
@@ -43,6 +43,7 @@ __inline__, SCSPEC, RID_INLINE
 __label__, LABEL, NORID
 __real, REALPART, NORID
 __real__, REALPART, NORID
+__restrict, TYPE_QUAL, restrict_qualifier
 __signed, TYPESPEC, RID_SIGNED
 __signed__, TYPESPEC, RID_SIGNED
 __typeof, TYPEOF, NORID
diff --git a/src/qualifiers.h b/src/qualifiers.h
index d4eceb0..194200c 100644
--- a/src/qualifiers.h
+++ b/src/qualifiers.h
@@ -1,3 +1,3 @@
 Q(const,       TYPE_QUAL,      const_qualifier,        2)
 Q(volatile,    TYPE_QUAL,      volatile_qualifier,     4)
-Q(__restrict,  TYPE_QUAL,      restrict_qualifier,     8)
+Q(restrict,    TYPE_QUAL,      restrict_qualifier,     8)
dgay42 commented 6 years ago

I strongly suggest changing the definition in qualifiers.h will make restrict in input files produce 'restrict' in the output. This may break earlier gcc versions. (the dual of this is that 'restrict' will produce 'restrict' in the output - this should be fine as long as gcc continues to accept __restrict).

And, Eric: you were supposed to review this pull request ;)

cire831 commented 6 years ago

On Thu, May 3, 2018 at 9:53 PM, dgay42 notifications@github.com wrote:

I strongly suggest changing the definition in qualifiers.h will make restrict in input files produce 'restrict' in the output. This may break earlier gcc versions. (the dual of this is that 'restrict' will produce 'restrict' in the output - this should be fine as long as gcc continues to accept __restrict).

And, Eric: you were supposed to review this pull request ;)

I know. trying to get a freeze to happen. understaffed overworked :-)

I suspect you know the feeling :-)

I'll get to it next week.

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/tinyos/nesc/pull/42#issuecomment-386505458, or mute the thread https://github.com/notifications/unsubscribe-auth/AAY46SE7W37QSDQHOwCKjN6JloHTnDGjks5tu961gaJpZM4TC_fw .

-- Eric B. Decker Senior (over 50 :-) Researcher