Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Stan 2.33] Make expiring deprecations into errors #1287

Merged
merged 21 commits into from
Aug 15, 2023
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 32 additions & 50 deletions src/frontend/Deprecation_analysis.ml
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,32 @@ open Core_kernel
open Ast
open Middle

let current_removal_version = (2, 32)

let expired (major, minor) =
let removal_major, removal_minor = current_removal_version in
removal_major > major || (removal_major = major && removal_minor >= minor)

let deprecated_functions =
String.Map.of_alist_exn
[ ("multiply_log", ("lmultiply", "2.32.0"))
; ("binomial_coefficient_log", ("lchoose", "2.32.0"))
; ("cov_exp_quad", ("gp_exp_quad_cov", "2.32.0"))
; ("fabs", ("abs", "2.33.0")) ]
[ ("multiply_log", ("lmultiply", (2, 32)))
; ("binomial_coefficient_log", ("lchoose", (2, 32)))
; ("cov_exp_quad", ("gp_exp_quad_cov", (2, 32))); ("fabs", ("abs", (2, 33)))
]

(* TODO need to mark lkj_cov as deprecated *)

let deprecated_odes =
String.Map.of_alist_exn
[ ("integrate_ode", ("ode_rk45", "3.0"))
; ("integrate_ode_rk45", ("ode_rk45", "3.0"))
; ("integrate_ode_bdf", ("ode_bdf", "3.0"))
; ("integrate_ode_adams", ("ode_adams", "3.0")) ]
[ ("integrate_ode", ("ode_rk45", (3, 0)))
; ("integrate_ode_rk45", ("ode_rk45", (3, 0)))
; ("integrate_ode_bdf", ("ode_bdf", (3, 0)))
; ("integrate_ode_adams", ("ode_adams", (3, 0))) ]

let deprecated_distributions =
String.Map.of_alist_exn
(List.map
~f:(fun (x, y) -> (x, (y, "2.32.0")))
~f:(fun (x, y) -> (x, (y, (2, 32))))
(List.concat_map Middle.Stan_math_signatures.distributions
~f:(fun (fnkinds, name, _, _) ->
List.filter_map fnkinds ~f:(function
Expand All @@ -36,8 +44,8 @@ let stan_lib_deprecations =
[%message
"Common key in deprecation map"
(key : string)
(x : string * string)
(y : string * string)] )
(x : string * (int * int))
(y : string * (int * int))] )

let is_deprecated_distribution name =
Option.is_some (Map.find deprecated_distributions name)
Expand Down Expand Up @@ -109,33 +117,23 @@ let rec collect_deprecated_expr (acc : (Location_span.t * string) list)
({expr; emeta} : (typed_expr_meta, fun_kind) expr_with) :
(Location_span.t * string) list =
match expr with
| FunApp (StanLib FnPlain, {name= "if_else"; _}, l) ->
acc
@ [ ( emeta.loc
, "The function `if_else` is deprecated and will be removed in Stan \
2.32.0. Use the conditional operator (x ? y : z) instead; this \
can be automatically changed using the canonicalize flag for \
stanc" ) ]
@ List.concat_map l ~f:(fun e -> collect_deprecated_expr [] e)
| FunApp ((StanLib _ | UserDefined _), {name; _}, l) ->
| CondDistApp ((StanLib _ | UserDefined _), {name; _}, l)
|FunApp ((StanLib _ | UserDefined _), {name; _}, l) ->
let w =
match Map.find stan_lib_deprecations name with
| Some (rename, version) ->
[ ( emeta.loc
, name ^ " is deprecated and will be removed in Stan " ^ version
^ ". Use " ^ rename
^ " instead. This can be automatically changed using the \
canonicalize flag for stanc" ) ]
| _ when String.is_suffix name ~suffix:"_cdf" ->
[ ( emeta.loc
, "Use of " ^ name
^ " without a vertical bar (|) between the first two arguments \
of a CDF is deprecated and will be removed in Stan 2.32.0. \
This can be automatically changed using the canonicalize \
flag for stanc" ) ]
| Some (rename, (major, minor)) ->
if expired (major, minor) then []
else
let version = string_of_int major ^ "." ^ string_of_int minor in
[ ( emeta.loc
, name ^ " is deprecated and will be removed in Stan " ^ version
^ ". Use " ^ rename
^ " instead. This can be automatically changed using the \
canonicalize flag for stanc" ) ]
| _ -> (
match Map.find deprecated_odes name with
| Some (rename, version) ->
| Some (rename, (major, minor)) ->
let version = string_of_int major ^ "." ^ string_of_int minor in
[ ( emeta.loc
, name ^ " is deprecated and will be removed in Stan " ^ version
^ ". Use " ^ rename
Expand Down Expand Up @@ -181,22 +179,6 @@ let rec collect_deprecated_stmt fundefs (acc : (Location_span.t * string) list)
, "Functions do not need to be declared before definition; all user \
defined function names are always in scope regardless of \
defintion order." ) ]
| FunDef
{ body
; funname= {name; id_loc}
; arguments= (_, ((UReal | UInt) as type_), _) :: _
; _ }
when String.is_suffix ~suffix:"_log" name ->
let acc =
acc
@ [ ( id_loc
, "Use of the _log suffix in user defined probability functions is \
deprecated and will be removed in Stan 2.32.0, use name '"
^ update_suffix name type_
^ "' instead if you intend on using this function in ~ \
statements or calling unnormalized probability functions \
inside of it." ) ] in
collect_deprecated_stmt fundefs acc body
| FunDef {body; _} -> collect_deprecated_stmt fundefs acc body
| IfThenElse ({emeta= {type_= UReal; loc; _}; _}, ifb, elseb) ->
let acc =
Expand Down
8 changes: 5 additions & 3 deletions src/frontend/Deprecation_analysis.mli
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@ val update_suffix : string -> Middle.UnsizedType.t -> string
val collect_userdef_distributions :
typed_program -> Middle.UnsizedType.t String.Map.t

val expired : int * int -> bool
val without_suffix : string list -> string -> string
val is_deprecated_distribution : string -> bool
val deprecated_distributions : (string * string) String.Map.t
val deprecated_functions : (string * string) String.Map.t
val rename_deprecated : (string * string) String.Map.t -> string -> string
val deprecated_distributions : (string * (int * int)) String.Map.t
val deprecated_functions : (string * (int * int)) String.Map.t
val rename_deprecated : (string * (int * int)) String.Map.t -> string -> string
val stan_lib_deprecations : (string * (int * int)) String.Map.t

val userdef_functions :
('a, 'b, 'c, 'd) statement_with program
Expand Down
131 changes: 131 additions & 0 deletions src/frontend/Deprecation_removals.ml
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
(** Generate errors for expired deprecations

Code in this module should generally only be hot for ONE version
after the removal, then the errors will be moved to the
parser/typechecker/etc as appropriate.
*)

open Core_kernel
open Core_kernel.Poly
open Middle
open Ast
open Deprecation_analysis

let pound_comment_usages : Location_span.t list ref = ref []
let old_array_usages : (Location_span.t * bool) list ref = ref []

let rec collect_removed_expr (acc : (Location_span.t * string) list)
({expr; emeta} : (typed_expr_meta, fun_kind) expr_with) :
(Location_span.t * string) list =
match expr with
| GetLP ->
acc
@ [ ( emeta.loc
, "The get_lp() function was removed in Stan 2.32.0. Use target() \
instead. This can be done automatically with the canonicalize \
flag for stanc" ) ]
| FunApp (StanLib FnPlain, {name= "if_else"; _}, l) ->
acc
@ [ ( emeta.loc
, "The if_else() function was removed in Stan 2.32.0. Use the \
conditional operator (x ? y : z) instead; this can be \
automatically changed using the canonicalize flag for stanc" ) ]
@ List.concat_map l ~f:(fun e -> collect_removed_expr [] e)
| FunApp ((StanLib _ | UserDefined _), {name; _}, l) ->
let w =
match Map.find stan_lib_deprecations name with
| Some (rename, (major, minor)) ->
if expired (major, minor) then
let version = string_of_int major ^ "." ^ string_of_int minor in
[ ( emeta.loc
, name ^ " was removed in Stan " ^ version ^ ". Use " ^ rename
^ " instead. This can be automatically changed using the \
canonicalize flag for stanc" ) ]
else []
| _ when String.is_suffix name ~suffix:"_cdf" ->
[ ( emeta.loc
, "Use of " ^ name
^ " without a vertical bar (|) between the first two arguments \
of a CDF was removed in Stan 2.32.0. This can be \
automatically changed using the canonicalize flag for stanc"
) ]
| _ -> [] in
acc @ w @ List.concat_map l ~f:(fun e -> collect_removed_expr [] e)
| _ -> fold_expression collect_removed_expr (fun l _ -> l) acc expr

let collect_removed_lval acc (l : Ast.typed_lval) =
match l.lval with
| LIndexed (l, _) ->
let rec flatten = function
| {lval= LVariable _; _} -> []
| {lval= LIndexed (l, idxs); _} ->
let flat = flatten l in
flat @ idxs in
if
not
(List.for_all
~f:(function
| Single {emeta= {type_= UnsizedType.UInt; _}; _} -> true
| _ -> false )
(flatten l) )
then
acc
@ [ ( l.lmeta.loc
, "Nested multi-indexing on the left hand side of assignment does \
not behave the same as nested indexing in expressions. This is \
considered a bug and has been disallowed in Stan 2.32.0. The \
indexing can be automatically fixed using the canonicalize flag \
for stanc." ) ]
else fold_lval_with collect_removed_expr (fun x _ -> x) acc l
| _ -> fold_lval_with collect_removed_expr (fun x _ -> x) acc l

let rec collect_removed_stmt (acc : (Location_span.t * string) list)
({stmt; _} : Ast.typed_statement) : (Location_span.t * string) list =
match stmt with
| Assignment {assign_lhs; assign_op= ArrowAssign; assign_rhs} ->
let acc =
acc
@ [ ( assign_lhs.lmeta.loc
, "The arrow-style assignment operator '<-' was removed in Stan \
2.32, use '=' instead. This can be done automatically with the \
canonicalize flag for stanc" ) ] in
collect_removed_lval [] assign_lhs @ collect_removed_expr acc assign_rhs
| IncrementLogProb e ->
let acc =
acc
@ [ ( e.emeta.loc
, "The increment_log_prob(...); function was removed in Stan \
2.32.0. Use target += ...; instead. This can be done \
automatically with the canonicalize flag for stanc" ) ] in
collect_removed_expr acc e
| _ ->
fold_statement collect_removed_expr collect_removed_stmt
collect_removed_lval
(fun l _ -> l)
acc stmt

let collect_removals (program : typed_program) =
let pounds =
List.map !pound_comment_usages ~f:(fun loc ->
( loc
, "Comments beginning with # were removed in Stan 2.32.0. Use // to \
begin line comments; this can be done automatically using the \
auto-format flag to stanc" ) ) in
let arrs =
List.map !old_array_usages ~f:(fun (loc, unsized) ->
let placement = if unsized then "a type" else "a variable name" in
( loc
, "Declaration of arrays by placing brackets after " ^ placement
^ " was removed in Stan 2.32.0. Instead use the array keyword before \
the type. This can be changed automatically using the auto-format \
flag to stanc" ) ) in
fold_program collect_removed_stmt (pounds @ arrs) program

let pp ?printed_filename ppf (span, message) =
let loc_str =
if span = Location_span.empty then ""
else " in " ^ Location.to_string ?printed_filename span.begin_loc in
Fmt.pf ppf "@[<hov 4>Error%s: %a@]" loc_str Fmt.text message

let pp_removals ?printed_filename ppf removals =
Fmt.(pf ppf "@[<v>%a@]@\n" (list ~sep:cut (pp ?printed_filename)) removals)
36 changes: 0 additions & 36 deletions src/frontend/Input_warnings.ml
Original file line number Diff line number Diff line change
Expand Up @@ -9,39 +9,3 @@ let empty file =
add_warning Middle.Location_span.empty
( "Empty file '" ^ file
^ "' detected; this is a valid stan model but likely unintended!" )

let deprecated token (pos, message) =
(* TODO(seantalts): should we only print deprecation warnings once per token? *)
let begin_pos =
{pos with Lexing.pos_cnum= pos.Lexing.pos_cnum - String.length token} in
let end_pos = {begin_pos with Lexing.pos_cnum= pos.pos_cnum - 1} in
let span =
Middle.Location_span.of_positions_opt begin_pos end_pos
|> Option.value ~default:Middle.Location_span.empty in
add_warning span message

let array_syntax ?(unsized = false) (pos1, pos2) =
let placement = if unsized then "a type" else "a variable name" in
add_warning
(Option.value ~default:Middle.Location_span.empty
(Middle.Location_span.of_positions_opt pos1 pos2) )
( "Declaration of arrays by placing brackets after " ^ placement
^ " is deprecated and will be removed in Stan 2.32.0. Instead use the \
array keyword before the type. This can be changed automatically using \
the auto-format flag to stanc" )

let drop_array_future () =
match !warnings with
| ( _
, "Variable name 'array' will be a reserved word starting in Stan 2.32.0. \
Please rename it!" )
:: tl ->
warnings := tl
| _ -> ()

let future_keyword kwrd version (pos1, pos2) =
add_warning
(Option.value ~default:Middle.Location_span.empty
(Middle.Location_span.of_positions_opt pos1 pos2) )
( "Variable name '" ^ kwrd ^ "' will be a reserved word starting in Stan "
^ version ^ ". Please rename it!" )
nhuurre marked this conversation as resolved.
Show resolved Hide resolved
15 changes: 0 additions & 15 deletions src/frontend/Input_warnings.mli
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,5 @@ val collect : unit -> Warnings.t list
val add_warning : Middle.Location_span.t -> string -> unit
(** Add a generic warning string to the current list *)

val deprecated : string -> Lexing.position * string -> unit
(** Register that a deprecated language construct has been found. *)

val empty : string -> unit
(** Register that an empty file is being parsed *)

val array_syntax : ?unsized:bool -> Lexing.position * Lexing.position -> unit
(** Warn on the old [real x\[3\]] syntax and suggest the [array] keyword*)

val future_keyword :
string -> string -> Lexing.position * Lexing.position -> unit
(** Warn on a keyword which will be reserved in the future*)

val drop_array_future : unit -> unit
(** Hack: Remove the most recent warning about array as a future keyword.
Needed due to the {e other} hack of how we currently parse arrays.
*)
22 changes: 14 additions & 8 deletions src/frontend/Semantic_error.ml
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ module StatementError = struct
| LValueMultiIndexing
| InvalidSamplingPDForPMF
| InvalidSamplingCDForCCDF of string
| InvalidSamplingNoSuchDistribution of string
| InvalidSamplingNoSuchDistribution of string * bool
| TargetPlusEqualsOutsideModelOrLogProb
| InvalidTruncationCDForCCDF of
(UnsizedType.autodifftype * UnsizedType.t) list
Expand Down Expand Up @@ -387,11 +387,16 @@ module StatementError = struct
"CDF and CCDF functions may not be used with sampling notation. Use \
target += %s_log(...) instead."
name
| InvalidSamplingNoSuchDistribution name ->
| InvalidSamplingNoSuchDistribution (name, true) ->
Fmt.pf ppf
"Ill-typed arguments to '~' statement. No distribution '%s' was \
found."
name
"Ill-typed arguments to '~' statement. No function '%s_lpmf' or \
'%s_lpdf' was found when looking for distribution '%s'."
name name name
| InvalidSamplingNoSuchDistribution (name, false) ->
Fmt.pf ppf
"Ill-typed arguments to '~' statement. No function '%s_lpdf' was \
found when looking for distribution '%s'."
name name
| InvalidTruncationCDForCCDF args ->
Fmt.pf ppf
"Truncation is only defined if distribution has _lcdf and _lccdf \
Expand Down Expand Up @@ -442,7 +447,7 @@ module StatementError = struct
Fmt.pf ppf "Function definitions must be wrapped in curly braces."
| NonRealProbFunDef ->
Fmt.pf ppf
"Real return type required for probability functions ending in _log, \
"Real return type required for probability functions ending in \
_lpdf, _lupdf, _lpmf, _lupmf, _cdf, _lcdf, or _lccdf."
| ProbDensityNonRealVariate (Some ut) ->
Fmt.pf ppf
Expand Down Expand Up @@ -635,8 +640,9 @@ let invalid_sampling_pdf_or_pmf loc =
let invalid_sampling_cdf_or_ccdf loc name =
StatementError (loc, StatementError.InvalidSamplingCDForCCDF name)

let invalid_sampling_no_such_dist loc name =
StatementError (loc, StatementError.InvalidSamplingNoSuchDistribution name)
let invalid_sampling_no_such_dist loc name is_int =
StatementError
(loc, StatementError.InvalidSamplingNoSuchDistribution (name, is_int))

let target_plusequals_outside_model_or_logprob loc =
StatementError (loc, StatementError.TargetPlusEqualsOutsideModelOrLogProb)
Expand Down
2 changes: 1 addition & 1 deletion src/frontend/Semantic_error.mli
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ val cannot_assign_function : Location_span.t -> UnsizedType.t -> string -> t
val cannot_assign_to_multiindex : Location_span.t -> t
val invalid_sampling_pdf_or_pmf : Location_span.t -> t
val invalid_sampling_cdf_or_ccdf : Location_span.t -> string -> t
val invalid_sampling_no_such_dist : Location_span.t -> string -> t
val invalid_sampling_no_such_dist : Location_span.t -> string -> bool -> t
val target_plusequals_outside_model_or_logprob : Location_span.t -> t

val invalid_truncation_cdf_or_ccdf :
Expand Down
Loading