Angle compiler cleanups

- rename `TcQueryGen` to `TcWhere`

- change `TcAll` and `TcNegation` to contain `TcPat` instead of `TcQuery` /
  `[TcStatement]` respectively (this is more uniform and
  consistent, and simplifies some things)

- refactoring in the type checker to unify how we handle
  locality. `oneBranch` is renamed `encloseLocal` and used consistently
  when typechecking `|`, `all`, `!`, and `if`, which all need
  locality. Probably fixes some bugs.

- update a few out-of-date comments

Author: simonmar

glean/db/Glean/Query/BindOrder.hs

@@ -33,22 +33,12 @@ import Glean.Query.Vars (VarSet)
 import Glean.RTS.Term as RTS
 
 -- -----------------------------------------------------------------------------
--- Fixing up MatchBind vs. MatchVar
-
--- Substitution can mess up MatchBind and MatchVar - we might end up
--- with multiple MatchBinds for a variable, or a MatchVar before a
--- MatchBind. To fix it up all we need to do is traverse the query in
--- the correct order, keeping track of which variables are in scope,
--- and change MatchBind<->MatchVar as appropriate.
---
--- This is all somewhat suboptimal, because the typechecker has
--- already figured out MatchBind vs. MatchVar and here we mess it up
--- by substitution and then fix it again. Which begs the question: why
--- do we have MatchBind and MatchVar at all, couldn't we leave it
--- until the last minute just before code generation to figure out
--- which variables are binding occurrences?  Yes, but it's nice to be
--- able to give the user out-of-scope error messages from the
--- typechecker.  Maybe we'll change this in the future.
+-- Resolving MatchBind vs. MatchVar
+
+-- Before the Reorder pass, MatchBind and MatchVar are the same (strictly
+-- speaking we should have just one variable form). The reorder pass resolves
+-- statement ordering so that variables are bound before their use;
+-- MatchBind indicates a variable binding and MatchVar indicates a variable use.
 
 data Scope = Scope
   { isScope :: VarSet

glean/db/Glean/Query/Codegen/Types.hs

@@ -81,46 +81,15 @@ data CgStatement_ var
   | CgAllStatement var (Expr_ var) [CgStatement_ var]
   | CgNegation [CgStatement_ var]
   | CgDisjunction [[CgStatement_ var]]
-  -- ^ For rationale, see Note [why do we have sequential composition?]
   | CgConditional
     { cond :: [CgStatement_ var]
     , then_ :: [CgStatement_ var]
     , else_ :: [CgStatement_ var]
     }
   deriving (Show, Functor, Foldable, Traversable)
 
-
 type CgStatement = CgStatement_ Var
 
-{- Note [why do we have sequential composition?]
-
-The issue is that queries for sum types can't necessarily be handled
-by nested generators. Consider
-
-  v = cxx1.FunctionName (name(cxx1.Name "xyz" ) | operator(cxx1.Name "+"))
-
-If we flattened this into nested generators we would get
-
-  x = cxx1.Name "xyz"
-  y = cxx1.Name "+"
-  z = cxx1.FunctionName (name x | operator y)
-
-Now suppose there is no name xyz. This query will match nothing,
-because the generator for cxx1.Name "xyz" would be empty.  (even if
-the generator matched, flattening out the generators like this will
-test too many combinations and do too much work).
-
-With sequential composition of queries we can do it like this:
-
- n = (name x where x = cxx1.Name "xyz") |
-     (operator x where x = cxx1.Name "+")
- v = cxx1.FunctionName n
-
-(Note that this query won't work if you write it because we can't
-typecheck the sub-query "name x where ...", but we can generate the
-AST for it in the JSON query compiler.)
--}
-
 type Generator = Generator_ Var
 
 -- | A generator produces a finite series of terms

glean/db/Glean/Query/Expand.hs

@@ -102,12 +102,12 @@ instantiateWithFreshVariables query numVars = do
     TcElementsOfSet (instantiatePat base pat)
   instantiateTcTerm base (TcElementsUnresolved ty pat) =
     TcElementsUnresolved ty (instantiatePat base pat)
-  instantiateTcTerm base (TcQueryGen query) =
-    TcQueryGen (instantiateQuery base query)
-  instantiateTcTerm base (TcAll query) =
-    TcAll (instantiateQuery base query)
-  instantiateTcTerm base (TcNegation stmts) =
-    TcNegation (map (instantiateStmt base) stmts)
+  instantiateTcTerm base (TcWhere query) =
+    TcWhere (instantiateQuery base query)
+  instantiateTcTerm base (TcAll pat) =
+    TcAll (instantiatePat base pat)
+  instantiateTcTerm base (TcNegation p) =
+    TcNegation (instantiatePat base p)
   instantiateTcTerm base (TcPrimCall op args) =
     TcPrimCall op (map (instantiatePat base) args)
   instantiateTcTerm base (TcIf (Typed ty cond) then_ else_) =

glean/db/Glean/Query/Flatten.hs

@@ -210,20 +210,25 @@ flattenSeqGenerators (Ref (MatchExt (Typed ty match))) = case match of
   TcElementsOfSet pat -> do
     r <- flattenPattern pat
     return [(mempty, stmts, SetElementGenerator ty pat') | (stmts,pat') <- r ]
-  TcQueryGen query -> do
+  TcWhere query -> do
     (group, term, _) <- flattenQuery' query
     return [(floatGroup group, mempty, TermGenerator term)]
-  TcAll query -> do
-    (group, term, _) <- flattenQuery' query
+  TcAll p -> do
+    let elemTy = case derefType ty of
+          Schema.SetTy t -> t
+          _other -> error "TcAll: not SetTy"
+    (group, term, _) <- flattenQuery' $
+      TcQuery elemTy p Nothing [] Angle.Unordered
     var <- fresh ty
     return
       [ (Statements [FlatAllStatement var term group]
       , mempty
       , TermGenerator (Ref (MatchVar var)))]
-  TcNegation stmts -> do
-    (ords, floats) <- mapAndUnzipM flattenStatement stmts
-    let neg = FlatNegation (mkGroup ords floats)
-    return [(oneStmt neg, mempty, TermGenerator $ Tuple [])]
+  TcNegation p -> do
+    (group, _term, _) <- flattenQuery' (TcQuery ty p Nothing [] Angle.Ordered)
+    -- Note: term is discarded. e.g. in !(X where ...) we don't do anything
+    -- with the X.
+    return [(oneStmt (FlatNegation group), mempty, TermGenerator $ Tuple [])]
   TcPrimCall op args -> do
     r <- manyTerms (\args -> PrimCall op args ty) <$> mapM flattenPattern args
     return [ (mempty, float, gen) | (float, gen) <- r ]

glean/db/Glean/Query/Prune.hs

@@ -155,12 +155,12 @@ prune hasFacts (QueryWithInfo q _ gen t) = do
           Ref . MatchExt . Typed ty . TcElementsOfSet <$> prunePat x
         TcElementsUnresolved ty' x ->
           Ref . MatchExt . Typed ty . TcElementsUnresolved ty' <$> prunePat x
-        TcQueryGen q ->
-          Ref . MatchExt . Typed ty . TcQueryGen <$> pruneTcQuery q
+        TcWhere q ->
+          Ref . MatchExt . Typed ty . TcWhere <$> pruneTcQuery q
         -- we dont' want to handle negation here because if it tries to match
         -- against things that are not in the database it should succeed.
-        TcAll query ->
-          Ref . MatchExt . Typed ty . TcAll <$> pruneTcQuery query
+        TcAll p ->
+          Ref . MatchExt . Typed ty . TcAll <$> prunePat p
         TcNegation{} -> Just pat
         TcPrimCall op xs -> Ref . MatchExt . Typed ty . TcPrimCall op
           <$> traverse prunePat xs
@@ -240,9 +240,9 @@ renumberVars gen ty q =
     TcElementsOfArray x -> TcElementsOfArray <$> renamePat x
     TcElementsOfSet x -> TcElementsOfSet <$> renamePat x
     TcElementsUnresolved ty x -> TcElementsUnresolved ty <$> renamePat x
-    TcQueryGen q -> TcQueryGen <$> renameQuery q
-    TcAll query -> TcAll <$> renameQuery query
-    TcNegation xs -> TcNegation <$> traverse renameStmt xs
+    TcWhere q -> TcWhere <$> renameQuery q
+    TcAll p -> TcAll <$> renamePat p
+    TcNegation p -> TcNegation <$> renamePat p
     TcPrimCall op xs -> TcPrimCall op <$> traverse renamePat xs
     TcIf cond then_ else_ ->
       TcIf <$> traverse renamePat cond <*> renamePat then_ <*> renamePat else_