-
Notifications
You must be signed in to change notification settings - Fork 0
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
refactor: Rename UserName to QualName #56
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of comments, but some are definitely for another PR.
I'm not unhappy with QualName, but other possibilities - QualId(entifier), UserId, UserVarName (too long!)
brat/Brat/Checker/Helpers.hs
Outdated
@@ -131,7 +131,7 @@ pullPorts toPort showFn (p:ports) types = do | |||
else pure (x, xs) | |||
| otherwise = second (x:) <$> pull1Port p xs | |||
|
|||
combineDisjointEnvs :: M.Map UserName v -> M.Map UserName v -> Checking (M.Map UserName v) | |||
combineDisjointEnvs :: M.Map QualName v -> M.Map QualName v -> Checking (M.Map QualName v) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
driveby: we should really use throwLeft
for this
@@ -139,29 +139,29 @@ throwLeft :: Either ErrorMsg a -> Checking a | |||
throwLeft (Right x) = pure x | |||
throwLeft (Left msg) = err msg | |||
|
|||
vlup :: UserName -> Checking [(Src, BinderType Brat)] | |||
vlup :: QualName -> Checking [(Src, BinderType Brat)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oooh, that's odd. We can have qualified names of variables?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think when we have functions imported from other modules they should be qualified, like kernel.CX
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right yes thank you. Ok - so vlup
is really the misnamed one - it does not necessarily look up variables; it just looks up names. Really should be nlup
or qnlup
or just lup
(or blup
to distinguish from klup
? Hey, that one's actually pronouncable in one syllable... 😁 😀 )
@@ -172,7 +172,7 @@ tlup (m, c) = req (TLup (m, c)) >>= \case | |||
Brat -> Kernel | |||
Kernel -> Brat | |||
|
|||
lookupAndUse :: UserName -> KEnv | |||
lookupAndUse :: QualName -> KEnv |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly, if it's something that might be used then will it really be a QualName?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
variables are parsed as qualified names, so they can include prefixes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even kernel names? Can declare a variable of type qubit whose name has a prefix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see, we could be more strict about kernel variables, you're right
brat/Brat/Parser.hs
Outdated
@@ -84,12 +84,12 @@ simpleName = token0 $ \case | |||
Ident str -> Just str | |||
_ -> Nothing | |||
|
|||
qualifiedName :: Parser UserName | |||
qualifiedName :: Parser QualName |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better would be to inline this into userName
(as that's the only use), and then rename that to qualifiedName
or qualName
show (PrefixName ps file) = intercalate "." (ps ++ [file]) | ||
|
||
plain :: String -> UserName | ||
plain :: String -> QualName |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I vote for renaming this. Maybe unqual
or something with "name" in it - singleName
, unqualName
??
No description provided.