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

Options used for different modules must be consistent with each other, and options used when loading an interface must be consistent with those used when the interface was created #2487

Closed
identicalsnowflake opened this issue Mar 4, 2017 · 32 comments

Comments

@identicalsnowflake
Copy link
Contributor

@identicalsnowflake identicalsnowflake commented Mar 4, 2017

Currently, even in a module marked {-# OPTIONS --safe #-}, you may still import (and use!) primTrustMe and postulates from other modules.

It wouldn't bother me if you could import modules not marked as --safe provided they still follow the safe rules, but it seems currently imported modules are totally exempt from these rules.

@gallais gallais added this to the 2.5.3 milestone Mar 4, 2017
@gallais
Copy link
Member

@gallais gallais commented Mar 4, 2017

I guess it's due to the fact it's the options in the persistent state that get used to typecheck submodules. I don't think we want to modify the persistent state but we could grab the status of the --safe flag right before the call to bracket_ and push it back in after the reset.

Doesn't seem to work.

@gallais gallais self-assigned this Mar 4, 2017
@gallais
Copy link
Member

@gallais gallais commented Mar 5, 2017

Minimal test case:

{-# OPTIONS --safe #-}
module Issue2487 where
open import Issue2487-2
module Issue2487-2 where
postulate A : Set

@gallais gallais removed their assignment Mar 5, 2017
@nad
Copy link
Contributor

@nad nad commented Mar 6, 2017

This is an instance of a more general problem: Agda does not check that options for different modules in the same project are consistent with each other. For instance, one can use --without-K in one module, and --with-K in another. I suggest that interfaces should contain information about "important" features (various options, use of unsafe features like postulates, and possibly more; perhaps we could even check whether the K rule is used, even when --without-K is not active). When modules are imported one can then check that the imported modules and the importing module use these features in a consistent way:

  • Modules using --safe may only import safe modules.
  • Modules using --without-K may only import modules that do not use the K rule.
  • Other features should also be considered: --cubical, perhaps sized types, perhaps something else.

@nad
Copy link
Contributor

@nad nad commented Mar 27, 2017

As noted by @identicalsnowflake (#2515 (comment)) there is an additional problem: when a .agdai file is loaded we don't check that the current options are consistent with those used when the file was created.

@nad nad changed the title Make --safe require safety of imported modules Options used for different modules must be consistent with each other, and options used when loading an interface must be consistent with those used when the interface was created Mar 28, 2017
@identicalsnowflake
Copy link
Contributor Author

@identicalsnowflake identicalsnowflake commented Mar 31, 2017

There may be some logic for this already, actually, it just seems to be backwards. For example, you may have a {-# OPTIONS --safe #-} module depend on a {-# OPTIONS --type-in-type #-} and Agda will not complain; however, if you try to make a --type-in-type module depend on a --safe module, Agda will forbid it.

@nad nad added the false label May 9, 2017
@nad
Copy link
Contributor

@nad nad commented May 9, 2017

I've added the "false" label, because I assume that one can currently use --cubical in one module, --with-K in another, and produce an inconsistency by combining the two.

@nad
Copy link
Contributor

@nad nad commented Oct 1, 2017

I guess it makes sense to fix #2783 and #2784 at the same time as (or before) this issue.

@MatthewDaggitt
Copy link
Contributor

@MatthewDaggitt MatthewDaggitt commented Apr 12, 2018

Given that this doesn't appear to be an immediate priority, an extra Known Issue should definitely be added at the bottom of the Safe Agda documentation, saying you currently need to delete all .agdai files before running agda --safe. As #3029 shows, this is still tripping myself and other people up.

@nad
Copy link
Contributor

@nad nad commented Jan 10, 2019

--guardedness and --sized-types do not have to be inconsistent with each other, as opposed to e.g. univalence and --with-K. We want to eventually fix the bugs that make them so.

@fredrikNordvallForsberg stated that his current implementation activates both when --safe is not used, but deactivates both when --safe is used. Does this sound reasonable to you?

Apparently the implementation also allows --safe --sized-types.

@Saizan
Copy link
Contributor

@Saizan Saizan commented Jan 11, 2019

@nad that sounds reasonable, yes.

fredrikNordvallForsberg added a commit that referenced this issue Jan 16, 2019
Reverts part of commit e134cce as we can now postulate String,
Char, Float, Word64 and the coinduction builtins again.
fredrikNordvallForsberg added a commit that referenced this issue Jan 17, 2019
Reverts part of commit e134cce as we can now postulate String,
Char, Float, Word64 and the coinduction builtins again.
fredrikNordvallForsberg added a commit that referenced this issue Jan 19, 2019
fredrikNordvallForsberg added a commit that referenced this issue Jan 21, 2019
fredrikNordvallForsberg added a commit that referenced this issue Jan 21, 2019
fredrikNordvallForsberg added a commit that referenced this issue Jan 22, 2019
fredrikNordvallForsberg added a commit that referenced this issue Jan 22, 2019
@nad
Copy link
Contributor

@nad nad commented Jan 22, 2019

Thanks!

gallais added a commit to agda/agda-stdlib that referenced this issue Feb 3, 2019
Most changes are:
* adding the sized-types/guardedness OPTIONS to modules using these
  notions (or importing modules which do)
* making sure these sized-types/guardedness OPTIONS come after the
  --safe one.

Also taking care of #603, uncovered thanks to consistency checking
and being forced to remove Data.Container.Indexed's dependency to
Relation.Binary.HeterogeneousEquality.
jespercockx pushed a commit to jespercockx/agda that referenced this issue Feb 26, 2019
jespercockx pushed a commit to jespercockx/agda that referenced this issue Feb 26, 2019
MatthewDaggitt pushed a commit to agda/agda-stdlib that referenced this issue Mar 25, 2019
Most changes are:
* adding the sized-types/guardedness OPTIONS to modules using these
  notions (or importing modules which do)
* making sure these sized-types/guardedness OPTIONS come after the
  --safe one.

Also taking care of #603, uncovered thanks to consistency checking
and being forced to remove Data.Container.Indexed's dependency to
Relation.Binary.HeterogeneousEquality.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants