Skip to content

Commit

Permalink
Remove faulty Identifier.toAstIdentifier() method
Browse files Browse the repository at this point in the history
Contrary to what the docstring says ("Identifier ready to be used in KS
expressions."), nothing could be further from the truth since it relies
on `humanReadable`, which is (as the name suggests) merely a
*human-readable* identifier, i.e. completely unusable for the purpose
stated here.

For this reason, using the `toAstIdentifier()` method in a hope that it
would work as declared often led to bugs, because for example it didn't
work for unnamed fields (`NumberedIdentifier`), as they normally cannot
appear in user-supplied expressions. Fortunately, all former usages of
`toAstIdentifier()` were already changed in commit e535b20 to use
`Ast.expr.InternalName`, which was specifically designed for the purpose
of injecting any identifiers we need (including unnamed fields) when
crafting an AST internally in the compiler. So we can finally remove
`toAstIdentifier()` for good because it is no longer used.
  • Loading branch information
generalmimon committed Aug 7, 2023
1 parent a100e4e commit 7071a9d
Showing 1 changed file with 0 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,6 @@ abstract class Identifier {
* error messaging purposes.
*/
def humanReadable: String

/**
* @return Identifier ready to be used in KS expressions.
*/
def toAstIdentifier: Ast.identifier = Ast.identifier(humanReadable)
}

/**
Expand Down

0 comments on commit 7071a9d

Please sign in to comment.