Github issue 156: #156

Modified ncgen to properly handle the L and UL suffixes for integer constants
to keep backward compatibility. Now it is the case the single L suffix
(e.g. 111L) is treated as a 32 bit integer. This makes it consistent with
the fact that NC_LONG (netcdf.h) is an alias for NC_INT.
It is probable that .cdl files now exist that assumed that L prefix was
64 bits. This will cause problems.

Changes:
- ncgen.l - fix lexing involving L/LL suffix.
- misc. test cases: either .cdl files, or ncdump reference files or .c files.
- semantics.c: to fix type inference rules for untyped attributes
- ncdump: the code was dumping constant text using the L prefix.

RELEASE_NOTES.md

@@ -7,12 +7,20 @@ This file contains a high-level description of this package's evolution. Release
 
 ## 4.4.0 Released TBD
 
+* Modified ncgen to properly handle the L and UL suffixes for integer constants
+  to keep backward compatibility. Now it is the case the single L suffix
+  (e.g. 111L) is treated as a 32 bit integer. This makes it consistent with
+  the fact that NC_LONG (netcdf.h) is an alias for NC_INT. Existing .cdl
+  files should be examined for occurrences of the L prefix to ensure that
+  this change will not affect them.
+  (see Github issue 156[https://github.com/Unidata/netcdf-c/issues/156]).
+
 * Updated documentation to reference the new `NodeJS` interface to netcdf4, by Sven Willner.  It is available from [https://www.npmjs.com/package/netcdf4](https://www.npmjs.com/package/netcdf4) or from the GitHub repository at [https://github.com/swillner/netcdf4-js](https://github.com/swillner/netcdf4-js).
 
 * Incorporated pull request https://github.com/Unidata/netcdf-c/pull/150 from Greg Sjaardema to remove the internal hard-wired use of `NC_MAX_DIMS`, instead using a dynamic memory allocation.
 
 ### 4.4.0-RC5 Released - November 11, 2015
-
+ 
 * Added a fix for https://github.com/Unidata/netcdf-c/issues/149, which was reported several times in quick succession within an hour of the RC4 release.
 
 ### 4.4.0-RC4 Released - November 10, 2015

ncdump/Makefile.am

@@ -112,7 +112,7 @@ iter.* \
 tst_nc_test_netcdf4_4_0.cdl tst_mud4.nc tst_mud4.cdl tst_mud4-bc.cdl    \
 tst_ncf213.cdl tst_ncf213.nc tst_h_scalar.cdl tst_h_scalar.nc           \
 tst_mud4_chars.cdl tst_mud4_chars.nc                                    \
-inttags.nc inttags4.nc
+inttags.nc inttags4.nc tst_inttags.cdl tst_inttags4.cdl
 
 # These files all have to be included with the distribution.
 EXTRA_DIST = run_tests.sh tst_64bit.sh tst_output.sh test0.cdl		\

ncdump/inttags4.cdl

@@ -7,6 +7,8 @@ variables:
 	uint ui(d);
 	int64 i64(d);
 	uint64 ui64(d);
+// global attributes:
+		:attrll = -23232244LL, 1214124123423LL, -2353424234LL ;
 data:
 
  ub = 255UB, 255ub, 255u ;

ncdump/ref_inttags4.cdl

@@ -7,6 +7,9 @@ variables:
 	uint ui(d) ;
 	int i64(d) ;
 	uint64 ui64(d) ;
+
+// global attributes:
+		:attrll = -23232244LL, 1214124123423LL, -2353424234LL ;
 data:
 
  ub = 255, 255, 255 ;

ncdump/tst_inttags.sh

@@ -15,6 +15,6 @@ echo "*** creating tst_inttags.cdl from inttags.nc..."
 echo "*** comparing tst_inttags.cdl to ref_inttags.nc..."
 diff -b -w tst_inttags.cdl $srcdir/ref_inttags.cdl
 
-#rm inttags.nc tst_inttags.cdl
+rm inttags.nc tst_inttags.cdl
 
 exit 0

ncdump/tst_inttags4.sh

@@ -16,6 +16,6 @@ echo "*** creating tst_inttags4.cdl from inttags4.nc..."
 echo "*** comparing tst_inttags4.cdl to ref_inttags4.nc..."
 diff -b -w tst_inttags4.cdl $srcdir/ref_inttags4.cdl
 
-#rm inttags4.nc tst_inttags4.cdl
+rm inttags4.nc tst_inttags4.cdl
 
 exit 0

ncgen/cdata.c

@@ -64,10 +64,10 @@ c_constant(Generator* generator, NCConstant* con, Bytebuffer* buf,...)
 	    bbprintf(codetmp,"%lf",con->value.doublev);
 	break;
     case NC_UBYTE:
-        bbprintf(codetmp,"%hhu",con->value.uint8v);
+        bbprintf(codetmp,"%hhuU",con->value.uint8v);
 	break;
     case NC_USHORT:
-	bbprintf(codetmp,"%hu",con->value.uint16v);
+	bbprintf(codetmp,"%huU",con->value.uint16v);
 	break;
     case NC_UINT:
 	bbprintf(codetmp,"%uU",con->value.uint32v);
@@ -76,7 +76,7 @@ c_constant(Generator* generator, NCConstant* con, Bytebuffer* buf,...)
 	bbprintf(codetmp,"%lldLL",con->value.int64v);
 	break;
     case NC_UINT64:
-	bbprintf(codetmp,"%lluLLU",con->value.uint64v);
+	bbprintf(codetmp,"%lluULL",con->value.uint64v);
 	break;
     case NC_ECONST:
 	bbprintf(codetmp,"%s",cname(con->value.enumv));

ncgen/data.c

@@ -153,6 +153,12 @@ srcpeek(Datasrc* ds)
     return NULL;
 }
 
+void
+srcreset(Datasrc* ds)
+{
+    ds->index = 0;
+}
+
 NCConstant*
 srcnext(Datasrc* ds)
 {

ncgen/data.h

@@ -122,6 +122,8 @@ void srcsetfill(Datasrc* ds, Datalist* list);
 NCConstant* srcnext(Datasrc*);
 int srcmore(Datasrc*);
 int srcline(Datasrc* ds);
+void srcreset(Datasrc* ds);
+#define srclen(s) ((s)==NULL?0:(s)->length)
 
 #define islistconst(con) ((con)!=NULL && (con)->nctype == NC_COMPOUND)
 #define isfillconst(con) ((con)!=NULL && (con)->nctype == NC_FILLVALUE)

ncgen/ncgen.1

@@ -490,7 +490,7 @@ except that type suffixes must be appended to shorts and floats to
 distinguish them from longs and doubles.
 .LP
 A \fIbyte\fP constant is represented by
- an integer constant with a `b' (or
+an integer constant with a `b' (or
 `B') appended.  In the old netCDF-2 API, byte constants could also be
 represented using single characters or standard C character escape
 sequences such as `a' or `\n'.  This is still supported for backward

ncgen/ncgen.l

@@ -394,7 +394,7 @@ NIL|nil|Nil {
 		    }			
 		    /* drop the tag from the input text */
 		    ncgtext[slen - strlen(stag)] = '\0';
-		    hasU = (tag >= NC_UBYTE);
+		    hasU = isuinttype(tag);
 		    if(!tstdecimal(c)) {
 			pos = ncgtext+1;
 			isneg = (c == '-');
@@ -691,17 +691,18 @@ downconvert(unsigned long long uint64, int* tagp, int isneg, int hasU)
 {
     nc_type nct = NC_NAT;
     int tag = *tagp;
-    int bit64 = (uint64 >> 63);
-    int allones = (uint64 == 0xffffffffffffffffULL);
+    int bit63set = (uint64 >> 63);
     long long int64 = *((long long*)&uint64);
 
     if(isneg && hasU) {
 	return (*tagp = NC_NAT);
     }
-    /* Special cases: all (u)int64 values */
-    if(allones) {
+    /* To simplify the code, we look for special case of NC_UINT64
+       constants that will not fit into an NC_INT64 constant.
+     */
+    if(tag == NC_UINT64 && bit63set) {
         uint64_val = uint64;
-	return (*tagp = NC_UINT64);
+	return tag;
     }
     /* At this point we need deal only with int64 value */
     /* Apply the isneg */
@@ -725,7 +726,7 @@ downconvert(unsigned long long uint64, int* tagp, int isneg, int hasU)
 	}
         goto done;
     }
-    if(tag >= NC_UBYTE && int64 < 0)
+    if(isuinttype(tag) && int64 < 0)
 	goto outofrange;
     switch (tag) {
     case NC_UBYTE:

ncgen/ncgenl.c

@@ -1794,7 +1794,7 @@ YY_RULE_SETUP
 		    }			
 		    /* drop the tag from the input text */
 		    ncgtext[slen - strlen(stag)] = '\0';
-		    hasU = (tag >= NC_UBYTE);
+		    hasU = isuinttype(tag);
 		    if(!tstdecimal(c)) {
 			pos = ncgtext+1;
 			isneg = (c == '-');
@@ -3126,17 +3126,18 @@ downconvert(unsigned long long uint64, int* tagp, int isneg, int hasU)
 {
     nc_type nct = NC_NAT;
     int tag = *tagp;
-    int bit64 = (uint64 >> 63);
-    int allones = (uint64 == 0xffffffffffffffffULL);
+    int bit63set = (uint64 >> 63);
     long long int64 = *((long long*)&uint64);
 
     if(isneg && hasU) {
 	return (*tagp = NC_NAT);
     }
-    /* Special cases: all (u)int64 values */
-    if(allones) {
+    /* To simplify the code, we look for special case of NC_UINT64
+       constants that will not fit into an NC_INT64 constant.
+     */
+    if(tag == NC_UINT64 && bit63set) {
         uint64_val = uint64;
-	return (*tagp = NC_UINT64);
+	return tag;
     }
     /* At this point we need deal only with int64 value */
     /* Apply the isneg */
@@ -3160,7 +3161,7 @@ downconvert(unsigned long long uint64, int* tagp, int isneg, int hasU)
 	}
         goto done;
     }
-    if(tag >= NC_UBYTE && int64 < 0)
+    if(isuinttype(tag) && int64 < 0)
 	goto outofrange;
     switch (tag) {
     case NC_UBYTE:

ncgen/semantics.c

@@ -775,54 +775,110 @@ processattributes(void)
 
 /*
 Given two types, attempt to upgrade to the "bigger type"
+Rules:
+- type size has precedence over signed/unsigned:
+   e.g. NC_INT over NC_UBYTE
 */
 static nc_type
-infertype(nc_type prior, nc_type next)
+infertype(nc_type prior, nc_type next, int hasneg)
 {
+    nc_type sp, sn;
+    /* assert isinttype(prior) && isinttype(next) */
     if(prior == NC_NAT) return next;
     if(prior == next) return next;
-    if(isfloattype(prior) && isfloattype(next)) {
-	return (prior > next ? prior : next);
-    }
-    if(isinttype(prior) && isinttype(next)) {
-	/* 1. base size takes precedence => e.g. NC_INT over NC_UBYTE */
-	if(signedtype(prior) > signedtype(next)) return prior;
-	if(signedtype(prior) < signedtype(next)) return next;
-	/* 2. unsigned over signed */
-	/* assert signedtype(prior) == signedtype(next) */
-	/* Note, prior == next was handled above */
-	return isuinttype(prior) ? prior : next;
-    }
+    sp = signedtype(prior);
+    sn = signedtype(next);
+    if(sp <= sn)
+	return next;
+    if(sn < sp)
+	return prior;
     return NC_NAT; /* all other cases illegal */
 }
 
 /*
- Look at the primitive values of the
- attribute's datalist to infer the type of the attribute.
- There is a potential ambiguity when that value is a string.
- Is the attribute type NC_CHAR or NC_STRING?
- The answer is we always assume it is NC_CHAR in order to
- be back compatible with ncgen.
+Collect info by repeated walking of the attribute value list.
 */
-
 static nc_type
 inferattributetype1(Datasrc* src)
 {
     nc_type result = NC_NAT;
-    /* Recurse down any enclosing compound markers to find first non-fill "primitive"*/
-    while(result == NC_NAT && srcmore(src)) {
-	if(issublist(src)) {
-	    srcpush(src);
-	    result = inferattributetype1(src);
-	    srcpop(src);
-	} else {	
-	    NCConstant* con = srcnext(src);
-	    while(con != NULL && isprimplus(con->nctype)) {
-		result = infertype(result,con->nctype);
-		if(result == NC_NAT) break;
-		con = srcnext(src);
-	    }
+    int hasneg = 0;
+    int stringcount = 0;
+    int charcount = 0;
+    int forcefloat = 0;
+    int forcedouble = 0;
+    int forceuint64 = 0;
+
+    /* Walk the top level set of attribute values to ensure non-nesting */
+    while(srcmore(src)) {
+	NCConstant* con = srcnext(src);
+	if(con == NULL) return NC_NAT;
+	if(con->nctype > NC_MAX_ATOMIC_TYPE) { /* illegal */
+	    return NC_NAT;
 	}
+	srcnext(src);
+    }	    
+    /* Walk repeatedly to get info for inference (loops could be combined) */
+
+    /* Compute: all strings or chars? */
+    srcreset(src);
+    stringcount = 0;
+    charcount = 0;
+    while(srcmore(src)) {
+	NCConstant* con = srcnext(src);
+	if(con->nctype == NC_STRING) stringcount++;
+	else if(con->nctype == NC_CHAR) charcount++;
+    }
+    if((stringcount+charcount) > 0) {
+        if((stringcount+charcount) < srclen(src)) 
+	    return NC_NAT; /* not all textual */
+	return NC_CHAR;
+    }
+
+    /* Compute: any floats/doubles? */
+    srcreset(src);
+    forcefloat = 0;
+    forcedouble = 0;
+    while(srcmore(src)) {
+	NCConstant* con = srcnext(src);
+	if(con->nctype == NC_FLOAT) forcefloat = 1;
+	else if(con->nctype == NC_DOUBLE) {forcedouble=1; break;}
+    }
+    if(forcedouble) return NC_DOUBLE;
+    if(forcefloat)  return NC_FLOAT;
+
+    /* At this point all the constants should be integers */
+
+    /* Compute: are there any uint64 values > NC_MAX_INT64? */
+    srcreset(src);
+    forceuint64 = 0;
+    while(srcmore(src)) {
+	NCConstant* con = srcnext(src);
+	if(con->nctype != NC_UINT64) continue;
+	if(con->value.uint64v > NC_MAX_INT64) {forceuint64=1; break;}
+    }
+    if(forceuint64)
+	return NC_UINT64;
+
+    /* Compute: are there any negative constants? */
+    srcreset(src);
+    hasneg = 0;
+    while(srcmore(src)) {
+	NCConstant* con = srcnext(src);
+	switch (con->nctype) {
+	case NC_BYTE :   if(con->value.int8v < 0)   {hasneg = 1;} break;
+	case NC_SHORT:   if(con->value.int16v < 0)  {hasneg = 1;} break;
+	case NC_INT:     if(con->value.int32v < 0)  {hasneg = 1;} break;
+	}	
+    }
+
+    /* Compute: inferred integer type */
+    srcreset(src);
+    result = NC_NAT;
+    while(srcmore(src)) {
+	NCConstant* con = srcnext(src);
+	result = infertype(result,con->nctype,hasneg);	
+	if(result == NC_NAT) break; /* something wrong */
     }
     return result;
 }
@@ -843,6 +899,10 @@ inferattributetype(Symbol* asym)
     src = datalist2src(datalist);
     nctype = inferattributetype1(src);    
     freedatasrc(src);
+    if(nctype == NC_NAT) { /* Illegal attribute value list */
+	semerror(asym->lineno,"Non-simple list of values for untyped attribute: %s",fullname(asym));
+	return;
+    }
     /* get the corresponding primitive type built-in symbol*/
     /* special case for string*/
     if(nctype == NC_STRING)

ncgen/util.c

@@ -299,9 +299,12 @@ isinttype(nc_type nctype)
 }
 
 int
-isuinttype(nc_type nctype)
+isuinttype(nc_type t)
 {
-    return isinttype(nctype) && (nctype >= NC_UBYTE);
+    return isinttype(t)
+	   && t >= NC_UBYTE
+           && t <= NC_UINT64
+           && t != NC_INT64;
 }
 
 int

ncgen/util.h

@@ -8,7 +8,6 @@
 
 #define MAX(x,y) ((x)>(y)?(x):(y))
 
-
 extern void expe2d(char*);
 extern int pow2(int);
 extern void tztrim(char*);