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

Check fixed choices in name collision checker #11481

Merged
merged 2 commits into from
Nov 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 6 additions & 15 deletions compiler/daml-lf-tools/src/DA/Daml/LF/TypeChecker/Check.hs
Original file line number Diff line number Diff line change
Expand Up @@ -885,27 +885,18 @@ checkTemplate m t@(Template _loc tpl param precond signatories observers text ch
whenJust mbKey $ checkTemplateKey param tcon
forM_ implements $ checkIfaceImplementation tcon

-- Check template choice and interface fixed choice name collisions.
foldM_ checkFixedChoiceCollision (S.fromList (NM.names choices)) implements
-- ^ We don't use NM.namesSet here because Data.HashSet is assymptotically
-- slower than Data.Set when it comes to unions and checking for disjointness.

where
withPart p = withContext (ContextTemplate m t p)

checkFixedChoiceCollision :: S.Set ChoiceName -> TemplateImplements -> m (S.Set ChoiceName)
checkFixedChoiceCollision !accum ifaceImpl = do
iface <- inWorld $ lookupInterface (tpiInterface ifaceImpl)
let newNames = S.fromList (NM.names (intFixedChoices iface))
unless (S.disjoint accum newNames) $ do
let choiceName = head (S.toList (S.intersection accum newNames))
throwWithContext (EDuplicateTemplateChoiceViaInterfaces tpl choiceName)
pure (S.union accum newNames)

checkIfaceImplementation :: MonadGamma m => Qualified TypeConName -> TemplateImplements -> m ()
checkIfaceImplementation tplTcon TemplateImplements{..} = do
let tplName = qualObject tplTcon
DefInterface {intVirtualChoices, intMethods} <- inWorld $ lookupInterface tpiInterface
DefInterface {intFixedChoices, intVirtualChoices, intMethods} <- inWorld $ lookupInterface tpiInterface

-- check fixed choices
let inheritedChoices = S.fromList (NM.names intFixedChoices)
unless (inheritedChoices == tpiInheritedChoiceNames) $
Copy link
Contributor

Choose a reason for hiding this comment

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

is it possible to trigger this in Daml? If so, a test might be nice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it's possible. ATM this only serves as a sanity check against the LF completer.

throwWithContext $ EBadInheritedChoices tpiInterface (S.toList inheritedChoices) (S.toList tpiInheritedChoiceNames)

-- check virtual choices
forM_ intVirtualChoices $ \InterfaceChoice {ifcName, ifcConsuming, ifcArgType, ifcRetType} -> do
Expand Down
7 changes: 7 additions & 0 deletions compiler/daml-lf-tools/src/DA/Daml/LF/TypeChecker/Error.hs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ data Error
| EDuplicateInterfaceChoiceName !TypeConName !ChoiceName
| EDuplicateInterfaceMethodName !TypeConName !MethodName
| EUnknownInterface !TypeConName
| EBadInheritedChoices { ebicInterface :: !(Qualified TypeConName), ebicExpected :: ![ChoiceName], ebicGot :: ![ChoiceName] }
| EMissingInterfaceChoice !ChoiceName
| EBadInterfaceChoiceImplConsuming !ChoiceName !Bool !Bool
| EBadInterfaceChoiceImplArgType !ChoiceName !Type !Type
Expand Down Expand Up @@ -389,6 +390,12 @@ instance Pretty Error where
EDuplicateInterfaceMethodName iface method ->
"Duplicate method name '" <> pretty method <> "' in interface definition for " <> pretty iface
EUnknownInterface tcon -> "Unknown interface: " <> pretty tcon
EBadInheritedChoices {ebicInterface, ebicExpected, ebicGot} ->
vcat
[ "List of inherited choices does not match interface definition for " <> pretty ebicInterface
, "Expected: " <> pretty ebicExpected
, "But got: " <> pretty ebicGot
]
EMissingInterfaceChoice ch -> "Missing interface choice implementation for " <> pretty ch
EBadInterfaceChoiceImplConsuming ch ifaceConsuming tplConsuming ->
vcat
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ data Name
| NEnumCon ModuleName TypeConName VariantConName
| NField ModuleName TypeConName FieldName
| NChoice ModuleName TypeConName ChoiceName
| NChoiceViaInterface ModuleName TypeConName ChoiceName (Qualified TypeConName)
| NInterface ModuleName TypeConName

-- | Helper method so we can turn collisions with virtual modules into warnings
Expand Down Expand Up @@ -69,6 +70,8 @@ displayName = \case
T.concat ["field ", dot m, ":", dot t, ".", f]
NChoice (ModuleName m) (TypeConName t) (ChoiceName c) ->
T.concat ["choice ", dot m, ":", dot t, ".", c]
NChoiceViaInterface (ModuleName m) (TypeConName t) (ChoiceName c) (Qualified _ (ModuleName imod) (TypeConName ityp)) ->
T.concat ["choice ", dot m, ":", dot t, ".", c, " (via interface ", dot imod, ":", dot ityp, ")"]
NInterface (ModuleName m) (TypeConName t) ->
T.concat ["interface ", dot m, ":", dot t]
where
Expand Down Expand Up @@ -125,6 +128,8 @@ fullyResolve = FRName . map T.toLower . \case
m ++ t ++ [f]
NChoice (ModuleName m) (TypeConName t) (ChoiceName c) ->
m ++ t ++ [c]
NChoiceViaInterface (ModuleName m) (TypeConName t) (ChoiceName c) _ ->
m ++ t ++ [c]
NInterface (ModuleName m) (TypeConName t) ->
m ++ t

Expand Down Expand Up @@ -199,6 +204,9 @@ checkTemplate :: ModuleName -> Template -> NCMonad ()
checkTemplate moduleName Template{..} = do
forM_ tplChoices $ \TemplateChoice{..} ->
checkName (NChoice moduleName tplTypeCon chcName)
forM_ tplImplements $ \TemplateImplements{..} ->
forM_ tpiInheritedChoiceNames $ \choiceName ->
checkName (NChoiceViaInterface moduleName tplTypeCon choiceName tpiInterface)

checkSynonym :: ModuleName -> DefTypeSyn -> NCMonad ()
checkSynonym moduleName DefTypeSyn{..} =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
-- SPDX-License-Identifier: Apache-2.0

-- @SINCE-LF-FEATURE DAML_INTERFACE
-- @ERROR Duplicate choice name 'MyArchive' in template T via interfaces.
-- @ERROR name collision between choice InterfaceChoiceCollision:T.MyArchive (via interface InterfaceChoiceCollision:InterfaceA) and choice InterfaceChoiceCollision:T.MyArchive (via interface InterfaceChoiceCollision:InterfaceB)
module InterfaceChoiceCollision where

interface InterfaceA where
Expand Down