#1043 Compiler bug: NPE when parsing readonly const field

katox Sat 27 Mar 2010

Test sample

class X
{
readonly const Int x := 2
}

Running fan test.fan yields

ERROR: cannot compile script
sys::NullErr: java.lang.NullPointerException
  compiler::Parser.fieldDef (Parser.fan:500)
  compiler::Parser.slotDef (Parser.fan:421)
  compiler::Parser.typeDef (Parser.fan:223)
  compiler::Parser.parse (Parser.fan:49)
  compiler::Parse.run (Parse.fan:47)
  fan.sys.List.each (List.java:527)
  compiler::Parse.run (Parse.fan:44)
  compiler::Compiler.frontend (Compiler.fan:80)
  compiler::Compiler.compile (Compiler.fan:63)
  compiler::Main.compileScript (Main.fan:52)
  java.lang.reflect.Method.invoke (Method.java:597)
  fan.sys.Method.invoke (Method.java:536)
  fan.sys.Method$MethodFunc.call (Method.java:281)
  fan.sys.Method.call (Method.java:152)
  fanx.util.EnvScripts.compile (EnvScripts.java:87)
  fanx.util.EnvScripts.compile (EnvScripts.java:45)
  fan.sys.Env.compileScript (Env.java:132)
  fanx.tools.Fan.executeFile (Fan.java:56)
  fanx.tools.Fan.execute (Fan.java:34)
  fanx.tools.Fan.run (Fan.java:236)
  2 More...

brian Sun 28 Mar 2010

The readonly keyword is essentially sugar for private set to make the setter private. However const fields don't have setters, therefore they can't make their setter private. So I pushed a fix to just report that that is an invalid combination of modifiers.

I think this is fine actually since in general only the class can set their own const fields. However the one case where it might crop up is if you wanted to expose a it-block constructor to allow setting some const field, but not others. But I think you can just handle that yourself by doing checks after you call the it-block, or using a little boiler plate with a getter method and private field.

andy Mon 29 Mar 2010

I thought we were going to remove readonly keyword?

brian Mon 29 Mar 2010

I thought we were going to remove readonly keyword?

We discussed it - can't remember all the details. But it turns out there is a lot of code that benefits from an easy way to make the setter private, so I think its a nice thing to keep in the language.

tactics Mon 29 Mar 2010

I could potentially see removing readonly if things swayed that way. Learning Fantom, I always got confused by the difference between const and readonly. (const means it can't change and readonly means you can't change it).

andy Mon 29 Mar 2010

Learning Fantom, I always got confused by the difference between const and readonly

I sorta agree there - I think this code reads more clearly and avoids the ambiguity between const and readonly:

Int x { private set }

KevinKelley Mon 29 Mar 2010

Int x { private set }

Agree, that's what I do, I didn't realize we even had a readonly. :-)

Login or Signup to reply.