Improve layout history error message: review new messages #2091
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
goldfirere
reviewed
Dec 6, 2023
ccasin
reviewed
Dec 12, 2023
alanechang
commented
Dec 12, 2023
ccasin
reviewed
Dec 12, 2023
ccasin
reviewed
Dec 12, 2023
ccasin
reviewed
Dec 12, 2023
alanechang
commented
Dec 12, 2023
ccasin
reviewed
Dec 12, 2023
ccasin
reviewed
Dec 12, 2023
goldfirere
reviewed
Dec 12, 2023
alanechang
force-pushed
the
aec/remove-crs
branch
from
aff63f0 to
173e6bb
Compare
|
I went through all the feedback addressing the ones that are straightforward to fix and filing tickets for all the rest. It would be helpful to do a quick review of the changes in this PR. Then we can rebase and merge the |
goldfirere
reviewed
Dec 19, 2023
alanechang
requested review from
lpw25,
riaqn,
stedolan,
mshinwell,
poechsel,
gretay-js,
xclerc and
lukemaurer
as code owners
|
@goldfirere please review |
goldfirere
reviewed
Dec 26, 2023
goldfirere
approved these changes
Dec 27, 2023
alanechang
added a commit
to alanechang/flambda-backend
that referenced
this pull request
Dec 27, 2023
* enable display_histories * accept test changes and mark them as unreviewed * improve CR messages for unreviewed tests --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Remove duplicated creation_reasons from layout history error messages (ocaml-flambda#1825) * stop storing duplicates in creation_reasons * accept test changes * reduce to only displaying one creation reason * accept test changes * prefer other reasons over Concrete_creation * accept test changes * stop building history tree if flatten is enabled * remove CR version Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> * Add CR for assumption in combine_histories Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> Improve layout history error message formatting and wording (ocaml-flambda#1832) * remove extra whitespace * fix Unequal_var_layouts error fmt * improve message wording * fix spacing * accept test changes * grammar and phrasing * test changes --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Improve layout history error message: add path field to Type_argument value_creation_reason (ocaml-flambda#1834) * add Path.t to Type_argument * test changes * add Imported_type_argument * Imported_type_argument for imported type params * fix float64 error format * test changes * add more tests * update layout reason not just in failure case * tests with more than one type param * combine Imported_type_argument with Type_argument * add option_argument_layout to predef * track position and arity for Type_argument * test changes * more tests around typa argument arity * have Imported_type_argument as a creation_reason * test changes * add note about performance benchmark * extract position format logic into function * add comments about 1-indexed * change equals to is * test changes * define list_argument_layout * format comments --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Improve layout history error message: additional info on value layout defaulting (ocaml-flambda#1855) * add message when defaulting to layout value * test changes * relax to all const defaulting * test changes * improve wording * test changes --------- Co-authored-by: alanechang <alanechang@janestreet.com> Improve layout history error message: change message wording and add more tests (ocaml-flambda#1887) * change imported reason wording * test changes * lowercase * test changes * add more tests * change wording for match * update Immediate_polymorphic_variant message * test changes Improve layout history error message: hide layout history reason of various generalized vars (ocaml-flambda#1889) * add generalized creation reason * use change layout reason for value decl * change layout reason for decl type params * test changes * additional tests * add "the" * test changes * improve generalize on type decl * test changes * update err msg test case * add print_loc_in_lowercase * test changes * update_decls_layout_reason code cleanup Improve layout history error message: better test coverage (ocaml-flambda#1924) * Annotation * concrete layout * value layout * immediate layout * float64 layout * the rest * fix probe test * fix tests * more changes * more value test * add note * remove unused With_constraint constructor * fix format * remove unused void_creation_reason * remove unused Value_kind * add Missing_cmi test * consistent names for Type_variable&Univar * add param to Unannotated_type_parameter * add CRs * fix wording * add warning to all reasons we can't produce * more CRs * fix formatting * add new tests * enhance creation reason when missing cmi * fix Unannotated_type_parameter * test changes * code formatting (only) * Record missing cmi in the layout history * Add a CR * add broken test * fix is_missing_cmi check --------- Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> Support for [@error_message] attribute on layout annotations (ocaml-flambda#1943) support for error_message attr on layout annots add error_message to the builtin_attrs table change error_message_attr to return first refactor - layout_of_annotation_or_attr to layout_of_annotation - Err_msg_attr to With_error_message fix tests improve formatting Support for [@error_message] attribute on Pexp_constraint (ocaml-flambda#1954) * implement error_message on Pexp_constraint * add tests * force message to be on new line * fix typo Improve layout history error message: omit history for signature mismatch (ocaml-flambda#2089) strip away layout history for signature mismatch Layout history: mark `typing-immediate/immediate.ml` as reviewed (ocaml-flambda#2144) mark test file as reviewed Improve layout history error message: review new messages (ocaml-flambda#2091) * remove layouts v2.9 CRs * "must be representable" instead of var name * catch missing history generalization * add history to Bad_univar_layout * remove crs * Toplevel_nonvalue refer to types * change Non_value_in_sig to refer to types * fix typo of Polymorphic * type argument to a class constructor * change message of Not_a_value * remove stale test file * refer to types in all messages * update unannotated universal variable message * split into Class_type_argument&Class_term_argument * add comment on update_generalized_ty_layout_reason * undo change to Not_a_value * update Not_a_value message
alanechang
added a commit
to alanechang/flambda-backend
that referenced
this pull request
Dec 27, 2023
* enable display_histories * accept test changes and mark them as unreviewed * improve CR messages for unreviewed tests --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Remove duplicated creation_reasons from layout history error messages (ocaml-flambda#1825) * stop storing duplicates in creation_reasons * accept test changes * reduce to only displaying one creation reason * accept test changes * prefer other reasons over Concrete_creation * accept test changes * stop building history tree if flatten is enabled * remove CR version Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> * Add CR for assumption in combine_histories Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> Improve layout history error message formatting and wording (ocaml-flambda#1832) * remove extra whitespace * fix Unequal_var_layouts error fmt * improve message wording * fix spacing * accept test changes * grammar and phrasing * test changes --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Improve layout history error message: add path field to Type_argument value_creation_reason (ocaml-flambda#1834) * add Path.t to Type_argument * test changes * add Imported_type_argument * Imported_type_argument for imported type params * fix float64 error format * test changes * add more tests * update layout reason not just in failure case * tests with more than one type param * combine Imported_type_argument with Type_argument * add option_argument_layout to predef * track position and arity for Type_argument * test changes * more tests around typa argument arity * have Imported_type_argument as a creation_reason * test changes * add note about performance benchmark * extract position format logic into function * add comments about 1-indexed * change equals to is * test changes * define list_argument_layout * format comments --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Improve layout history error message: additional info on value layout defaulting (ocaml-flambda#1855) * add message when defaulting to layout value * test changes * relax to all const defaulting * test changes * improve wording * test changes --------- Co-authored-by: alanechang <alanechang@janestreet.com> Improve layout history error message: change message wording and add more tests (ocaml-flambda#1887) * change imported reason wording * test changes * lowercase * test changes * add more tests * change wording for match * update Immediate_polymorphic_variant message * test changes Improve layout history error message: hide layout history reason of various generalized vars (ocaml-flambda#1889) * add generalized creation reason * use change layout reason for value decl * change layout reason for decl type params * test changes * additional tests * add "the" * test changes * improve generalize on type decl * test changes * update err msg test case * add print_loc_in_lowercase * test changes * update_decls_layout_reason code cleanup Improve layout history error message: better test coverage (ocaml-flambda#1924) * Annotation * concrete layout * value layout * immediate layout * float64 layout * the rest * fix probe test * fix tests * more changes * more value test * add note * remove unused With_constraint constructor * fix format * remove unused void_creation_reason * remove unused Value_kind * add Missing_cmi test * consistent names for Type_variable&Univar * add param to Unannotated_type_parameter * add CRs * fix wording * add warning to all reasons we can't produce * more CRs * fix formatting * add new tests * enhance creation reason when missing cmi * fix Unannotated_type_parameter * test changes * code formatting (only) * Record missing cmi in the layout history * Add a CR * add broken test * fix is_missing_cmi check --------- Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> Support for [@error_message] attribute on layout annotations (ocaml-flambda#1943) support for error_message attr on layout annots add error_message to the builtin_attrs table change error_message_attr to return first refactor - layout_of_annotation_or_attr to layout_of_annotation - Err_msg_attr to With_error_message fix tests improve formatting Support for [@error_message] attribute on Pexp_constraint (ocaml-flambda#1954) * implement error_message on Pexp_constraint * add tests * force message to be on new line * fix typo Improve layout history error message: omit history for signature mismatch (ocaml-flambda#2089) strip away layout history for signature mismatch Layout history: mark `typing-immediate/immediate.ml` as reviewed (ocaml-flambda#2144) mark test file as reviewed Improve layout history error message: review new messages (ocaml-flambda#2091) * remove layouts v2.9 CRs * "must be representable" instead of var name * catch missing history generalization * add history to Bad_univar_layout * remove crs * Toplevel_nonvalue refer to types * change Non_value_in_sig to refer to types * fix typo of Polymorphic * type argument to a class constructor * change message of Not_a_value * remove stale test file * refer to types in all messages * update unannotated universal variable message * split into Class_type_argument&Class_term_argument * add comment on update_generalized_ty_layout_reason * undo change to Not_a_value * update Not_a_value message fix merge conflicts
alanechang
added a commit
to alanechang/flambda-backend
that referenced
this pull request
Dec 27, 2023
* enable display_histories * accept test changes and mark them as unreviewed * improve CR messages for unreviewed tests --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Remove duplicated creation_reasons from layout history error messages (ocaml-flambda#1825) * stop storing duplicates in creation_reasons * accept test changes * reduce to only displaying one creation reason * accept test changes * prefer other reasons over Concrete_creation * accept test changes * stop building history tree if flatten is enabled * remove CR version Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> * Add CR for assumption in combine_histories Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> Improve layout history error message formatting and wording (ocaml-flambda#1832) * remove extra whitespace * fix Unequal_var_layouts error fmt * improve message wording * fix spacing * accept test changes * grammar and phrasing * test changes --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Improve layout history error message: add path field to Type_argument value_creation_reason (ocaml-flambda#1834) * add Path.t to Type_argument * test changes * add Imported_type_argument * Imported_type_argument for imported type params * fix float64 error format * test changes * add more tests * update layout reason not just in failure case * tests with more than one type param * combine Imported_type_argument with Type_argument * add option_argument_layout to predef * track position and arity for Type_argument * test changes * more tests around typa argument arity * have Imported_type_argument as a creation_reason * test changes * add note about performance benchmark * extract position format logic into function * add comments about 1-indexed * change equals to is * test changes * define list_argument_layout * format comments --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Improve layout history error message: additional info on value layout defaulting (ocaml-flambda#1855) * add message when defaulting to layout value * test changes * relax to all const defaulting * test changes * improve wording * test changes --------- Co-authored-by: alanechang <alanechang@janestreet.com> Improve layout history error message: change message wording and add more tests (ocaml-flambda#1887) * change imported reason wording * test changes * lowercase * test changes * add more tests * change wording for match * update Immediate_polymorphic_variant message * test changes Improve layout history error message: hide layout history reason of various generalized vars (ocaml-flambda#1889) * add generalized creation reason * use change layout reason for value decl * change layout reason for decl type params * test changes * additional tests * add "the" * test changes * improve generalize on type decl * test changes * update err msg test case * add print_loc_in_lowercase * test changes * update_decls_layout_reason code cleanup Improve layout history error message: better test coverage (ocaml-flambda#1924) * Annotation * concrete layout * value layout * immediate layout * float64 layout * the rest * fix probe test * fix tests * more changes * more value test * add note * remove unused With_constraint constructor * fix format * remove unused void_creation_reason * remove unused Value_kind * add Missing_cmi test * consistent names for Type_variable&Univar * add param to Unannotated_type_parameter * add CRs * fix wording * add warning to all reasons we can't produce * more CRs * fix formatting * add new tests * enhance creation reason when missing cmi * fix Unannotated_type_parameter * test changes * code formatting (only) * Record missing cmi in the layout history * Add a CR * add broken test * fix is_missing_cmi check --------- Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> Support for [@error_message] attribute on layout annotations (ocaml-flambda#1943) support for error_message attr on layout annots add error_message to the builtin_attrs table change error_message_attr to return first refactor - layout_of_annotation_or_attr to layout_of_annotation - Err_msg_attr to With_error_message fix tests improve formatting Support for [@error_message] attribute on Pexp_constraint (ocaml-flambda#1954) * implement error_message on Pexp_constraint * add tests * force message to be on new line * fix typo Improve layout history error message: omit history for signature mismatch (ocaml-flambda#2089) strip away layout history for signature mismatch Layout history: mark `typing-immediate/immediate.ml` as reviewed (ocaml-flambda#2144) mark test file as reviewed Improve layout history error message: review new messages (ocaml-flambda#2091) * remove layouts v2.9 CRs * "must be representable" instead of var name * catch missing history generalization * add history to Bad_univar_layout * remove crs * Toplevel_nonvalue refer to types * change Non_value_in_sig to refer to types * fix typo of Polymorphic * type argument to a class constructor * change message of Not_a_value * remove stale test file * refer to types in all messages * update unannotated universal variable message * split into Class_type_argument&Class_term_argument * add comment on update_generalized_ty_layout_reason * undo change to Not_a_value * update Not_a_value message fix merge conflicts
alanechang
added a commit
to alanechang/flambda-backend
that referenced
this pull request
Dec 27, 2023
* enable display_histories * accept test changes and mark them as unreviewed * improve CR messages for unreviewed tests --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Remove duplicated creation_reasons from layout history error messages (ocaml-flambda#1825) * stop storing duplicates in creation_reasons * accept test changes * reduce to only displaying one creation reason * accept test changes * prefer other reasons over Concrete_creation * accept test changes * stop building history tree if flatten is enabled * remove CR version Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> * Add CR for assumption in combine_histories Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> Improve layout history error message formatting and wording (ocaml-flambda#1832) * remove extra whitespace * fix Unequal_var_layouts error fmt * improve message wording * fix spacing * accept test changes * grammar and phrasing * test changes --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Improve layout history error message: add path field to Type_argument value_creation_reason (ocaml-flambda#1834) * add Path.t to Type_argument * test changes * add Imported_type_argument * Imported_type_argument for imported type params * fix float64 error format * test changes * add more tests * update layout reason not just in failure case * tests with more than one type param * combine Imported_type_argument with Type_argument * add option_argument_layout to predef * track position and arity for Type_argument * test changes * more tests around typa argument arity * have Imported_type_argument as a creation_reason * test changes * add note about performance benchmark * extract position format logic into function * add comments about 1-indexed * change equals to is * test changes * define list_argument_layout * format comments --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Improve layout history error message: additional info on value layout defaulting (ocaml-flambda#1855) * add message when defaulting to layout value * test changes * relax to all const defaulting * test changes * improve wording * test changes --------- Co-authored-by: alanechang <alanechang@janestreet.com> Improve layout history error message: change message wording and add more tests (ocaml-flambda#1887) * change imported reason wording * test changes * lowercase * test changes * add more tests * change wording for match * update Immediate_polymorphic_variant message * test changes Improve layout history error message: hide layout history reason of various generalized vars (ocaml-flambda#1889) * add generalized creation reason * use change layout reason for value decl * change layout reason for decl type params * test changes * additional tests * add "the" * test changes * improve generalize on type decl * test changes * update err msg test case * add print_loc_in_lowercase * test changes * update_decls_layout_reason code cleanup Improve layout history error message: better test coverage (ocaml-flambda#1924) * Annotation * concrete layout * value layout * immediate layout * float64 layout * the rest * fix probe test * fix tests * more changes * more value test * add note * remove unused With_constraint constructor * fix format * remove unused void_creation_reason * remove unused Value_kind * add Missing_cmi test * consistent names for Type_variable&Univar * add param to Unannotated_type_parameter * add CRs * fix wording * add warning to all reasons we can't produce * more CRs * fix formatting * add new tests * enhance creation reason when missing cmi * fix Unannotated_type_parameter * test changes * code formatting (only) * Record missing cmi in the layout history * Add a CR * add broken test * fix is_missing_cmi check --------- Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> Support for [@error_message] attribute on layout annotations (ocaml-flambda#1943) support for error_message attr on layout annots add error_message to the builtin_attrs table change error_message_attr to return first refactor - layout_of_annotation_or_attr to layout_of_annotation - Err_msg_attr to With_error_message fix tests improve formatting Support for [@error_message] attribute on Pexp_constraint (ocaml-flambda#1954) * implement error_message on Pexp_constraint * add tests * force message to be on new line * fix typo Improve layout history error message: omit history for signature mismatch (ocaml-flambda#2089) strip away layout history for signature mismatch Layout history: mark `typing-immediate/immediate.ml` as reviewed (ocaml-flambda#2144) mark test file as reviewed Improve layout history error message: review new messages (ocaml-flambda#2091) * remove layouts v2.9 CRs * "must be representable" instead of var name * catch missing history generalization * add history to Bad_univar_layout * remove crs * Toplevel_nonvalue refer to types * change Non_value_in_sig to refer to types * fix typo of Polymorphic * type argument to a class constructor * change message of Not_a_value * remove stale test file * refer to types in all messages * update unannotated universal variable message * split into Class_type_argument&Class_term_argument * add comment on update_generalized_ty_layout_reason * undo change to Not_a_value * update Not_a_value message fix merge conflicts
alanechang
added a commit
to alanechang/flambda-backend
that referenced
this pull request
Dec 27, 2023
* enable display_histories * accept test changes and mark them as unreviewed * improve CR messages for unreviewed tests --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Remove duplicated creation_reasons from layout history error messages (ocaml-flambda#1825) * stop storing duplicates in creation_reasons * accept test changes * reduce to only displaying one creation reason * accept test changes * prefer other reasons over Concrete_creation * accept test changes * stop building history tree if flatten is enabled * remove CR version Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> * Add CR for assumption in combine_histories Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> Improve layout history error message formatting and wording (ocaml-flambda#1832) * remove extra whitespace * fix Unequal_var_layouts error fmt * improve message wording * fix spacing * accept test changes * grammar and phrasing * test changes --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Improve layout history error message: add path field to Type_argument value_creation_reason (ocaml-flambda#1834) * add Path.t to Type_argument * test changes * add Imported_type_argument * Imported_type_argument for imported type params * fix float64 error format * test changes * add more tests * update layout reason not just in failure case * tests with more than one type param * combine Imported_type_argument with Type_argument * add option_argument_layout to predef * track position and arity for Type_argument * test changes * more tests around typa argument arity * have Imported_type_argument as a creation_reason * test changes * add note about performance benchmark * extract position format logic into function * add comments about 1-indexed * change equals to is * test changes * define list_argument_layout * format comments --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Improve layout history error message: additional info on value layout defaulting (ocaml-flambda#1855) * add message when defaulting to layout value * test changes * relax to all const defaulting * test changes * improve wording * test changes --------- Co-authored-by: alanechang <alanechang@janestreet.com> Improve layout history error message: change message wording and add more tests (ocaml-flambda#1887) * change imported reason wording * test changes * lowercase * test changes * add more tests * change wording for match * update Immediate_polymorphic_variant message * test changes Improve layout history error message: hide layout history reason of various generalized vars (ocaml-flambda#1889) * add generalized creation reason * use change layout reason for value decl * change layout reason for decl type params * test changes * additional tests * add "the" * test changes * improve generalize on type decl * test changes * update err msg test case * add print_loc_in_lowercase * test changes * update_decls_layout_reason code cleanup Improve layout history error message: better test coverage (ocaml-flambda#1924) * Annotation * concrete layout * value layout * immediate layout * float64 layout * the rest * fix probe test * fix tests * more changes * more value test * add note * remove unused With_constraint constructor * fix format * remove unused void_creation_reason * remove unused Value_kind * add Missing_cmi test * consistent names for Type_variable&Univar * add param to Unannotated_type_parameter * add CRs * fix wording * add warning to all reasons we can't produce * more CRs * fix formatting * add new tests * enhance creation reason when missing cmi * fix Unannotated_type_parameter * test changes * code formatting (only) * Record missing cmi in the layout history * Add a CR * add broken test * fix is_missing_cmi check --------- Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> Support for [@error_message] attribute on layout annotations (ocaml-flambda#1943) support for error_message attr on layout annots add error_message to the builtin_attrs table change error_message_attr to return first refactor - layout_of_annotation_or_attr to layout_of_annotation - Err_msg_attr to With_error_message fix tests improve formatting Support for [@error_message] attribute on Pexp_constraint (ocaml-flambda#1954) * implement error_message on Pexp_constraint * add tests * force message to be on new line * fix typo Improve layout history error message: omit history for signature mismatch (ocaml-flambda#2089) strip away layout history for signature mismatch Layout history: mark `typing-immediate/immediate.ml` as reviewed (ocaml-flambda#2144) mark test file as reviewed Improve layout history error message: review new messages (ocaml-flambda#2091) * remove layouts v2.9 CRs * "must be representable" instead of var name * catch missing history generalization * add history to Bad_univar_layout * remove crs * Toplevel_nonvalue refer to types * change Non_value_in_sig to refer to types * fix typo of Polymorphic * type argument to a class constructor * change message of Not_a_value * remove stale test file * refer to types in all messages * update unannotated universal variable message * split into Class_type_argument&Class_term_argument * add comment on update_generalized_ty_layout_reason * undo change to Not_a_value * update Not_a_value message fix merge conflicts
alanechang
added a commit
that referenced
this pull request
Dec 29, 2023
* enable display_histories * accept test changes and mark them as unreviewed * improve CR messages for unreviewed tests --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Remove duplicated creation_reasons from layout history error messages (#1825) * stop storing duplicates in creation_reasons * accept test changes * reduce to only displaying one creation reason * accept test changes * prefer other reasons over Concrete_creation * accept test changes * stop building history tree if flatten is enabled * remove CR version Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> * Add CR for assumption in combine_histories Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> Improve layout history error message formatting and wording (#1832) * remove extra whitespace * fix Unequal_var_layouts error fmt * improve message wording * fix spacing * accept test changes * grammar and phrasing * test changes --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Improve layout history error message: add path field to Type_argument value_creation_reason (#1834) * add Path.t to Type_argument * test changes * add Imported_type_argument * Imported_type_argument for imported type params * fix float64 error format * test changes * add more tests * update layout reason not just in failure case * tests with more than one type param * combine Imported_type_argument with Type_argument * add option_argument_layout to predef * track position and arity for Type_argument * test changes * more tests around typa argument arity * have Imported_type_argument as a creation_reason * test changes * add note about performance benchmark * extract position format logic into function * add comments about 1-indexed * change equals to is * test changes * define list_argument_layout * format comments --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Improve layout history error message: additional info on value layout defaulting (#1855) * add message when defaulting to layout value * test changes * relax to all const defaulting * test changes * improve wording * test changes --------- Co-authored-by: alanechang <alanechang@janestreet.com> Improve layout history error message: change message wording and add more tests (#1887) * change imported reason wording * test changes * lowercase * test changes * add more tests * change wording for match * update Immediate_polymorphic_variant message * test changes Improve layout history error message: hide layout history reason of various generalized vars (#1889) * add generalized creation reason * use change layout reason for value decl * change layout reason for decl type params * test changes * additional tests * add "the" * test changes * improve generalize on type decl * test changes * update err msg test case * add print_loc_in_lowercase * test changes * update_decls_layout_reason code cleanup Improve layout history error message: better test coverage (#1924) * Annotation * concrete layout * value layout * immediate layout * float64 layout * the rest * fix probe test * fix tests * more changes * more value test * add note * remove unused With_constraint constructor * fix format * remove unused void_creation_reason * remove unused Value_kind * add Missing_cmi test * consistent names for Type_variable&Univar * add param to Unannotated_type_parameter * add CRs * fix wording * add warning to all reasons we can't produce * more CRs * fix formatting * add new tests * enhance creation reason when missing cmi * fix Unannotated_type_parameter * test changes * code formatting (only) * Record missing cmi in the layout history * Add a CR * add broken test * fix is_missing_cmi check --------- Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> Support for [@error_message] attribute on layout annotations (#1943) support for error_message attr on layout annots add error_message to the builtin_attrs table change error_message_attr to return first refactor - layout_of_annotation_or_attr to layout_of_annotation - Err_msg_attr to With_error_message fix tests improve formatting Support for [@error_message] attribute on Pexp_constraint (#1954) * implement error_message on Pexp_constraint * add tests * force message to be on new line * fix typo Improve layout history error message: omit history for signature mismatch (#2089) strip away layout history for signature mismatch Layout history: mark `typing-immediate/immediate.ml` as reviewed (#2144) mark test file as reviewed Improve layout history error message: review new messages (#2091) * remove layouts v2.9 CRs * "must be representable" instead of var name * catch missing history generalization * add history to Bad_univar_layout * remove crs * Toplevel_nonvalue refer to types * change Non_value_in_sig to refer to types * fix typo of Polymorphic * type argument to a class constructor * change message of Not_a_value * remove stale test file * refer to types in all messages * update unannotated universal variable message * split into Class_type_argument&Class_term_argument * add comment on update_generalized_ty_layout_reason * undo change to Not_a_value * update Not_a_value message fix merge conflicts Layout histories: resolve rebase conflicts (#2197) * get things to build * test changes * remove unused jkind reason * fix formatting
goldfirere
pushed a commit
that referenced
this pull request
Dec 29, 2023
* Enable layout histories (#1823) * enable display_histories * accept test changes and mark them as unreviewed * improve CR messages for unreviewed tests --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Remove duplicated creation_reasons from layout history error messages (#1825) * stop storing duplicates in creation_reasons * accept test changes * reduce to only displaying one creation reason * accept test changes * prefer other reasons over Concrete_creation * accept test changes * stop building history tree if flatten is enabled * remove CR version Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> * Add CR for assumption in combine_histories Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> Improve layout history error message formatting and wording (#1832) * remove extra whitespace * fix Unequal_var_layouts error fmt * improve message wording * fix spacing * accept test changes * grammar and phrasing * test changes --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Improve layout history error message: add path field to Type_argument value_creation_reason (#1834) * add Path.t to Type_argument * test changes * add Imported_type_argument * Imported_type_argument for imported type params * fix float64 error format * test changes * add more tests * update layout reason not just in failure case * tests with more than one type param * combine Imported_type_argument with Type_argument * add option_argument_layout to predef * track position and arity for Type_argument * test changes * more tests around typa argument arity * have Imported_type_argument as a creation_reason * test changes * add note about performance benchmark * extract position format logic into function * add comments about 1-indexed * change equals to is * test changes * define list_argument_layout * format comments --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Improve layout history error message: additional info on value layout defaulting (#1855) * add message when defaulting to layout value * test changes * relax to all const defaulting * test changes * improve wording * test changes --------- Co-authored-by: alanechang <alanechang@janestreet.com> Improve layout history error message: change message wording and add more tests (#1887) * change imported reason wording * test changes * lowercase * test changes * add more tests * change wording for match * update Immediate_polymorphic_variant message * test changes Improve layout history error message: hide layout history reason of various generalized vars (#1889) * add generalized creation reason * use change layout reason for value decl * change layout reason for decl type params * test changes * additional tests * add "the" * test changes * improve generalize on type decl * test changes * update err msg test case * add print_loc_in_lowercase * test changes * update_decls_layout_reason code cleanup Improve layout history error message: better test coverage (#1924) * Annotation * concrete layout * value layout * immediate layout * float64 layout * the rest * fix probe test * fix tests * more changes * more value test * add note * remove unused With_constraint constructor * fix format * remove unused void_creation_reason * remove unused Value_kind * add Missing_cmi test * consistent names for Type_variable&Univar * add param to Unannotated_type_parameter * add CRs * fix wording * add warning to all reasons we can't produce * more CRs * fix formatting * add new tests * enhance creation reason when missing cmi * fix Unannotated_type_parameter * test changes * code formatting (only) * Record missing cmi in the layout history * Add a CR * add broken test * fix is_missing_cmi check --------- Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> Support for [@error_message] attribute on layout annotations (#1943) support for error_message attr on layout annots add error_message to the builtin_attrs table change error_message_attr to return first refactor - layout_of_annotation_or_attr to layout_of_annotation - Err_msg_attr to With_error_message fix tests improve formatting Support for [@error_message] attribute on Pexp_constraint (#1954) * implement error_message on Pexp_constraint * add tests * force message to be on new line * fix typo Improve layout history error message: omit history for signature mismatch (#2089) strip away layout history for signature mismatch Layout history: mark `typing-immediate/immediate.ml` as reviewed (#2144) mark test file as reviewed Improve layout history error message: review new messages (#2091) * remove layouts v2.9 CRs * "must be representable" instead of var name * catch missing history generalization * add history to Bad_univar_layout * remove crs * Toplevel_nonvalue refer to types * change Non_value_in_sig to refer to types * fix typo of Polymorphic * type argument to a class constructor * change message of Not_a_value * remove stale test file * refer to types in all messages * update unannotated universal variable message * split into Class_type_argument&Class_term_argument * add comment on update_generalized_ty_layout_reason * undo change to Not_a_value * update Not_a_value message fix merge conflicts Layout histories: resolve rebase conflicts (#2197) * get things to build * test changes * remove unused jkind reason * fix formatting * fix rebase conflicts * fix format
lukemaurer
pushed a commit
to lukemaurer/flambda-backend
that referenced
this pull request
Oct 23, 2024
* Enable layout histories (ocaml-flambda#1823) * enable display_histories * accept test changes and mark them as unreviewed * improve CR messages for unreviewed tests --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Remove duplicated creation_reasons from layout history error messages (ocaml-flambda#1825) * stop storing duplicates in creation_reasons * accept test changes * reduce to only displaying one creation reason * accept test changes * prefer other reasons over Concrete_creation * accept test changes * stop building history tree if flatten is enabled * remove CR version Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> * Add CR for assumption in combine_histories Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> Improve layout history error message formatting and wording (ocaml-flambda#1832) * remove extra whitespace * fix Unequal_var_layouts error fmt * improve message wording * fix spacing * accept test changes * grammar and phrasing * test changes --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Improve layout history error message: add path field to Type_argument value_creation_reason (ocaml-flambda#1834) * add Path.t to Type_argument * test changes * add Imported_type_argument * Imported_type_argument for imported type params * fix float64 error format * test changes * add more tests * update layout reason not just in failure case * tests with more than one type param * combine Imported_type_argument with Type_argument * add option_argument_layout to predef * track position and arity for Type_argument * test changes * more tests around typa argument arity * have Imported_type_argument as a creation_reason * test changes * add note about performance benchmark * extract position format logic into function * add comments about 1-indexed * change equals to is * test changes * define list_argument_layout * format comments --------- Co-authored-by: Alan Chang <alanechang@janestreet.com> Improve layout history error message: additional info on value layout defaulting (ocaml-flambda#1855) * add message when defaulting to layout value * test changes * relax to all const defaulting * test changes * improve wording * test changes --------- Co-authored-by: alanechang <alanechang@janestreet.com> Improve layout history error message: change message wording and add more tests (ocaml-flambda#1887) * change imported reason wording * test changes * lowercase * test changes * add more tests * change wording for match * update Immediate_polymorphic_variant message * test changes Improve layout history error message: hide layout history reason of various generalized vars (ocaml-flambda#1889) * add generalized creation reason * use change layout reason for value decl * change layout reason for decl type params * test changes * additional tests * add "the" * test changes * improve generalize on type decl * test changes * update err msg test case * add print_loc_in_lowercase * test changes * update_decls_layout_reason code cleanup Improve layout history error message: better test coverage (ocaml-flambda#1924) * Annotation * concrete layout * value layout * immediate layout * float64 layout * the rest * fix probe test * fix tests * more changes * more value test * add note * remove unused With_constraint constructor * fix format * remove unused void_creation_reason * remove unused Value_kind * add Missing_cmi test * consistent names for Type_variable&Univar * add param to Unannotated_type_parameter * add CRs * fix wording * add warning to all reasons we can't produce * more CRs * fix formatting * add new tests * enhance creation reason when missing cmi * fix Unannotated_type_parameter * test changes * code formatting (only) * Record missing cmi in the layout history * Add a CR * add broken test * fix is_missing_cmi check --------- Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com> Support for [@error_message] attribute on layout annotations (ocaml-flambda#1943) support for error_message attr on layout annots add error_message to the builtin_attrs table change error_message_attr to return first refactor - layout_of_annotation_or_attr to layout_of_annotation - Err_msg_attr to With_error_message fix tests improve formatting Support for [@error_message] attribute on Pexp_constraint (ocaml-flambda#1954) * implement error_message on Pexp_constraint * add tests * force message to be on new line * fix typo Improve layout history error message: omit history for signature mismatch (ocaml-flambda#2089) strip away layout history for signature mismatch Layout history: mark `typing-immediate/immediate.ml` as reviewed (ocaml-flambda#2144) mark test file as reviewed Improve layout history error message: review new messages (ocaml-flambda#2091) * remove layouts v2.9 CRs * "must be representable" instead of var name * catch missing history generalization * add history to Bad_univar_layout * remove crs * Toplevel_nonvalue refer to types * change Non_value_in_sig to refer to types * fix typo of Polymorphic * type argument to a class constructor * change message of Not_a_value * remove stale test file * refer to types in all messages * update unannotated universal variable message * split into Class_type_argument&Class_term_argument * add comment on update_generalized_ty_layout_reason * undo change to Not_a_value * update Not_a_value message fix merge conflicts Layout histories: resolve rebase conflicts (ocaml-flambda#2197) * get things to build * test changes * remove unused jkind reason * fix formatting * fix rebase conflicts * fix format
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR removes all the
error messages below here are unreviewedCRs. Only merge it after the new error messages are in fact reviewed.