Combined enum bitset test missing == value causes compiler Exception

[hopscc]

This program

class EnumComb
    enum States
        has Flags
        Init=1, Settling=2, Pour=4

    def main is shared
        x = States.Init
        y = States(Init, Settling)
        #assert (x & y) == x
        if x & y 
            print 'x non0 in y -bitwise'

causes this error

Unhandled Exception: Cobra.Lang_ert_04ffb25fa124e79edfd9952341f0b305.FallThroughException: info=TruthExpr-sh(14586, didBindInh=false, didBindInt=false, didStartBindImp=true, isBindingImp=true, didBindImp=false, token=Token(AMPERSAND, '&', '&', ln 10, col 8, EnumCombineTestBug.cobra), Treatment=AsIs, type=nil, 14586)
   at TruthExpr._bindImp() in c:\home\hops\src\cobra\wkspace1\Source\Expr.cobra:line 2195
   at Node.BindImp() in c:\home\hops\src\cobra\wkspace1\Source\Node.cobra:line 560
   at Stmt.BindImp() in c:\home\hops\src\cobra\wkspace1\Source\Statements.cobra:line 108
   at Expr.BindImp() in c:\home\hops\src\cobra\wkspace1\Source\Expr.cobra:line 193
   at IfStmt._bindImp() in c:\home\hops\src\cobra\wkspace1\Source\Statements.cobra:line 680
   at Node.BindImp() in c:\home\hops\src\cobra\wkspace1\Source\Node.cobra:line 560
   at Stmt.BindImp() in c:\home\hops\src\cobra\wkspace1\Source\Statements.cobra:line 108
   at AbstractMethod._bindImp() in c:\home\hops\src\cobra\wkspace1\Source\Members.cobra:line 886
   at Method._bindImp() in c:\home\hops\src\cobra\wkspace1\Source\Members.cobra:line 1201
   at Node.BindImp() in c:\home\hops\src\cobra\wkspace1\Source\Node.cobra:line 560
   at BoxMember.BindImp() in c:\home\hops\src\cobra\wkspace1\Source\Members.cobra:line 256
   at Box._bindImp() in c:\home\hops\src\cobra\wkspace1\Source\Boxes.cobra:line
740
   at ClassOrStruct._bindImp() in c:\home\hops\src\cobra\wkspace1\Source\Boxes.cobra:line 1132
   at Node.BindImp() in c:\home\hops\src\cobra\wkspace1\Source\Node.cobra:line 560
   at NameSpace._bindImp() in c:\home\hops\src\cobra\wkspace1\Source\NameSpace.cobra:line 233
   at Node.BindImp() in c:\home\hops\src\cobra\wkspace1\Source\Node.cobra:line 560
   at CobraModule._bindImp() in c:\home\hops\src\cobra\wkspace1\Source\Module.cobra:line 97
   at Node.BindImp() in c:\home\hops\src\cobra\wkspace1\Source\Node.cobra:line 560
   at BindImplementationPhase.InnerRun() in c:\home\hops\src\cobra\wkspace1\Source\Phases\phases\BindImplementationPhase.cobra:line 18
   at Phase.Run() in c:\home\hops\src\cobra\wkspace1\Source\Phases\phases\Phase.cobra:line 102
   at Compiler.RunPhase(Phase phase) in c:\home\hops\src\cobra\wkspace1\Source\Compiler.cobra:line 322
   at Compiler._compileFilesNamed(IList`1 paths, Boolean writeTestInvocation, Predicate`1 stopCompilation) in c:\home\hops\src\cobra\wkspace1\Source\Compiler.cobra:line 390
   at Compiler.CompileFilesNamed(IList`1 paths, Boolean writeTestInvocation, Predicate`1 stopCompilation) in c:\home\hops\src\cobra\wkspace1\Source\Compiler.cobra:line 344
   at CommandLine.DoCompile(List`1 paths, Boolean willPrintSuccessMsg, Boolean writeTestInvocation, Predicate`1 stopCompilation) in c:\home\hops\src\cobra\wkspace1\Source\CommandLine.cobra:line 676
   at CommandLine.DoRun(List`1 paths) in c:\home\hops\src\cobra\wkspace1\Source\CommandLine.cobra:line 790
   at CommandLine.Run(List`1 args) in c:\home\hops\src\cobra\wkspace1\Source\CommandLine.cobra:line 637
   at CommandLine.Run() in c:\home\hops\src\cobra\wkspace1\Source\CommandLine.cobra:line 575
   at CobraMain.Main() in c:\home\hops\src\cobra\wkspace1\Source\cobra.cobra:line 17
(null)
624 xx:...hops/src/cobra/Tst>

which is corrected by this patch

Index: Expr.cobra
===================================================================
--- Expr.cobra  (revision 2461)
+++ Expr.cobra  (working copy)
@@ -2192,7 +2192,7 @@
                type = _expr.type
                if type is .compiler.boolType
                        _treatment = Treatment.AsIs
-               else if type inherits AbstractNumberType
+               else if type inherits AbstractNumberType or type inherits EnumDecl
                        _treatment = Treatment.CompareToZero
                else if _expr inherits NilLiteral
                        .compiler.warning(this, 'The value nil will always evaluate to false.')

[Charles]

Looking at what fixes it reveals that it has nothing to do with combining enums or examining bits. A simple enum used as a truth value will do it:

enum E
    A

class X

    def main
        a = E.A
        if a, CobraCore.noOp(a)

[Charles]

Should we really allow "if someEnum"? It's not clear to me that we should:

enum Colors
    Red
    Green
    Blue

class Test

    def main
        c = Colors.Red
        if c, print 'true'
        else, print 'false'

# uh... what does it mean for a Colors value to be true?

In most cases, I think "if someEnum" is going to be senseless because enums are symbolic. In most cases it will be equivalent to "if someEnum is not the first enum value, ...". And for the flag tests, it seems easy enough to write:

#!/cobra
if someEnum & MyEnum.FlagA == MyEnum.FlagA

...as we do now.

[hopscc]

Theres a recco in the MS Enum description about the use of a 0 value member
 Enum Class under best practises
(default or OOB value named None)

If followed this would make for an easy non-default or valid test

enum State
   None
   Invalid
   Settling
   Held
   Valid

def main
    gadgetReady = State()
    ...
    if not gadgetReady
        throw UnInitialisedGadgetException()
     
    # otherwise can safely transition through valid states

also

if not somenum

is always easier to write than

if someEnum & MyEnum.FlagA == MyEnum.FlagA

regardless of the utility of the above 0/non0 test or not,
the compiler exception is a problem since its easy enough
(for convenience or laziness or habit) to write a bitset test leaving off the part after the '=='

if someEnum & MyEnum.FlagA 

#instead of 
if someEnum & MyEnum.FlagA  == MyEnum.FlagA

Which is how I found the problem initially.
Doing this should not incur a compiler crash.
...

On a slightly different topic:
Without bitsets and using Enums how would you test that a enum value was in a combined enum set of values. (without listing all the combined values in a set explicitly

e.g trivial example -testing that a day was a weekend

enum DaysOfWeek
    Mon,Tues, Wed, Thurs, Fri, Sat, Sun
def main
    day = aRandomDay   # arbitrary day of the week
    ...
    weekEndDays = DaysOfWeek(Sat,Sun)
    
    # have enum and combined value subset, how tell day in subset without
    # if day in [DaysOfWeek.Sat, DaysOfWeek.Sun]
    # why cant I do some sort of inclusion test on the combined value
    #if day in weekEndDays

I dont want the bitset settings cos its tedious to setup, error prone(and doesnt scale well),

• maybe useful if could specify to autogenerate bitset values for an enum on declaration

I dont like the if day in [DaysOfWeek.Sat, DaysOfWeek.Sun] (which works) cos I have to individually list, name and repeat the values in the combined enum value
which I already have.

Perhaps if there was some cobra Enum class extension that could turn a combined enum into a list (the native conversions give various C# conversion errors from enumName member to int SFAICT )
or just some magic for enums hidden behind 'in'

e.g

if day in Enum.toList(weekEndDays)
if day in weekEndDays

[hopscc]

Ignore all the stuff below 'On a slightly different topic'.
I corrected some misunderstandings I had and found something that works.

I still think it would be a useful enhancement to have an option that gets cobra to autogenerate the bitset values..

[Charles]

3 things:

1)
Could we have a BitsetAttribute? that applies to enums and causes the compiler to (a) also add the FlagsAttribute? and (b) generate the values (1, 2, 4, 8, ...)?

2)
You said:

if not someEnum

is always easier to write than:

if someEnum & MyEnum.FlagA == MyEnum.FlagA

...but are those really equivalent? The "if not someEnum" will mean any non-zero value, while the check with "FlagA" will mean something different depending on whether FlagA is 0 or 1. If these are really flags then it will have the value 1. The "if not someEnum" would be true even if it was FlagB that was on, where as the check with FlagA would not.

I agree that the gadgetReady example is nice, but it won't always be the case that people (or the libs they use) will follow that case.

My current thinking is to make it an error which tells the programmer to use an explicit check so that programmers don't get themselves into trouble. We know this won't break any code since the current use causes an internal compiler error.

3)
On a related note, I think it would be interesting to be able to say:

if MyEnum.FlagA in flags, ...

For any enum expression whose enum type has the Flags attribute. This eliminates the repetition found in the pattern with &.

[hopscc]

Re 1)

Yes - I was going to add a clause to the enum decl to activate the same functionality(as autobitset or something similar).

Dont care about the exact mechanism but it would be very convenient to have something that does this.

re 2)

Equivalent accounting for zero or non zero flag values when Flags Attrib set

if not someNum & MyNum.FlagA 
                    # ==> someNum & MyNum.FlagA == MyNum.FlagA when FlagA == 0
if someNum & MyNum,FlagA 
                    # ==> someNum & MyNum.FlagA == MyNum.FlagA when FlagA <> 0

still simpler
Also (and probably more importantly) congruent to the condition test mappings for zero/non zero for non boolean Type values.

I dont see why an Enum value or enum bitset expr test is so special it must have a complete explicit test or emit an error
when other non-boolean types (Number, Nilable, Char, Reference) do not.

re 3)
That capability is exactly what I was expecting/looking for but for both FlagAttributed and not enum Types.
If its Flagged the bitset lookup is an optimisation, if not its a slightly less optimal set/List inclusion check
either way its simple and (more) obvious than either the bitset or multiple single
equality checks..

BTW theres another bug in this area (not done a ticket for it yet) where Cobra Enums
dont know about the static and (single non static- (hasFlag specifically)) methods on the .Net Enum baseclass.

[Charles]

1)
I'll consider it some more. Question: Should the last assert pass or fail?

enum E
    A=1, B=2
class P
    def main
        i = 0
        assert not i
        i = 1
        assert i
        e = E.A
        assert e

If we allow truthfulness on enums, does it mean "non-zero" like with numbers? (And ints back enums.) Or does it mean, "not the first enum"?

2)
Regarding a much earlier comment you made regarding:

if day in [DaysOfWeek.Sat, DaysOfWeek.Sun] 

I don't see anything wrong with that for non-flag-enums. You could always make a utility method and/or cache the list if you like. For flag enums, I guess I'm still deciding between:

if E.A in e, ...
if e.hasFlag(E.A), ...
if e & E.A, ...

The .hasFlag version strikes me as most clear.

[hopscc]

pass
truthfulness maps to non-zero (same as elsewhere)

2)
If done should be done for both ( flag and not)
otherwise For non flag means you HAVE to explicitly list each member and for flag you use the bit test syntax - this is no obvious improvement over the C# separation-of-syntax-for-optimisation stupidity

i.e I want ( and see it obvious/advantageous to do)

!cobra
day as DaysOfWeek()
...
weekend = DaysOfWeek(Sat, Sun)
..
if day in weekend

regardless of how Enum DaysOfWeek? was declared

Flag enums is a built in optimisation but you have to set it up correctly and
use it with the right syntax - I want the compiler to have a common test paradigm and just do the right thing regardless of whether you've chosen to bitset or not the Enum values

I'd go for the

if E.A in e, ...

as the clearest given that hasFlags is .Net specific to bitsets.

[Charles]

1)
Then for:

enum E
    A=1, B=2

...we would need:

    assert e
    if e, print e

...to issue warnings that the expression will always be true. Right?

2)
Currently SomeEnum(A, B) means that A and B of SomeEnum are bitwise ORed together and the resulting type is SomeEnum. It's not clear to me how to satisfy your desire that the case you gave be made to work whether SomeEnum is bit-flags-based or not. What implementation do you propose? What is the type of SomeEnum(A, B)?

[hopscc]

1) Yeah - that fits with the rest of expression value checking in asserts
2) Set<of SomeEnum> would be the obvious candidate
manually equiv to something like

d = someRandomDayOfWeek
inweekend = d in Set<of DaysOfWeek>([Days.Sat, Days.Sun])
#vs inweekend = d in DaysOfWeek(Sat, Sun)
print '[d] in week end', inweekend

[Charles]

See also:  http://cobra-language.com/forums/viewtopic.php?f=4&t=757&start=0

When a user writes SomeEnum.A or SomeEnum.B they are not expecting true.

[Charles]

Re: @hopscc last message,

1) True

2) I would find it weird that SomeEnum(A, B) would be produce a Set<of SomeEnum> for non-flag enums, but produce a SomeEnum for flag-based enums. Unless you meant to always use Set<>, but I would find that odd as well.

[Charles]

Resolved in changeset:2801

The more I thought about it...

1) The more I perceive enums as special symbolic types. Truthfulness is not obvious for them.

2) The more comfortable I became with using a bitwise operator (&) for a bit check.

Also, regarding hopscc desire to use "in" with both types of enums, I am uncomfortable having two different return types for the expression SomeEnum(A, B). You can still use "in" with DaysOfWeek?... you just have to create the Set yourself.

[hopscc]

Re 2) 2 above and 'weird'.
Not advocating that - they.'re two different concepts - symbolic value and set of same; we're poisoned/polluted/corrupted by them being conflated together through an implementation hack using ints and bit wise math in C/C#.

Re above comment
1) yes agree absolutely 'a symbolic value OR a set of symbolic values'
2) No,no,no, noooooooo - &,| bit wise ops are an aberration on the implementation of Enums as 'special ints' and the ability to value them as bit patterns that the bit wise ops can work (rapidly) on for doing and testing set inclusion with.

it only looks reasonable due to historical familiarity...
(have a look at The java tutorial description around Typesafety enums and EnumBitset?).

(reopened to halite discussion points above).

[Charles]

We can continue the discussion via the forums. I'm closing this ticket as the compiler no long gives an internal error as originally reported.

A future enum proposal can be a new ticket.