From 1bee52e40c889d4cbf4cc95b664d0b354cb6c82c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mois=C3=A9s=20Ackerman?=
 <6054733+akrmn@users.noreply.github.com>
Date: Fri, 8 Sep 2023 12:30:21 +0200
Subject: [PATCH] Remove compiler support for `controller..can` choice syntax
 (#17362)

* Update GHC_REV

* Remove 'controller..can' syntax in java codegen StakeholdersTest

* Remove 'controller..can' syntax in ChoiceSyntaxes daml-test-file

* Remove daml-test-files {,No}WarnControllerCan

* Remove 'controller..can' syntax in SemanticsEvalOrder{,RightToLeft} daml-test-files

* Update {Structure,Reference}.daml code snippets for docs.daml.com

* Remove 'controller..can' syntax in TestGrammar.daml

* changelog entry for 'controller..can' removal

* Update UNRELEASED.md

Co-authored-by: carrielaben-da <91496516+carrielaben-da@users.noreply.github.com>
---
 UNRELEASED.md                                 | 13 ++++
 bazel_tools/ghc-lib/version.bzl               |  2 +-
 .../daml-extension/syntaxes/TestGrammar.daml  | 11 ----
 .../tests/daml-test-files/ChoiceSyntaxes.daml | 14 -----
 .../daml-test-files/NoWarnControllerCan.daml  | 17 ------
 .../daml-test-files/SemanticsEvalOrder.daml   | 26 +-------
 .../SemanticsEvalOrderRightToLeft.daml        | 26 +-------
 .../daml-test-files/WarnControllerCan.daml    | 17 ------
 docs/source/daml/code-snippets/Reference.daml | 60 ++++++++++++++++++-
 docs/source/daml/code-snippets/Structure.daml | 34 ++++++++++-
 .../src/ledger-tests/daml/Stakeholders.daml   | 41 -------------
 .../com/digitalasset/StakeholdersTest.scala   | 35 ++---------
 12 files changed, 110 insertions(+), 186 deletions(-)
 delete mode 100644 compiler/damlc/tests/daml-test-files/NoWarnControllerCan.daml
 delete mode 100644 compiler/damlc/tests/daml-test-files/WarnControllerCan.daml

diff --git a/UNRELEASED.md b/UNRELEASED.md
index d6cea26f5d88..c2b5cb446f2f 100644
--- a/UNRELEASED.md
+++ b/UNRELEASED.md
@@ -60,6 +60,19 @@ following warning will be shown during compilation or in the IDE:
 
 For more information, see [Reference: Templates: Template-local Definitions (Deprecated)](https://docs.daml.com/2.8.0/daml/reference/templates.html#template-local-definitions-deprecated)
 
+## Removal of deprecated 'controller..can' syntax
+
+The 'controller..can' syntax for defining template choices, deprecated since
+Daml 2.0, has now been completely removed. Projects that use this syntax are no
+longer accepted. Those choices should instead be defined using 'choice-first'
+syntax.
+
+Note that, as a consequence, the warning flags `-Wcontroller-can` and
+`-Wnocontroller-can` are not accepted anymore.
+
+See [Deprecation of `controller`-first syntax: Migrating](https://docs.daml.com/2.7.0/daml/reference/choices.html#migrating)
+for more information on how to adapt existing projects.
+
 ## What’s New
 ### Daml-script json support for --all
 The daml-script binary runner has been refactored to be more consistent across using `--script-name` and `--all`.  
diff --git a/bazel_tools/ghc-lib/version.bzl b/bazel_tools/ghc-lib/version.bzl
index 2c41e49c46df..660a82cdf402 100644
--- a/bazel_tools/ghc-lib/version.bzl
+++ b/bazel_tools/ghc-lib/version.bzl
@@ -9,7 +9,7 @@ GHC_LIB_PATCHES = [
 ]
 
 GHC_REPO_URL = "https://github.com/digital-asset/ghc"
-GHC_REV = "760149aceaf1c870b122308e8af792c1c4546091"
+GHC_REV = "6cea01bd8eec50722bffa9b3b1b9067653ee6123"
 GHC_PATCHES = [
 ]
 
diff --git a/compiler/daml-extension/syntaxes/TestGrammar.daml b/compiler/daml-extension/syntaxes/TestGrammar.daml
index c268fbacdd2a..862951429e91 100644
--- a/compiler/daml-extension/syntaxes/TestGrammar.daml
+++ b/compiler/daml-extension/syntaxes/TestGrammar.daml
@@ -29,14 +29,3 @@ template FeatureSuggestion
       do
         assert $ elem sponsor founders  -- assert -> source.daml
         create FeatureAssignment with employee, description, sponsor -- create -> source.daml
-
-    -- deprecated syntax: 'controller ... can' will be removed in a future version of Daml.
-    controller employee can
-      nonconsuming DoNothing : ()
-        do
-          return ()
-
-    -- deprecated syntax: 'controller ... can' will be removed in a future version of Daml.
-    controller employee can
-      Revoke : ()
-        do pure ()
diff --git a/compiler/damlc/tests/daml-test-files/ChoiceSyntaxes.daml b/compiler/damlc/tests/daml-test-files/ChoiceSyntaxes.daml
index 82494004a08d..06d66542b856 100644
--- a/compiler/damlc/tests/daml-test-files/ChoiceSyntaxes.daml
+++ b/compiler/damlc/tests/daml-test-files/ChoiceSyntaxes.daml
@@ -3,8 +3,6 @@
 
 -- Check that all syntaxes for defining choices can be parsed.
 
-{-# OPTIONS_GHC -Wno-controller-can #-}
-
 module ChoiceSyntaxes where
 
 template T with
@@ -22,15 +20,3 @@ template T with
       controller p
       do
         pure ()
-
-    -- deprecated syntax: 'controller ... can' will be removed in a future version of Daml.
-    -- This file tests the various options for defining choices, so we keep it until it is removed.
-    controller p can
-      C3 : () with
-          x : ()
-        do
-          pure  ()
-
-      C4 : ()
-        do
-          pure  ()
diff --git a/compiler/damlc/tests/daml-test-files/NoWarnControllerCan.daml b/compiler/damlc/tests/daml-test-files/NoWarnControllerCan.daml
deleted file mode 100644
index ebf38ce61e27..000000000000
--- a/compiler/damlc/tests/daml-test-files/NoWarnControllerCan.daml
+++ /dev/null
@@ -1,17 +0,0 @@
--- Copyright (c) 2021, Digital Asset (Switzerland) GmbH and/or its affiliates.
--- All rights reserved.
-
-{-# OPTIONS_GHC -Wno-controller-can #-}
-
-module NoWarnControllerCan where
-
-template T with
-    p : Party
-    q : Party
-  where
-    signatory p
-
-    controller q can
-      nonconsuming Pass : ()
-        do
-          pure ()
diff --git a/compiler/damlc/tests/daml-test-files/SemanticsEvalOrder.daml b/compiler/damlc/tests/daml-test-files/SemanticsEvalOrder.daml
index 634553a16221..8913bd84d61c 100644
--- a/compiler/damlc/tests/daml-test-files/SemanticsEvalOrder.daml
+++ b/compiler/damlc/tests/daml-test-files/SemanticsEvalOrder.daml
@@ -10,7 +10,6 @@
 -- the same rule, we use `R_1`, `R_2`, `R_3`, and so on. This is why there is
 -- `evExpRecUpdErr1`, `evExpRecUpdErr2_1`, and `evExpRecUpdErr2_2`, for example.)
 
-{-# OPTIONS_GHC -Wno-controller-can #-}
 {-# LANGUAGE ApplicativeDo #-}
 
 module SemanticsEvalOrder where
@@ -246,7 +245,7 @@ evUpdCreateErr1 = script do
     createCmd (T_EvUpdCreateErr1 p)
   abort "EvUpdCreateErr1 failed (4)"
 
--- @ERROR range=260:1-260:16; Template precondition violated
+-- @ERROR range=259:1-259:16; Template precondition violated
 template T_EvUpdCreateFail
   with
     p : Party
@@ -371,29 +370,6 @@ evUpdCreateWithKeyErr2 = script do
     createCmd (T_EvUpdCreateWithKeyErr2 p)
   abort "EvUpdCreateWithKeyErr2 failed"
 
--- | `controller P can ...` syntax adds `P` to the observers,
--- so if `P` causes an error, this is triggered on create,
--- instead of on exercise.
--- @ERROR controllerCanAddsObserver OK
-template T_ControllerCanAddsObserver
-  with
-    p : Party
-  where
-    signatory p
-    -- deprecated syntax: 'controller ... can' will be removed in a future version of Daml.
-    -- This tests that `controller X can` implicitly adds `X` as an observer, so we keep
-    -- it until it is removed.
-    controller (error @Party "controllerCanAddsObserver OK") can
-      C_ControllerCanAddsObserver: ()
-        do pure ()
-
-controllerCanAddsObserver : Script ()
-controllerCanAddsObserver = script do
-  p <- allocateParty "Alice"
-  submit p do
-    createCmd (T_ControllerCanAddsObserver p)
-  abort "controllerCanAddsObserver failed"
-
 -- | `choice ... controller P ...` doesn't add `P` as observer.
 -- This test is here to contrast with the previous.
 -- @ERROR choiceControllerDoesntAddObserver OK
diff --git a/compiler/damlc/tests/daml-test-files/SemanticsEvalOrderRightToLeft.daml b/compiler/damlc/tests/daml-test-files/SemanticsEvalOrderRightToLeft.daml
index 35edd30ea4ce..d1596187185d 100644
--- a/compiler/damlc/tests/daml-test-files/SemanticsEvalOrderRightToLeft.daml
+++ b/compiler/damlc/tests/daml-test-files/SemanticsEvalOrderRightToLeft.daml
@@ -10,7 +10,6 @@
 -- the same rule, we use `R_1`, `R_2`, `R_3`, and so on. This is why there is
 -- `evExpRecUpdErr1`, `evExpRecUpdErr2_1`, and `evExpRecUpdErr2_2`, for example.)
 
-{-# OPTIONS_GHC -Wno-controller-can #-}
 {-# LANGUAGE ApplicativeDo #-}
 
 module SemanticsEvalOrderRightToLeft where
@@ -246,7 +245,7 @@ evUpdCreateErr1 = script do
     createCmd (T_EvUpdCreateErr1 p)
   abort "EvUpdCreateErr1 failed (4)"
 
--- @ERROR range=260:1-260:16; Template precondition violated
+-- @ERROR range=259:1-259:16; Template precondition violated
 template T_EvUpdCreateFail
   with
     p : Party
@@ -371,29 +370,6 @@ evUpdCreateWithKeyErr2 = script do
     createCmd (T_EvUpdCreateWithKeyErr2 p)
   abort "EvUpdCreateWithKeyErr2 failed"
 
--- | `controller P can ...` syntax adds `P` to the observers,
--- so if `P` causes an error, this is triggered on create,
--- instead of on exercise.
--- @ERROR controllerCanAddsObserver OK
-template T_ControllerCanAddsObserver
-  with
-    p : Party
-  where
-    signatory p
-    -- deprecated syntax: 'controller ... can' will be removed in a future version of Daml.
-    -- This tests that `controller X can` implicitly adds `X` as an observer, so we keep
-    -- it until it is removed.
-    controller (error @Party "controllerCanAddsObserver OK") can
-      C_ControllerCanAddsObserver: ()
-        do pure ()
-
-controllerCanAddsObserver : Script ()
-controllerCanAddsObserver = script do
-  p <- allocateParty "Alice"
-  submit p do
-    createCmd (T_ControllerCanAddsObserver p)
-  abort "controllerCanAddsObserver failed"
-
 -- | `choice ... controller P ...` doesn't add `P` as observer.
 -- This test is here to contrast with the previous.
 -- @ERROR choiceControllerDoesntAddObserver OK
diff --git a/compiler/damlc/tests/daml-test-files/WarnControllerCan.daml b/compiler/damlc/tests/daml-test-files/WarnControllerCan.daml
deleted file mode 100644
index 24fb042c9a4b..000000000000
--- a/compiler/damlc/tests/daml-test-files/WarnControllerCan.daml
+++ /dev/null
@@ -1,17 +0,0 @@
--- Copyright (c) 2021, Digital Asset (Switzerland) GmbH and/or its affiliates.
--- All rights reserved.
-
--- @WARN range=14:5-17:18; The syntax 'controller ... can' is deprecated
-
-module WarnControllerCan where
-
-template T with
-    p : Party
-    q : Party
-  where
-    signatory p
-
-    controller q can
-      nonconsuming Pass : ()
-        do
-          pure ()
diff --git a/docs/source/daml/code-snippets/Reference.daml b/docs/source/daml/code-snippets/Reference.daml
index 34a813f374ff..e8aecaf630b5 100644
--- a/docs/source/daml/code-snippets/Reference.daml
+++ b/docs/source/daml/code-snippets/Reference.daml
@@ -1,8 +1,6 @@
 -- Copyright (c) 2023 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved.
 -- SPDX-License-Identifier: Apache-2.0
 
-{-# OPTIONS -Wno-controller-can #-}
-
 -- template-let warning disabled because 'start template let snippet' is
 -- used by docs.daml.com for older compiler versions and to explain
 -- new deprecation warning in 2.8.0
@@ -50,6 +48,63 @@ template NameOfTemplate
     key (exampleParty, exampleParam) : (Party, Text)
     maintainer (exampleFunction key)
 -- end contract key snippet
+
+-- start new template choice snippet
+    choice NameOfChoice
+      : ()  -- replace () with the actual return type
+      with
+        exampleParameter : Text -- parameters here
+      controller exampleParty
+      do
+        return () -- replace this line with the choice body
+-- end new template choice snippet
+
+-- start new choice name snippet
+    choice ExampleChoice
+      : () -- replace () with the actual return type
+-- end new choice name snippet
+-- start new choice params snippet
+      with
+        exampleParameter : Text
+-- end new choice params snippet
+-- start new controller snippet
+      controller exampleParty
+-- end new controller snippet
+      do
+        return () -- replace () with the actual return type
+-- start new preconsuming snippet
+    preconsuming choice ExamplePreconsumingChoice
+      : () -- replace () with the actual return type
+-- end new preconsuming snippet
+      with -- params
+      controller exampleParty
+      do
+        return ()
+-- start new postconsuming snippet
+    postconsuming choice ExamplePostconsumingChoice
+      : () -- replace () with the actual return type
+-- end new postconsuming snippet
+      with -- params
+      controller exampleParty
+      do
+        return ()
+-- start new nonconsuming snippet
+    nonconsuming choice ExampleNonconsumingChoice
+      : () -- replace () with the actual return type
+-- end new nonconsuming snippet
+      with -- params
+      controller exampleParty
+      do
+        return ()
+
+{- LEGACY CHOICE SNIPPETS
+Before Daml 2.8.0, two syntaxes were supported for declaring choices,
+choice name first and controller-first. The latter was removed, so the
+code below doesn't compile anymore, but docs.daml.com uses the version
+of this file in the `main` branch, including when building the docs
+for Daml <2.8.0, so it's easier to leave the old snippets alone for
+those versions.
+
 -- start template choice snippet
     -- option 1 for specifying choices: choice name first
     choice NameOfChoice1
@@ -147,3 +202,4 @@ template NameOfTemplate
 -- end controller-first nonconsuming snippet
         do
           return () -- replace () with the actual return type
+-}
diff --git a/docs/source/daml/code-snippets/Structure.daml b/docs/source/daml/code-snippets/Structure.daml
index 7c844ebb7629..3b919b2b4306 100644
--- a/docs/source/daml/code-snippets/Structure.daml
+++ b/docs/source/daml/code-snippets/Structure.daml
@@ -1,7 +1,6 @@
 -- Copyright (c) 2023 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved.
 -- SPDX-License-Identifier: Apache-2.0
 
-{-# OPTIONS -Wno-controller-can #-}
 module Structure where
 
 exampleFunction : (Party, Text) -> Party
@@ -28,6 +27,36 @@ template NameOfTemplate
     maintainer (exampleFunction key)
     -- a choice goes here; see next section
     -- end of template outline snippet
+
+    -- start of new choice snippet
+    choice NameOfChoice
+      : () -- replace () with the actual return type
+      with
+        party : Party -- parameters here
+      controller party
+      do
+        return () -- replace this line with the choice body
+    -- end of new choice snippet
+
+    -- start of new choice observer snippet
+    choice NameOfChoiceWithObserver
+      : () -- replace () with the actual return type
+      with
+        party : Party -- parameters here
+      observer party -- optional specification of choice observers
+      controller exampleParty
+      do
+        return () -- replace this line with the choice body
+    -- end of new choice observer snippet
+
+    {- LEGACY CHOICE SNIPPETS
+    Before Daml 2.8.0, two syntaxes were supported for declaring choices,
+    choice name first and controller-first. The latter was removed, so the
+    code below doesn't compile anymore, but docs.daml.com uses the version
+    of this file in the `main` branch, including when building the docs
+    for Daml <2.8.0, so it's easier to leave the old snippets alone for
+    those versions.
+
     -- start of choice snippet
     -- option 1 for specifying choices: choice name first
     choice NameOfChoice
@@ -46,7 +75,7 @@ template NameOfTemplate
           party : Party -- parameters here
         do
           return () -- replace the line with the choice body
-      -- end of choice snippet
+    -- end of choice snippet
 
     -- start of choice observer snippet
     -- choice observers may be specified if option 1 is used
@@ -59,3 +88,4 @@ template NameOfTemplate
       do
         return () -- replace this line with the choice body
     -- end of choice observer snippet
+    -}
diff --git a/language-support/java/codegen/src/ledger-tests/daml/Stakeholders.daml b/language-support/java/codegen/src/ledger-tests/daml/Stakeholders.daml
index ce1e57aec62c..50b7f6cb7e13 100644
--- a/language-support/java/codegen/src/ledger-tests/daml/Stakeholders.daml
+++ b/language-support/java/codegen/src/ledger-tests/daml/Stakeholders.daml
@@ -1,6 +1,5 @@
 -- Copyright (c) 2023 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved.
 -- SPDX-License-Identifier: Apache-2.0
-{-# OPTIONS -Wno-controller-can #-}
 
 module Stakeholders where
 
@@ -10,25 +9,6 @@ template OnlySignatories
   where
     signatory owner
 
-    -- deprecated syntax: 'controller ... can' will be removed in a future version of Daml.
-    -- This file tests the various options for defining choices, so we keep it until it is removed.
-    controller owner can
-      OnlySignatories_Choice : ()
-        do return ()
-
-template ImplicitObservers
-  with
-    owner: Party
-    thirdParty: Party
-  where
-    signatory owner
-
-    -- deprecated syntax: 'controller ... can' will be removed in a future version of Daml.
-    -- This file tests the various options for defining choices, so we keep it until it is removed.
-    controller thirdParty can
-      ImplicitObservers_Choice : ()
-        do return ()
-
 template ExplicitObservers
   with
     owner: Party
@@ -36,24 +16,3 @@ template ExplicitObservers
   where
     signatory owner
     observer thirdParty
-
-    -- deprecated syntax: 'controller ... can' will be removed in a future version of Daml.
-    -- This file tests the various options for defining choices, so we keep it until it is removed.
-    controller owner can
-      ExplicitObservers_Choice : ()
-        do return ()
-
-template MixedObservers
-  with
-    owner: Party
-    thirdParty1: Party
-    thirdParty2: Party
-  where
-    signatory owner
-    observer thirdParty1
-
-    -- deprecated syntax: 'controller ... can' will be removed in a future version of Daml.
-    -- This file tests the various options for defining choices, so we keep it until it is removed.
-    controller thirdParty2 can
-      MixedObservers_Choice : ()
-        do return ()
diff --git a/language-support/java/codegen/src/ledger-tests/scala/com/digitalasset/StakeholdersTest.scala b/language-support/java/codegen/src/ledger-tests/scala/com/digitalasset/StakeholdersTest.scala
index ca04ea7efb0c..4bd71756d3dd 100644
--- a/language-support/java/codegen/src/ledger-tests/scala/com/digitalasset/StakeholdersTest.scala
+++ b/language-support/java/codegen/src/ledger-tests/scala/com/digitalasset/StakeholdersTest.scala
@@ -9,7 +9,7 @@ import io.grpc.Channel
 import org.scalatest.Assertion
 import org.scalatest.flatspec.AsyncFlatSpecLike
 import org.scalatest.matchers.should.Matchers
-import stakeholders.{ExplicitObservers, ImplicitObservers, MixedObservers, OnlySignatories}
+import stakeholders.{ExplicitObservers, OnlySignatories}
 
 import scala.concurrent.Future
 
@@ -26,20 +26,16 @@ trait StakeholdersTest
       testCode: (
           String,
           OnlySignatories,
-          ImplicitObservers,
           ExplicitObservers,
-          MixedObservers,
           Channel,
       ) => Assertion
   ): Future[Assertion] = {
     for {
       List(alice, bob, charlie) <- Future.sequence(List.fill(3)(allocateParty))
       onlySignatories = new OnlySignatories(alice)
-      implicitObservers = new ImplicitObservers(alice, bob)
       explicitObservers = new ExplicitObservers(alice, bob)
-      mixedObservers = new MixedObservers(alice, bob, charlie)
       result <- withClient(
-        testCode(alice, onlySignatories, implicitObservers, explicitObservers, mixedObservers, _)
+        testCode(alice, onlySignatories, explicitObservers, _)
       )
     } yield result
   }
@@ -47,7 +43,7 @@ trait StakeholdersTest
   behavior of "Stakeholders"
 
   they should "be exposed correctly for contracts without observers" in withUniqueParties {
-    (alice, onlySignatories, _, _, _, client) =>
+    (alice, onlySignatories, _, client) =>
       sendCmd(client, alice, onlySignatories.create())
 
       val contract :: _ =
@@ -57,19 +53,8 @@ trait StakeholdersTest
       contract.observers shouldBe empty
   }
 
-  they should "be exposed correctly for contracts with only implicit observers" in withUniqueParties {
-    (alice, _, implicitObservers, _, _, client) =>
-      sendCmd(client, alice, implicitObservers.create())
-
-      val contract :: _ =
-        readActiveContracts(ImplicitObservers.Contract.fromCreatedEvent)(client, alice)
-
-      contract.signatories should contain only implicitObservers.owner
-      contract.observers should contain only implicitObservers.thirdParty
-  }
-
   they should "be exposed correctly for contracts with only explicit observers" in withUniqueParties {
-    (alice, _, _, explicitObservers, _, client) =>
+    (alice, _, explicitObservers, client) =>
       sendCmd(client, alice, explicitObservers.create())
 
       val contract :: _ =
@@ -78,16 +63,4 @@ trait StakeholdersTest
       contract.signatories should contain only explicitObservers.owner
       contract.observers should contain only explicitObservers.thirdParty
   }
-
-  they should "be exposed correctly for contracts with both implicit and explicit observers" in withUniqueParties {
-    (alice, _, _, _, mixedObservers, client) =>
-      sendCmd(client, alice, mixedObservers.create())
-
-      val contract :: _ =
-        readActiveContracts(MixedObservers.Contract.fromCreatedEvent)(client, alice)
-
-      contract.signatories should contain only mixedObservers.owner
-      contract.observers should contain.only(mixedObservers.thirdParty1, mixedObservers.thirdParty2)
-  }
-
 }