Skip to content
Snippets Groups Projects
Commit bec92f6a authored by Allan Blanchard's avatar Allan Blanchard
Browse files

[RENAME PLUGIN] Builtin -> Instantiate

parent 135bf06e
No related branches found
No related tags found
No related merge requests found
Showing
with 88 additions and 91 deletions
...@@ -938,34 +938,34 @@ src/plugins/occurrence/options.mli: CEA_LGPL_OR_PROPRIETARY ...@@ -938,34 +938,34 @@ src/plugins/occurrence/options.mli: CEA_LGPL_OR_PROPRIETARY
src/plugins/occurrence/register.ml: CEA_LGPL_OR_PROPRIETARY src/plugins/occurrence/register.ml: CEA_LGPL_OR_PROPRIETARY
src/plugins/occurrence/register_gui.ml: CEA_LGPL_OR_PROPRIETARY src/plugins/occurrence/register_gui.ml: CEA_LGPL_OR_PROPRIETARY
src/plugins/occurrence/register_gui.mli: CEA_LGPL_OR_PROPRIETARY src/plugins/occurrence/register_gui.mli: CEA_LGPL_OR_PROPRIETARY
src/plugins/builtin/basic_blocks.ml: CEA_LGPL_OR_PROPRIETARY src/plugins/instantiate/basic_blocks.ml: CEA_LGPL_OR_PROPRIETARY
src/plugins/builtin/basic_blocks.mli: CEA_LGPL_OR_PROPRIETARY src/plugins/instantiate/basic_blocks.mli: CEA_LGPL_OR_PROPRIETARY
src/plugins/builtin/Builtin.mli: CEA_LGPL_OR_PROPRIETARY src/plugins/instantiate/Instantiate.mli: CEA_LGPL_OR_PROPRIETARY
src/plugins/builtin/builtin_builder.ml: CEA_LGPL_OR_PROPRIETARY src/plugins/instantiate/instantiator_builder.ml: CEA_LGPL_OR_PROPRIETARY
src/plugins/builtin/builtin_builder.mli: CEA_LGPL_OR_PROPRIETARY src/plugins/instantiate/instantiator_builder.mli: CEA_LGPL_OR_PROPRIETARY
src/plugins/builtin/configure.ac: CEA_LGPL_OR_PROPRIETARY src/plugins/instantiate/configure.ac: CEA_LGPL_OR_PROPRIETARY
src/plugins/builtin/Makefile.in: CEA_LGPL_OR_PROPRIETARY src/plugins/instantiate/Makefile.in: CEA_LGPL_OR_PROPRIETARY
src/plugins/builtin/stdlib/basic_alloc.ml: CEA_LGPL_OR_PROPRIETARY src/plugins/instantiate/stdlib/basic_alloc.ml: CEA_LGPL_OR_PROPRIETARY
src/plugins/builtin/stdlib/basic_alloc.mli: CEA_LGPL_OR_PROPRIETARY src/plugins/instantiate/stdlib/basic_alloc.mli: CEA_LGPL_OR_PROPRIETARY
src/plugins/builtin/stdlib/calloc.ml: CEA_LGPL_OR_PROPRIETARY src/plugins/instantiate/stdlib/calloc.ml: CEA_LGPL_OR_PROPRIETARY
src/plugins/builtin/stdlib/calloc.mli: CEA_LGPL_OR_PROPRIETARY src/plugins/instantiate/stdlib/calloc.mli: CEA_LGPL_OR_PROPRIETARY
src/plugins/builtin/stdlib/free.ml: CEA_LGPL_OR_PROPRIETARY src/plugins/instantiate/stdlib/free.ml: CEA_LGPL_OR_PROPRIETARY
src/plugins/builtin/stdlib/free.mli: CEA_LGPL_OR_PROPRIETARY src/plugins/instantiate/stdlib/free.mli: CEA_LGPL_OR_PROPRIETARY
src/plugins/builtin/stdlib/malloc.ml: CEA_LGPL_OR_PROPRIETARY src/plugins/instantiate/stdlib/malloc.ml: CEA_LGPL_OR_PROPRIETARY
src/plugins/builtin/stdlib/malloc.mli: CEA_LGPL_OR_PROPRIETARY src/plugins/instantiate/stdlib/malloc.mli: CEA_LGPL_OR_PROPRIETARY
src/plugins/builtin/string/memcmp.ml: CEA_LGPL_OR_PROPRIETARY src/plugins/instantiate/string/memcmp.ml: CEA_LGPL_OR_PROPRIETARY
src/plugins/builtin/string/memcmp.mli: CEA_LGPL_OR_PROPRIETARY src/plugins/instantiate/string/memcmp.mli: CEA_LGPL_OR_PROPRIETARY
src/plugins/builtin/string/memcpy.ml: CEA_LGPL_OR_PROPRIETARY src/plugins/instantiate/string/memcpy.ml: CEA_LGPL_OR_PROPRIETARY
src/plugins/builtin/string/memcpy.mli: CEA_LGPL_OR_PROPRIETARY src/plugins/instantiate/string/memcpy.mli: CEA_LGPL_OR_PROPRIETARY
src/plugins/builtin/string/memmove.ml: CEA_LGPL_OR_PROPRIETARY src/plugins/instantiate/string/memmove.ml: CEA_LGPL_OR_PROPRIETARY
src/plugins/builtin/string/memmove.mli: CEA_LGPL_OR_PROPRIETARY src/plugins/instantiate/string/memmove.mli: CEA_LGPL_OR_PROPRIETARY
src/plugins/builtin/string/memset.ml: CEA_LGPL_OR_PROPRIETARY src/plugins/instantiate/string/memset.ml: CEA_LGPL_OR_PROPRIETARY
src/plugins/builtin/string/memset.mli: CEA_LGPL_OR_PROPRIETARY src/plugins/instantiate/string/memset.mli: CEA_LGPL_OR_PROPRIETARY
src/plugins/builtin/options.ml: CEA_LGPL_OR_PROPRIETARY src/plugins/instantiate/options.ml: CEA_LGPL_OR_PROPRIETARY
src/plugins/builtin/options.mli: CEA_LGPL_OR_PROPRIETARY src/plugins/instantiate/options.mli: CEA_LGPL_OR_PROPRIETARY
src/plugins/builtin/register.ml: CEA_LGPL_OR_PROPRIETARY src/plugins/instantiate/register.ml: CEA_LGPL_OR_PROPRIETARY
src/plugins/builtin/transform.ml: CEA_LGPL_OR_PROPRIETARY src/plugins/instantiate/transform.ml: CEA_LGPL_OR_PROPRIETARY
src/plugins/builtin/transform.mli: CEA_LGPL_OR_PROPRIETARY src/plugins/instantiate/transform.mli: CEA_LGPL_OR_PROPRIETARY
src/plugins/pdg/Pdg.mli: CEA_LGPL_OR_PROPRIETARY src/plugins/pdg/Pdg.mli: CEA_LGPL_OR_PROPRIETARY
src/plugins/pdg/annot.ml: CEA_LGPL_OR_PROPRIETARY src/plugins/pdg/annot.ml: CEA_LGPL_OR_PROPRIETARY
src/plugins/pdg/annot.mli: CEA_LGPL_OR_PROPRIETARY src/plugins/pdg/annot.mli: CEA_LGPL_OR_PROPRIETARY
......
OPT: @PTEST_FILE@ -builtin -print
\ No newline at end of file
OPT: @PTEST_FILE@ -builtin -print -check -then -ocode @PTEST_DIR@/result/@PTEST_NAME@.c -print -then -no-builtin @PTEST_DIR@/result/@PTEST_NAME@.c @PTEST_FILE@ -ocode="" -print
\ No newline at end of file
OPT: @PTEST_FILE@ -builtin -print -check -then -ocode @PTEST_DIR@/result/@PTEST_NAME@.c -print -then -no-builtin @PTEST_DIR@/result/@PTEST_NAME@.c @PTEST_FILE@ -ocode="" -print
\ No newline at end of file
File moved
...@@ -22,23 +22,23 @@ ...@@ -22,23 +22,23 @@
open Cil_types open Cil_types
module Builtin_builder: sig module Instantiator_builder: sig
(** Builds a [Builtin] module (used by [Transform]) from a [Generator_sig] *) (** Builds a [Instantiator] module (used by [Transform]) from a [Generator_sig] *)
(** Signature for a new builtin generator. (** Signature for a new instantiator generator.
In order to support a new function, this module must be implemented and In order to support a new function, this module must be implemented and
registered to the [Transform] module. registered to the [Transform] module.
*) *)
module type Generator_sig = sig module type Generator_sig = sig
(** [Hashtbl] module used by the [Make_builtin] module to generate the (** [Hashtbl] module used by the [Make_instantiator] module to generate the
function cache. The [key] ([override_key]) must identify a function function cache. The [key] ([override_key]) must identify a function
override. override.
*) *)
module Hashtbl: Datatype.Hashtbl module Hashtbl: Datatype.Hashtbl
type override_key = Hashtbl.key type override_key = Hashtbl.key
(** Name of the implemented builtin function *) (** Name of the implemented instantiator function *)
val function_name: string val function_name: string
(** [well_typed_call ret args] must return true if and only if the (** [well_typed_call ret args] must return true if and only if the
...@@ -68,13 +68,13 @@ module Builtin_builder: sig ...@@ -68,13 +68,13 @@ module Builtin_builder: sig
override function such that the new list can be given to the original override function such that the new list can be given to the original
function. For example, if for a call: function. For example, if for a call:
[foo(x, 0, z)] [foo(x, 0, z)]
The [Builtin] module generates an override: The [Instantiator] module generates an override:
[void foo_spec(int p1, int p3);] [void foo_spec(int p1, int p3);]
The received list of expression is [ p1 ; p3 ] and thus the returned list The received list of expression is [ p1 ; p3 ] and thus the returned list
should be [ p1 ; 0 ; p3 ] (so that the replaced call [foo_spec(x, z)] has should be [ p1 ; 0 ; p3 ] (so that the replaced call [foo_spec(x, z)] has
the same behavior). the same behavior).
Note that there is no need to introduce casts to the original type, it is Note that there is no need to introduce casts to the original type, it is
introduced by [Make_builtin]. introduced by [Make_instantiator].
*) *)
val args_for_original: override_key -> exp list -> exp list val args_for_original: override_key -> exp list -> exp list
...@@ -93,9 +93,9 @@ module Builtin_builder: sig ...@@ -93,9 +93,9 @@ module Builtin_builder: sig
end end
module Transform: sig module Transform: sig
(** Registers a new [Builtin] to the visitor from the [Generator_sig] module (** Registers a new [Instantiator] to the visitor from the [Generator_sig] module
of this builtin. Each new builtin function generator should call this of this instantiator. Each new instantiator function generator should call this
globally. globally.
*) *)
val register: (module Builtin_builder.Generator_sig) -> unit val register: (module Instantiator_builder.Generator_sig) -> unit
end end
...@@ -48,14 +48,14 @@ SRC_STDLIB:=$(addprefix stdlib/, $(SRC_STDLIB)) ...@@ -48,14 +48,14 @@ SRC_STDLIB:=$(addprefix stdlib/, $(SRC_STDLIB))
################### ###################
PLUGIN_DIR ?= . PLUGIN_DIR ?= .
PLUGIN_ENABLE := @ENABLE_BUILTIN@ PLUGIN_ENABLE := @ENABLE_INSTANTIATE@
PLUGIN_NAME := Builtin PLUGIN_NAME := Instantiate
PLUGIN_EXTRA_DIRS:=\ PLUGIN_EXTRA_DIRS:=\
string\ string\
stdlib stdlib
PLUGIN_CMI := PLUGIN_CMI :=
PLUGIN_CMO := \ PLUGIN_CMO := \
basic_blocks options builtin_builder transform register \ basic_blocks options instantiator_builder transform register \
$(SRC_STRING) \ $(SRC_STRING) \
$(SRC_STDLIB) $(SRC_STDLIB)
...@@ -80,6 +80,6 @@ else ...@@ -80,6 +80,6 @@ else
CONFIG_STATUS_DIR=. CONFIG_STATUS_DIR=.
endif endif
$(Builtin_DIR)/Makefile: $(Builtin_DIR)/Makefile.in \ $(Instantiate_DIR)/Makefile: $(Instantiate_DIR)/Makefile.in \
$(CONFIG_STATUS_DIR)/config.status $(CONFIG_STATUS_DIR)/config.status
cd $(CONFIG_STATUS_DIR) && ./config.status --file $@ cd $(CONFIG_STATUS_DIR) && ./config.status --file $@
# Builtin plugin # Instantiate plugin
## Description ## Description
The builtin plugin is meant to build function specialization to C functions in The instantiate plugin is meant to build function specialization to C functions
a project when the original specification (or prototype of the function) cannot in a project when the original specification (or prototype of the function)
be efficiently used by some plugins. cannot be efficiently used by some plugins.
For example, the specification of a function like `memcpy` cannot be efficiently For example, the specification of a function like `memcpy` cannot be efficiently
used by a plugin like `WP` because it involves `void*` pointers and requires to used by a plugin like `WP` because it involves `void*` pointers and requires to
...@@ -26,20 +26,20 @@ int *memcpy_int(int *dest, int const *src, size_t len); ...@@ -26,20 +26,20 @@ int *memcpy_int(int *dest, int const *src, size_t len);
That can be easily used by WP. That can be easily used by WP.
The role of the Builtin plugin is to find each call for which a builtin The role of the Instantiate plugin is to find each call for which an
generator is available and if the call is well-typed according to the generator instantiation generator is available and if the call is well-typed according to
(for example for `memcpy` if `dest` and `src` are both pointers to integers), to the generator (for example for `memcpy` if `dest` and `src` are both pointers to
replace this call with a call to an automatically generated specialization integers), to replace this call with a call to an automatically generated
corresponding to the call. specialization corresponding to the call.
## Options ## Options
- `-builtin` activate builtin replacement - `-instantiate` activate function replacement
- `-builtin-fct=f,...` only replace calls in specified functions (default to all) - `-instantiate-fct=f,...` only replace calls in specified functions (default to all)
- `-builtin-(no-)<function_name>` activate or deactivate a particular builtin - `-instantiate-(no-)<function_name>` activate or deactivate a particular
function, defaults to true. function, defaults to true.
## Available builtin functions ## Available functions
### `string.h` ### `string.h`
...@@ -106,10 +106,10 @@ void* malloc(size_t num, size_t size); ...@@ -106,10 +106,10 @@ void* malloc(size_t num, size_t size);
`malloc` is typed according to the variable that receives its result. It must be `malloc` is typed according to the variable that receives its result. It must be
a pointer type different from `void*`. a pointer type different from `void*`.
## Adding a new builtin ## Adding a new instantiator
To add a new builtin, one has to register a new module implementing the To add a new instantiator, one has to register a new module implementing the
`Builtin_builder.Generator_sig` module type to the `Transform` module. The `Instantiator_builder.Generator_sig` module type to the `Transform` module. The
lasts steps of the code should look like: lasts steps of the code should look like:
```ocaml ```ocaml
...@@ -117,7 +117,7 @@ let () = Transform.register (module struct ...@@ -117,7 +117,7 @@ let () = Transform.register (module struct
module Hashtbl = ... module Hashtbl = ...
type override_key = ... type override_key = ...
let function_name = "my_builtin_function" let function_name = "my_function"
let well_typed_call = ... let well_typed_call = ...
let key_from_call = ... let key_from_call = ...
let retype_args = ... let retype_args = ...
...@@ -127,4 +127,4 @@ let () = Transform.register (module struct ...@@ -127,4 +127,4 @@ let () = Transform.register (module struct
end) end)
``` ```
The role and types of each function is documented in `Builtin_builder.mli`. The role and types of each function is documented in `Instantiator_builder.mli`.
\ No newline at end of file \ No newline at end of file
...@@ -21,7 +21,7 @@ ...@@ -21,7 +21,7 @@
########################################################################## ##########################################################################
########################################## ##########################################
# Builtin stdlib C plugin # # Instantiate plugin #
########################################## ##########################################
m4_define([plugin_file],Makefile.in) m4_define([plugin_file],Makefile.in)
...@@ -35,8 +35,8 @@ m4_define([FRAMAC_SHARE], ...@@ -35,8 +35,8 @@ m4_define([FRAMAC_SHARE],
m4_ifndef([FRAMAC_M4_MACROS], [m4_include(FRAMAC_SHARE/configure.ac)]) m4_ifndef([FRAMAC_M4_MACROS], [m4_include(FRAMAC_SHARE/configure.ac)])
check_plugin(builtin,PLUGIN_RELATIVE_PATH(plugin_file), check_plugin(instantiate,PLUGIN_RELATIVE_PATH(plugin_file),
[support for builtin plug-in],yes) [support for instantiate plug-in],yes)
####################### #######################
# Generating Makefile # # Generating Makefile #
......
...@@ -36,7 +36,7 @@ module type Generator_sig = sig ...@@ -36,7 +36,7 @@ module type Generator_sig = sig
val generate_spec: override_key -> fundec -> location -> funspec val generate_spec: override_key -> fundec -> location -> funspec
end end
module type Builtin = sig module type Instantiator = sig
module Enabled: Parameter_sig.Bool module Enabled: Parameter_sig.Bool
type override_key type override_key
...@@ -65,14 +65,14 @@ let build_body caller callee args_generator = ...@@ -65,14 +65,14 @@ let build_body caller callee args_generator =
let ret = Cil.mkStmt (Return ( (opt_map Cil.evar ret_var), loc)) in let ret = Cil.mkStmt (Return ( (opt_map Cil.evar ret_var), loc)) in
{ (Cil.mkBlock [call ; ret]) with blocals = list_of_opt ret_var } { (Cil.mkBlock [call ; ret]) with blocals = list_of_opt ret_var }
module Make_builtin (G: Generator_sig) = struct module Make_instantiator (G: Generator_sig) = struct
include G include G
module Enabled = Options.NewBuiltin (G) module Enabled = Options.NewInstantiator (G)
module Cache = State_builder.Hashtbl (G.Hashtbl) (Cil_datatype.Fundec) module Cache = State_builder.Hashtbl (G.Hashtbl) (Cil_datatype.Fundec)
(struct (struct
let size = 5 let size = 5
let dependencies = [] let dependencies = []
let name = "Builtins." ^ G.function_name let name = "Instantiator." ^ G.function_name
end) end)
let make_fundec key = let make_fundec key =
......
...@@ -22,22 +22,22 @@ ...@@ -22,22 +22,22 @@
open Cil_types open Cil_types
(** Builds a [Builtin] module (used by [Transform]) from a [Generator_sig] *) (** Builds a [Instantiator] module (used by [Transform]) from a [Generator_sig] *)
(** Signature for a new builtin generator. (** Signature for a new instantiator generator.
In order to support a new function, this module must be implemented and In order to support a new function, this module must be implemented and
registered to the [Transform] module. registered to the [Transform] module.
*) *)
module type Generator_sig = sig module type Generator_sig = sig
(** [Hashtbl] module used by the [Make_builtin] module to generate the (** [Hashtbl] module used by the [Make_instantiator] module to generate the
function cache. The [key] ([override_key]) must identify a function function cache. The [key] ([override_key]) must identify a function
override. override.
*) *)
module Hashtbl: Datatype.Hashtbl module Hashtbl: Datatype.Hashtbl
type override_key = Hashtbl.key type override_key = Hashtbl.key
(** Name of the implemented builtin function *) (** Name of the implemented function *)
val function_name: string val function_name: string
(** [well_typed_call ret args] must return true if and only if the (** [well_typed_call ret args] must return true if and only if the
...@@ -67,13 +67,13 @@ module type Generator_sig = sig ...@@ -67,13 +67,13 @@ module type Generator_sig = sig
override function such that the new list can be given to the original override function such that the new list can be given to the original
function. For example, if for a call: function. For example, if for a call:
[foo(x, 0, z)] [foo(x, 0, z)]
The [Builtin] module generates an override: The [Instantiator] module generates an override:
[void foo_spec(int p1, int p3);] [void foo_spec(int p1, int p3);]
The received list of expression is [ p1 ; p3 ] and thus the returned list The received list of expression is [ p1 ; p3 ] and thus the returned list
should be [ p1 ; 0 ; p3 ] (so that the replaced call [foo_spec(x, z)] has should be [ p1 ; 0 ; p3 ] (so that the replaced call [foo_spec(x, z)] has
the same behavior). the same behavior).
Note that there is no need to introduce casts to the original type, it is Note that there is no need to introduce casts to the original type, it is
introduced by [Make_builtin]. introduced by [Make_instantiator].
*) *)
val args_for_original: override_key -> exp list -> exp list val args_for_original: override_key -> exp list -> exp list
...@@ -90,14 +90,14 @@ module type Generator_sig = sig ...@@ -90,14 +90,14 @@ module type Generator_sig = sig
val generate_spec: override_key -> fundec -> location -> funspec val generate_spec: override_key -> fundec -> location -> funspec
end end
(** Signature of a builtin. (** Signature of a instantiator.
Module built by the [Transform] module from a [Generator_sig]. Module built by the [Transform] module from a [Generator_sig].
Used to perform the different replacements in the AST. This Used to perform the different replacements in the AST. This
should be only used by the [Transform] module. should be only used by the [Transform] module.
*) *)
module type Builtin = sig module type Instantiator = sig
(** Plugin option that allows to check whether the builtin is enabled. *) (** Plugin option that allows to check whether the instantiator is enabled. *)
module Enabled: Parameter_sig.Bool module Enabled: Parameter_sig.Bool
(** Same as [Generator_sig.override_key] *) (** Same as [Generator_sig.override_key] *)
type override_key type override_key
...@@ -129,8 +129,8 @@ module type Builtin = sig ...@@ -129,8 +129,8 @@ module type Builtin = sig
val mark_as_computed: ?project:Project.t -> unit -> unit val mark_as_computed: ?project:Project.t -> unit -> unit
end end
(** Generates a [Builtin] from a [Generator_sig] adding all necessary stuff for (** Generates a [Instantiator] from a [Generator_sig] adding all necessary stuff for
cache and function definition generation, as well as specification cache and function definition generation, as well as specification
registration. This should only be used by [Transform]. registration. This should only be used by [Transform].
*) *)
module Make_builtin (G: Generator_sig) : Builtin module Make_instantiator (G: Generator_sig) : Instantiator
...@@ -20,8 +20,8 @@ ...@@ -20,8 +20,8 @@
(* *) (* *)
(**************************************************************************) (**************************************************************************)
let name = "Builtin" let name = "Instantiate"
let shortname = "builtin" let shortname = "instantiate"
include Plugin.Register include Plugin.Register
(struct (struct
...@@ -44,10 +44,10 @@ module Kfs = ...@@ -44,10 +44,10 @@ module Kfs =
let help = "Override stdlib functions only into the specified functions (defaults to all)." let help = "Override stdlib functions only into the specified functions (defaults to all)."
end) end)
module NewBuiltin (B: sig val function_name: string end) = True module NewInstantiator (I: sig val function_name: string end) = True
(struct (struct
let option_name = "-" ^ shortname ^ "-" ^ B.function_name let option_name = "-" ^ shortname ^ "-" ^ I.function_name
let help = "Activate replacement for function '" ^ B.function_name ^ "'" let help = "Activate replacement for function '" ^ I.function_name ^ "'"
end) end)
let emitter = Emitter.create shortname [Emitter.Funspec] ~correctness:[] ~tuning:[] let emitter = Emitter.create shortname [Emitter.Funspec] ~correctness:[] ~tuning:[]
...@@ -22,17 +22,17 @@ ...@@ -22,17 +22,17 @@
include Plugin.General_services include Plugin.General_services
(** Builtin transformtion enabled *) (** Instantiate transformation enabled *)
module Enabled : Parameter_sig.Bool module Enabled : Parameter_sig.Bool
(** Set of kernel function provided for transformation *) (** Set of kernel function provided for transformation *)
module Kfs : Parameter_sig.Kernel_function_set module Kfs : Parameter_sig.Kernel_function_set
(** Used by [Builtin_builder] to generate builtin options. For a given builtin, (** Used by [Instantiator_builder] to generate options. For a given instantiator
the module generate an option "-builtin-(no-)<function_name>" that defaults the module generates an option "-instantiate-(no-)<function_name>" that defaults
to true. to true.
*) *)
module NewBuiltin module NewInstantiator
(B : sig val function_name: string end) : Parameter_sig.Bool (B : sig val function_name: string end) : Parameter_sig.Bool
val emitter: Emitter.t val emitter: Emitter.t
...@@ -20,7 +20,7 @@ ...@@ -20,7 +20,7 @@
(* *) (* *)
(**************************************************************************) (**************************************************************************)
let category = File.register_code_transformation_category "builtin" let category = File.register_code_transformation_category "instantiate"
let () = let () =
let perform file = let perform file =
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment