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

Some options should be listed in restartOptions #6265

Closed
nad opened this issue Oct 30, 2022 · 2 comments · Fixed by #6268
Closed

Some options should be listed in restartOptions #6265

nad opened this issue Oct 30, 2022 · 2 comments · Fixed by #6268
Assignees
Labels
release blocker Issues blocking the next Agda release type: bug Issues and pull requests about actual bugs ux: options Issues relating to Agda's command line options
Milestone

Comments

@nad
Copy link
Contributor

nad commented Oct 30, 2022

The following options are not listed in restartOptions:

  • optCopatterns
  • optFirstOrder
  • optFlatSplit
  • optForcing
  • optKeepCoveringClauses
  • optPatternMatching
  • optProjectionLike

The following options related to backends are also not included, but perhaps that is fine, if the idea is that the backends should take care of recompilation logic:

  • optCompileNoMain
  • optCountClusters

Perhaps the following options related to performance could be removed from the list:

  • optCallByName
  • optFastReduce
  • optSaveMetas

It is apparently easy to forget to add a new option to restartOptions. Perhaps we could instead use a function that blanks out options that should not trigger a "restart".

@nad nad added type: bug Issues and pull requests about actual bugs ux: options Issues relating to Agda's command line options release blocker Issues blocking the next Agda release labels Oct 30, 2022
@nad nad added this to the 2.6.3 milestone Oct 30, 2022
@nad nad self-assigned this Oct 30, 2022
nad added a commit that referenced this issue Oct 30, 2022
The difference between the new functionality and the old one is that
the old functionality does not trigger a recheck when one of the
following options changes, but the new one does:

* optCopatterns
* optFirstOrder
* optFlatSplit
* optForcing
* optKeepCoveringClauses
* optPatternMatching
* optProjectionLike

Furthermore the new functionality is safer in the sense that, if one
forgets to update the functionality when a new option is added, then
one risks getting too many rechecks rather than too few.
@nad
Copy link
Contributor Author

nad commented Oct 30, 2022

The following options are not listed in restartOptions:

If the changes in my pull request are included changes to these options should trigger a recheck.

The following options related to backends are also not included, but perhaps that is fine, if the idea is that the backends should take care of recompilation logic:

Perhaps the following options related to performance could be removed from the list:

I did not change the treatment of these options (except that I removed an occurrence of reportSLn).

I also noted that a change to optUnicode triggers a recheck. I did not change this behaviour.

It is apparently easy to forget to add a new option to restartOptions. Perhaps we could instead use a function that blanks out options that should not trigger a "restart".

I made the functionality safer in the sense that, if one forgets to update the functionality when a new option is added, then one risks getting too many rechecks rather than too few.

nad added a commit that referenced this issue Oct 30, 2022
The difference between the new functionality and the old one is that
the old functionality does not trigger a recheck when one of the
following options changes, but the new one does:

* optCopatterns
* optFirstOrder
* optFlatSplit
* optForcing
* optKeepCoveringClauses
* optPatternMatching
* optProjectionLike

Furthermore the new functionality is safer in the sense that, if one
forgets to update the functionality when a new option is added, then
one risks getting too many rechecks rather than too few.
@andreasabel
Copy link
Member

I made the functionality safer in the sense that, if one forgets to update the functionality when a new option is added, then one risks getting too many rechecks rather than too few.

Yes, this seems the safer way round.

@nad nad linked a pull request Oct 31, 2022 that will close this issue
nad added a commit that referenced this issue Nov 1, 2022
The difference between the new functionality and the old one is that
the old functionality does not trigger a recheck when one of the
following options changes, but the new one does:

* optCopatterns
* optFirstOrder
* optFlatSplit
* optForcing
* optKeepCoveringClauses
* optPatternMatching
* optProjectionLike

Furthermore the new functionality is safer in the sense that, if one
forgets to update the functionality when a new option is added, then
one risks getting too many rechecks rather than too few.
@nad nad closed this as completed in #6268 Nov 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release blocker Issues blocking the next Agda release type: bug Issues and pull requests about actual bugs ux: options Issues relating to Agda's command line options
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants