Clojure

Make Symbol.create methods intern their args

Details

  • Type: Enhancement Enhancement
  • Status: Closed Closed
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: Release 1.3
  • Component/s: None
  • Labels:
    None
  • Approval:
    Ok

Description

Right now the create methods presume interned strings, but not all users are disciplined about that. Also, Terracotta can correctly identity-share explicitly interned strings but not implicit ones (like string literals). We care because symbol equality is based on string identity.

Activity

Hide
Assembla Importer added a comment -

liwp said: [file:bEWc50RSmr3RYCeJe5afGb]: Fixes namespace and symbol name interning in Symbol/create

Show
Assembla Importer added a comment - liwp said: [file:bEWc50RSmr3RYCeJe5afGb]: Fixes namespace and symbol name interning in Symbol/create
Hide
Assembla Importer added a comment -

liwp said: Symbol/create(ns, name) gets called e.g. for def forms with ns == null, so I had to add a null check. I couldn't find the create() call that was causing this, so the null check was an easier option.

Show
Assembla Importer added a comment - liwp said: Symbol/create(ns, name) gets called e.g. for def forms with ns == null, so I had to add a null check. I couldn't find the create() call that was causing this, so the null check was an easier option.
Hide
Assembla Importer added a comment -

technomancy said: +1; this still applies cleanly and looks correct.

Show
Assembla Importer added a comment - technomancy said: +1; this still applies cleanly and looks correct.
Hide
Assembla Importer added a comment -

richhickey said: I think it might be best to get rid of the create methods entirely. It's only confusing to have both. I can do the refactor of the Clojure code - the patch would be daunting

Show
Assembla Importer added a comment - richhickey said: I think it might be best to get rid of the create methods entirely. It's only confusing to have both. I can do the refactor of the Clojure code - the patch would be daunting
Hide
Assembla Importer added a comment -
Show
Assembla Importer added a comment - stu said: [file:cq_Zfw17Sr36XAeJe5cbLA]
Hide
Assembla Importer added a comment -

stu said: second (big!) patch removes create, all callers now use intern.

Show
Assembla Importer added a comment - stu said: second (big!) patch removes create, all callers now use intern.
Hide
Assembla Importer added a comment -

stu said: Updating tickets (#182, #286)

Show
Assembla Importer added a comment - stu said: Updating tickets (#182, #286)

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: