[SOLVED] Bug in Select Case as Const

General FreeBASIC programming questions.
counting_pine
Site Admin
Posts: 6323
Joined: Jul 05, 2005 17:32
Location: Manchester, Lancs

Re: [SOLVED] Bug in Select Case as Const

Post by counting_pine »

My initial investigation started with the parser, where there is some binary search code that - I guess - might break if the range crosses 0. That's why I thought of using (first_case - 8192) as a preliminary bias.
(So this preliminary bias might help prevent wraparound errors while the case values are being compiled, but once they are, it makes sense to use the minimum of those as the offset..)

But then the parser sends this information (and more) on to the AST:
- minvalue
- maxvalue
- an array of values (ranging from minvalue to maxvalue)
- an array of jump labels corresponding to the values

The AST/emitters do the following:
- emit a jump table array (0 to maxvalue-minvalue) containing the labels to jump to
- take the temporary 'select case' variable and subtract minvalue, and check to make sure the unsigned result is no greater than (maxvalue-minvalue)
- if it's in range, take the result's index into the jump table, and goto the label it finds

I think then, the most elegant solution would be to pass the jump table as 0-based, rather than 'minvalue'-based. Then all that needs changing is the parts of the emitters that emit the jump table.
But I didn't get to working out the best way to do that, or exactly how many places needed changing.

(Note, the ON GOTO parser emits jump tables, and so that call may need a fix too.)
coderJeff
Site Admin
Posts: 4313
Joined: Nov 04, 2005 14:23
Location: Ontario, Canada
Contact:

Re: [SOLVED] Bug in Select Case as Const

Post by coderJeff »

I think, you think, what I am thinking; that we both thought initial investigation would reveal an easy fix, LOL.

What I have so far, for -gen gas only, is at jayrm/select-case-const

I like that you reused the term "bias" as I think that is the better term to use as far as the emitter is concerned. But, what about from the programmer's perspective?
For example, this compiles under several versions of fbc, including the WIP I made:

Code: Select all

dim b as byte = 65
select case as const b
case 0
	'' null char
case 9
	'' tab
case 32 to 127
	'' ascii char
case 128 to 255
	'' extended ascii (what? it's a signed byte)
case 256 to 8192-1
	'' what????	
case else
	'' unexpeced control char
end select 
Should this be acceptable? Notice that it is a BYTE type.
counting_pine
Site Admin
Posts: 6323
Joined: Jul 05, 2005 17:32
Location: Manchester, Lancs

Re: [SOLVED] Bug in Select Case as Const

Post by counting_pine »

Yeah, it's definitely proved harder than I expected!

The term "bias" is a helpful one. I initially wanted to call it a base value, but the term 'base' is already used in the code..

I think there is scope for simplifying the code by just doing a range check on the (unbiased) case values as soon as they're parsed. (Do we have a function that checks if a value fits in a datatype?)

For the rest of the parsing stage, we bias the values by (firstvalue-8192), which means everything fits in 0..16384, and I think this means we can simplify the code and potentially remove some edge cases.

Then before sending the value table to the AST, we re-bias it to fit in the range 0..(maxvalue-minvalue).
I think the AST (onwards) should accept the values in the format:
- values (0 to maxvalue-minvalue)
- labels (...)
- minvalue (the value to subtract from the case var)
- length

While it currently accepts them as:
- values (minvalue to maxvalue)
- labels (...)
- minvalue
- maxvalue

It just involves extra work in the emitters.

Your changes look good. I'm a little uncomfortable just commenting out the (minvalue <= maxvalue) test, but I guess it should be fine. Maybe it should really be: cunsg(maxvalue - minvalue) < 8192.

But the changes do add more code to the parser, and I wonder if there's actually scope to take code away instead..
coderJeff
Site Admin
Posts: 4313
Joined: Nov 04, 2005 14:23
Location: Ontario, Canada
Contact:

Re: [SOLVED] Bug in Select Case as Const

Post by coderJeff »

counting_pine, I followed your suggestions and I think results are looking good. I have posted the changes in Pull Request #80. I suspect the LLVM emitter needs some attention; I am struggling to set-up an environment to test.

Internally, I am using bias/span instead of previous minval/maxval. Using your suggestion of the initial case range value of bias=VALUE-8192, it simplifies the problem so that the values will never be more than 8192*2. After all the CASE ranges are parsed, the values are re-biased, and the bias adjusted.
minval = bias
maxval = bias + span, truncated (wrapped around) in a 32bit or 64bit register

By using the original data type when parsing the constant CASE range values, we will get overflow warnings for out-of-range values. Like other conversions, a (u)byte &hff is seen as -1 or 255 depending on the data type.

What this means for users:

- SELECT CASE AS CONST creates a jump table
- CASE values must be same type as the variable, or you get get an overflow warning (but not an error)
- the 8192 limit is an internal hard coded limit and can be exceeded by nested SELECT CASE AS CONST structures as well.
- works with integer types; boolean, (u)byte, (u)short, (u)long, (u)longint, *(zstring ptr), *(wstring ptr)
- boolean allows FALSE (0), or TRUE (-1), or converted true (1) without warning. All other (non-zero values) converted to TRUE.
- Duplicate CASE range values cause a warning
- If number of jump table slots is exceeded, can get a misleading error of "Duplicate Value", although actual reason is that number if jump table sloats have been exceeded.
counting_pine
Site Admin
Posts: 6323
Joined: Jul 05, 2005 17:32
Location: Manchester, Lancs

Re: [SOLVED] Bug in Select Case as Const

Post by counting_pine »

Yeah, the changes are looking good. I like how you pretty much got rid of the maxval field. I think you've also done a good job on the unit test, working out good ways to test it.

It's worth noting that casting the case values does mean that SELECT CONST's behaviour will differ from standard SELECT in some cases, e.g.

Code: Select all

select case as const cubyte(0)
case &hff to &h1: print "-1 to 1"
end select
Some cases also don't get a warning at the minute, e.g.

Code: Select all

select case as const cbyte(-128)
case 128: print 128
end select
coderJeff
Site Admin
Posts: 4313
Joined: Nov 04, 2005 14:23
Location: Ontario, Canada
Contact:

Re: [SOLVED] Bug in Select Case as Const

Post by coderJeff »

Yea, both those examples do what I expected in select case as const.
First example:
- SELECT CASE AS CONST does not compile and errors with "Invalid CASE range, start value is greater than the end value"
- if changed to SELECT CASE, it compiles; there is no check for start <= end, and consequently, there is no value that will ever satisfy the CASE if the values are constant. SELECT CASE will accept expressions (not only constants) for case ranges, so at run-time a start could be greater than end.

Second example:
- yes, all CASE values are cast to the datatype in SELECT CASE AS CONST expr.
- yes, currently no checks for signed/unsigned out of range
- SELECT CASE will accept expressions for CASE RANGE

I meditated on the matter of casting in SELECT CASE AS CONST for some time. Currently, there is no function that fully checks that a constant fits in a specific signed/unsigned type. cConstIntExpr() is close. I started to write a complete range checker function, then decided it could be done independently of fixing this bug, as it would be something new to freebasic. There are many more constraints in the SELECT CASE AS CONST structure compared to SELECT CASE. While similar in concept, each have unique capabilities.

Given SELECT CASE AS CONST cbyte(expr), what is &hff? -1 or 255? Should it matter that the constant is given in hex or dec? If the answer is that it depends on cbyte/cubyte, then no range checking can be done. Only the overflow check (i.e. 256, -129) as is already built-in to the compiler can be expected.
counting_pine
Site Admin
Posts: 6323
Joined: Jul 05, 2005 17:32
Location: Manchester, Lancs

Re: [SOLVED] Bug in Select Case as Const

Post by counting_pine »

Yeah, this quickly becomes a difficult issue..

I think in my mind it is better to allow a wastefully large jump table, than to change the semantics of the Case statement.

The issue, historically, is that there's no warning for the end-programmer when using case values that are out of range. Casting down the value addresses a lot of these cases, but not all of them, so cases like -128 change the behaviour without giving a warning.

The issue with signed/unsigned is thorny. It's unclear whether assignments like 'dim as byte b=&hff' should be safe, but how about equality tests like 'if b=&hff then ...'?

In my mind, the best (simple) solution is to show a warning whenever the "natural" interpretation of case value is out of range for the data type. I tentatively expect cases that are likely to cause the warning will be rare, and can (should?) be worked around by using explicit datatypes for them.
dodicat
Posts: 7976
Joined: Jan 10, 2006 20:30
Location: Scotland

Re: [SOLVED] Bug in Select Case as Const

Post by dodicat »

I think a warning is required.
I have lost my console in fbide with the run settings
cmd /c "<$file>" <$param> & pause

Same with a command prompt from fbide.
I have been trying to fix it for a few days now, I compared the console layout with other ides, it shows exactly the same values, but the console is enlarged and I cannot print to it.

This all happened after the crash using select case as const with a boolean false.
I had to re-boot (win 10)
I mentioned this a while ago, but obviously nobody else was affected.
MrSwiss
Posts: 3910
Joined: Jun 02, 2013 9:27
Location: Switzerland

Re: [SOLVED] Bug in Select Case as Const

Post by MrSwiss »

Hi dodicat,

are you absolutely certain, that it wasn't a Boolean = TRUE, because that would,
numerically be a -1 (FALSE = 0, should not give any problems)?
dodicat
Posts: 7976
Joined: Jan 10, 2006 20:30
Location: Scotland

Re: [SOLVED] Bug in Select Case as Const

Post by dodicat »

Hi MrSwiss.
I ran the given offending code then then stopped via the task manager.
I fiddled a bit then ran it again, so I would have used both true and false.
But the next time the task manager had no effect, the computer more or less froze.
fbide showed a compiling mode, but wouldn't stop compiling.
I had to pull the plug eventually.
But as I say, obviously I am the only casualty.
MrSwiss
Posts: 3910
Joined: Jun 02, 2013 9:27
Location: Switzerland

Re: [SOLVED] Bug in Select Case as Const

Post by MrSwiss »

dodicat wrote:But as I say, obviously I am the only casualty.
That is because you are *Braveheart* ...
I've done Boolean with "Select Case" (but, never "As Const"), working OK, that way.
coderJeff
Site Admin
Posts: 4313
Joined: Nov 04, 2005 14:23
Location: Ontario, Canada
Contact:

Re: [SOLVED] Bug in Select Case as Const

Post by coderJeff »

The bug fix is in the works, but not added to the main fbc repo yet:
- The problem causing the endless compiling hang-up is resolved.
- The warnings are being discussed and have to with how the SELECT CASE AS CONST statement itself handles data. All the CASE ranges must be constants, so it is a little different from the normal SELECT CASE.
paul doe
Moderator
Posts: 1730
Joined: Jul 25, 2017 17:22
Location: Argentina

Re: [SOLVED] Bug in Select Case as Const

Post by paul doe »

coderJeff wrote:The bug fix is in the works, but not added to the main fbc repo yet:
- The problem causing the endless compiling hang-up is resolved.
Many thanks. The work you and the other contributors do, striving for quality, is much appreciated. Keep it up =D
Post Reply