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

Proposal: Globals (pre-cursor to Compilation Units) #30

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Proposal: Globals (pre-cursor to Compilation Units) #30

wants to merge 8 commits into from

Conversation

gmalecha
Copy link
Contributor

This generalizes functions into globals. It still doesn't support function pointers though doing so is relatively simple. In summary,

funname -> globname

expr := ..
  | global (_ : globname)

Because it was simple, I also changed call from taking a funname to taking an expr (so f becomes global f). Generalizing the semantics completely isn't too difficult. I can add that to this PR if it would be useful. #27 is preventing a complete operational semantics, but if we solve that, it shouldn't be difficult to do this.

@gmalecha
Copy link
Contributor Author

gmalecha commented Sep 10, 2018

Just a note, on top of this definition we would have something like:

Record module :=
{ imports : list globname
; definitions : map globname (function + data)
; exports : list globname
}.

@@ -46,7 +49,7 @@ Section WeakestPrecondition.
End WithF.

Section WithFunctions.
Context (call : funname -> trace -> mem -> list word -> (trace -> mem -> list word -> Prop) -> Prop).
Context (call : word -> trace -> mem -> list word -> (trace -> mem -> list word -> Prop) -> Prop).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

funname should remain abstract in this file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is abstract, in this version of the semantics you don't call a funname, you call the word that it resolves to.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Further clarification. The resolve function maps globname to option word and global g denotes to resolve g (if it is defined). So previously, if you wanted to say that ""swap" has this specification", you would now say "resolve "swap" has this specification".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wouldn't be difficult to remove this functionality (just don't allow calling expressions), if that is truly desireable. This seems like an easy way to support function pointers within the "be concrete" mantra of bedrock2.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to be able to use the file WeakestPrecondition.v with funname being instantitated to string, while others might want to instantiate it to word or to Z or whatever. Can you adapt it so that this becomes possible again?

@samuelgruetter
Copy link
Contributor

I'd like to see what compiler.ExprImp.eval_cmd looks like with this (I think this could be done before or after moving the semantics in ExprImp.v from the compiler folder to the bedrock2 folder, whatever seems more convenient).

I still believe that funname and globname should be two different abstract types (but of course you can have files inside which the two are defined to be the same, if all users of such files are ok with it). But since I'm not working with functions myself at the moment, I'm fine with using the same name for funname and globname for the moment, and once I start working with functions myself, I'll see whether/why I want to separate them, so be prepared that I'll want to change it, but for the moment I'm ok to stick to the YAGNI principle 😉

"Because it was simple" is not a good reason for replacing funname by expr in call. Could you make the change in such a way that there's a simple way to restrict the input language back to what we have now for users who wish to do so?

Require Import Coq.ZArith.BinIntDef.
Require Import ExtLib.Data.HList.
Require Import ExtLib.Data.Fin.
Require Import ExtLib.Data.Map.FMapAList.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like these ExtLib dependencies are not needed (but make the build fail)

@gmalecha
Copy link
Contributor Author

@samuelgruetter I've ported compiler.ExprImp.eval_cmd. As the diff shows, it is mostly just alpha renaming and adding some cases for the global constructor.

If you can think of any reason within the compiler to separate the two, I'd be really interested in knowing what it is.

I've reverted the ability to call expressions per your request. I guess we can also drop the call_rec function that I implemented.

@samuelgruetter
Copy link
Contributor

In the latest build, ExprImp.v is never compiled, because the build fails before that. Before I spend more time on trying to understand what you're doing, would you mind making sure that ExprImp.v is compiled by travis?

- the automation needs some work right now.
@gmalecha
Copy link
Contributor Author

@samuelgruetter The only thing that didn't compile in the bedrock2 directory is the swap example. I've updated that with the latest push. Now what breaks is in the compiler directory.


Context {stateMap: MapFunctions var (mword)}.
Notation state := (map var mword).
Context {varset: SetFunctions var}.
Notation vars := (set var).

Variable resolve : glob -> option word.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: I'd prefer mword over word here, even though both refer to the same in this context, it's just that this file consistently uses mword. Or if you prefer, you could make sure nothing of bbv is imported to avoid confusion with bbv's word, and then replace all occurrences of mword by word.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine. Does nitpicking mean that you're considering merging this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you think that you want to keep using and contributing to (a part of) the code in this repo, and you tell me that merging this would enable you to do so, and you're willing to fix what you broke in the compiler subdirectory so that the build becomes green, and you're willing to deal with further potential nitpicks from my side, then I'd be happy to merge it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants