D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 4623 - Non-integer type allowed as static array size
Summary: Non-integer type allowed as static array size
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: All Linux
: P2 major
Assignee: No Owner
URL:
Keywords: accepts-invalid, patch
Depends on:
Blocks:
 
Reported: 2010-08-11 11:44 UTC by Iain Buclaw
Modified: 2015-06-09 05:11 UTC (History)
3 users (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this issue.
Description Iain Buclaw 2010-08-11 11:44:22 UTC 
The code:

void main()
{
    int[0.128] a;
}

Should not compile, rather error with the message stating that either the size of array 'a' has non-integer type, or that the compiler cannot implicitly convert expression (0.128) of type double to uint.

See: http://dstress.kuehne.cn/nocompile/o/opIndex_05.d

Regards
Comment 1 Don 2010-09-29 12:43:23 UTC 
PATCH mtype.c, TypeSArray::semantic(), line 3344.

        dinteger_t d1 = dim->toInteger();
-        dim = dim->castTo(sc, tsize_t);
+        dim = dim->implicitCastTo(sc, Type::tsize_t);
        dim = dim->optimize(WANTvalue);
        dinteger_t d2 = dim->toInteger();
        if (dim->op == TOKerror)
           goto Lbaddim;

        if (d1 != d2)
            goto Loverflow;

... and further down:

              Loverflow:
                error(loc, "index %jd overflow for static array", d1);
+              Lbaddim:
                dim = new IntegerExp(0, 1, tsize_t);

TEST CASE for test suite
//bug 4623
static assert( !is (typeof(() {int[123.1] x; return x; })));

I've done this patch in a relatively complicated way, so that something like:
int[7654321.0] bug4623b;
generates only one error message, not two.
Comment 2 Iain Buclaw 2010-09-29 14:50:14 UTC 
Can't you catch it in the lexer?

Just thinking out loud...
Comment 3 Iain Buclaw 2010-09-29 17:39:15 UTC 
You can catch it in parse.c

@@ -2428,6 +2428,12 @@
                 {
                     //printf("it's type[expression]\n");
                     inBrackets++;
+
+		    if (token.value != TOKint32v && token.value != TOKuns32v &&
+			token.value != TOKint64v && token.value != TOKuns64v &&
+			    peekNext() == TOKrbracket)
+			error("I should be a an integral value, not [%s]", token.toChars());
+
                     Expression *e = parseAssignExp();           // [ expression ]
                     if (token.value == TOKslice)
                     {


I certainly don't mind either way. Your patch works just great, thanks!
Comment 4 Don 2010-09-30 06:26:43 UTC 
(In reply to comment #3)
> You can catch it in parse.c
[snip]

You can, but then it doesn't catch things like:

const float f = 1.23;

int[f] z;

> I certainly don't mind either way. Your patch works just great, thanks!

It looks as though you got that case from dstress. Have you been running the dstress tests? If you have, I'd be very interested to see the results. I believe that all the compiler ICE bugs are fixed, but I have no idea about how many others are still failing.
Comment 5 Iain Buclaw 2010-10-01 11:18:20 UTC 
I haven't ran dstress using the DMD compiler. I think I stumbled upon the case somewhere from an archived message on the ML that probably got forgotten about.

I wouldn't go as far as saying all compiler ICE bugs are fixed just yet (I raised an issue recently with CTFE with a patch supplied, for example), though they certainly are very far and few between now.
Comment 6 Walter Bright 2010-10-08 18:18:23 UTC 
http://www.dsource.org/projects/dmd/changeset/712