From 2665b5780a3962dff158452b8957ecfb320581d5 Mon Sep 17 00:00:00 2001
From: Alexandre Duret-Lutz <adl@lrde.epita.fr>
Date: Thu, 7 Jul 2016 15:57:14 +0200
Subject: [PATCH] relabel: do not unregister old AP that are also new

Reported by Ayrat Khalimov against the trans.html page when using
ltl3ba.

* spot/twaalgos/relabel.cc: Here.
* tests/core/ltl3dra.test: Test it.
* NEWS: Mention it.
* THANKS: Add Ayrat.
---
 NEWS                     | 4 ++++
 THANKS                   | 1 +
 spot/twaalgos/relabel.cc | 8 +++++++-
 tests/core/ltl3dra.test  | 8 +++++++-
 4 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index dd6b179c3..f732c6002 100644
--- a/NEWS
+++ b/NEWS
@@ -13,6 +13,10 @@ New in spot 2.0.2a (Not yet released)
   * The generalized testing automata displayed by the on-line
     translator were incorrect (those output by ltl2tgta were OK).
   * ltl2tgta should not offer options --ba, --monitor, --tgba and such.
+  * the relabel() function could incorrectly unregister old atomic
+    propositions even if they are still used in the output (e.g., if
+    a&p0 is relabeled to p0&p1).  This could cause ltldo and on-line
+    translator to report errors.
 
 New in spot 2.0.2 (2016-06-17)
 
diff --git a/THANKS b/THANKS
index 4c9aa543a..0b5832d1a 100644
--- a/THANKS
+++ b/THANKS
@@ -2,6 +2,7 @@ We are grateful to these people for their comments, help, or
 suggestions.
 
 Akim Demaille
+Ayrat Khalimov
 Caroline Lemieux
 Christian Dax
 Christopher Ziegler
diff --git a/spot/twaalgos/relabel.cc b/spot/twaalgos/relabel.cc
index 0c8fb29fe..906e30795 100644
--- a/spot/twaalgos/relabel.cc
+++ b/spot/twaalgos/relabel.cc
@@ -27,6 +27,7 @@ namespace spot
     bddPair* pairs = bdd_newpair();
     auto d = aut->get_dict();
     std::vector<int> vars;
+    std::set<int> newvars;
     vars.reserve(relmap->size());
     for (auto& p: *relmap)
       {
@@ -34,10 +35,15 @@ namespace spot
         int newv = aut->register_ap(p.second);
         bdd_setpair(pairs, oldv, newv);
         vars.push_back(oldv);
+        newvars.insert(newv);
       }
     for (auto& t: aut->edges())
       t.cond = bdd_replace(t.cond, pairs);
+    // Erase all the old variable that are not reused in the new set.
+    // (E.g., if we relabel a&p0 into p0&p1 we should not unregister
+    // p0)
     for (auto v: vars)
-      aut->unregister_ap(v);
+      if (newvars.find(v) == newvars.end())
+        aut->unregister_ap(v);
   }
 }
diff --git a/tests/core/ltl3dra.test b/tests/core/ltl3dra.test
index e4f7af1fa..5c9b3743f 100755
--- a/tests/core/ltl3dra.test
+++ b/tests/core/ltl3dra.test
@@ -1,6 +1,6 @@
 #!/bin/sh
 # -*- coding: utf-8 -*-
-# Copyright (C) 2015 Laboratoire de Recherche et
+# Copyright (C) 2015, 2016 Laboratoire de Recherche et
 # Développement de l'Epita (LRDE).
 #
 # This file is part of Spot, a model checking library.
@@ -35,3 +35,9 @@ ltlcross 'ltl2tgba' 'ltl3dra' -f '(<>((((p0) &&
 && ([](p3))) || ((p1) && (!([](p3)))))))) || ((!(p0)) && ([](((!(p1))
 && ([](p3))) || ((p1) && (!([](p3))))))))) && (((p0) && (!(<>(p2))))
 || ((!(p0)) && (<>(p2)))))))'
+
+
+# This used to trigger an assertion because the formula "a=0"&p0 was
+# relabeled p0&p1, and then p0 was unregistered despite being one of
+# the new variables.
+ltldo ltl3dra -f '"a=0" & p0' | grep 'AP: 2.*p0'