Attached a patch which introduces a value semantic for the Converter list.
theConverters now doesn't return a reference any more but a copy of the list.
This way we could remove the locks in Graph.cpp.
The returned Converters is const, so the compiler shows up the places where
we touch the converter list where a other function must be called to get a
reference.
This way we get much more const correctness!
Maybe the mutual in Graph.h isn't perfect, but the implementation of Graph
also changes internal variable even when a in principle const function is
called.
But I'm not sure if this patch is more expensive than the locks in Graph.cpp,
and we have to introduce more such changes on all global lists.
Peter
Index: src/LyX.cpp
===================================================================
--- src/LyX.cpp (Revision 37226)
+++ src/LyX.cpp (Arbeitskopie)
@@ -1320,20 +1320,34 @@
}
-Converters & theConverters()
+Converters const theConverters()
{
LASSERT(singleton_, /**/);
return singleton_->pimpl_->converters_;
}
-Converters & theSystemConverters()
+Converters & refConverters()
{
LASSERT(singleton_, /**/);
+ return singleton_->pimpl_->converters_;
+}
+
+
+Converters const theSystemConverters()
+{
+ LASSERT(singleton_, /**/);
return singleton_->pimpl_->system_converters_;
}
+Converters & refSystemConverters()
+{
+ LASSERT(singleton_, /**/);
+ return singleton_->pimpl_->system_converters_;
+}
+
+
Movers & theMovers()
{
LASSERT(singleton_, /**/);
Index: src/Graph.h
===================================================================
--- src/Graph.h (Revision 37226)
+++ src/Graph.h (Arbeitskopie)
@@ -13,8 +13,6 @@
#ifndef GRAPH_H
#define GRAPH_H
-#include "support/mutex.h"
-
#include <list>
#include <queue>
#include <vector>
@@ -31,15 +29,15 @@
///
typedef std::vector<int> EdgePath;
/// \return a vector of the vertices from which "to" can be reached
- std::vector<int> const getReachableTo(int to, bool clear_visited);
+ std::vector<int> const getReachableTo(int to, bool clear_visited) const;
/// \return a vector of the vertices that can be reached from "from"
std::vector<int> const
- getReachable(int from, bool only_viewable, bool clear_visited);
+ getReachable(int from, bool only_viewable, bool clear_visited) const;
/// can "from" be reached from "to"?
- bool isReachable(int from, int to);
+ bool isReachable(int from, int to) const;
/// find a path from "from" to "to". always returns one of the
/// shortest such paths.
- EdgePath const getPath(int from, int to);
+ EdgePath const getPath(int from, int to) const;
/// called repeatedly to build the graph
void addEdge(int from, int to);
/// reset the internal data structures
@@ -47,10 +45,10 @@
private:
///
- bool bfs_init(int, bool clear_visited = true);
+ bool bfs_init(int, bool clear_visited = true) const;
/// clears the paths from a previous search. should be
/// called before each new one.
- void clearPaths();
+ void clearPaths() const;
/// used to recover a marked path
void getMarkedPath(int from, int to, EdgePath & path);
/// these represent the arrows connecting the nodes of the graph.
@@ -93,18 +91,15 @@
/// between these indices and the objects they represent. (in the case
/// of Format, this is easy, since the Format objects already have ints
/// as identifiers.)
- std::vector<Vertex> vertices_;
+ mutable std::vector<Vertex> vertices_;
///
- std::queue<int> Q_;
+ mutable std::queue<int> Q_;
/// a counter that we use to assign id's to the arrows
/// FIXME This technique assumes a correspondence between the
/// ids of the arrows and ids associated with Converters that
/// seems kind of fragile. Perhaps a better solution would be
/// to pass the ids as we create the arrows.
int numedges_;
-
- /// make public functions thread save
- Mutex mutex_;
};
Index: src/Converter.h
===================================================================
--- src/Converter.h (Revision 37226)
+++ src/Converter.h (Arbeitskopie)
@@ -93,24 +93,24 @@
void sort();
///
std::vector<Format const *> const
- getReachableTo(std::string const & target, bool clear_visited);
+ getReachableTo(std::string const & target, bool clear_visited) const;
///
std::vector<Format const *> const
getReachable(std::string const & from, bool only_viewable,
- bool clear_visited);
+ bool clear_visited) const;
- std::vector<Format const *> importableFormats();
- std::vector<Format const *> exportableFormats(bool only_viewable);
+ std::vector<Format const *> importableFormats() const;
+ std::vector<Format const *> exportableFormats(bool only_viewable) const;
std::vector<std::string> loaders() const;
std::vector<std::string> savers() const;
/// Does a conversion path from format \p from to format \p to exist?
- bool isReachable(std::string const & from, std::string const & to);
+ bool isReachable(std::string const & from, std::string const & to) const;
///
- Graph::EdgePath getPath(std::string const & from, std::string const & to);
+ Graph::EdgePath getPath(std::string const & from, std::string const & to) const;
///
- OutputParams::FLAVOR getFlavor(Graph::EdgePath const & path);
+ OutputParams::FLAVOR getFlavor(Graph::EdgePath const & path) const;
/// Flags for converting files
enum ConversionFlags {
/// No special flags
@@ -125,29 +125,30 @@
support::FileName const & from_file, support::FileName const & to_file,
support::FileName const & orig_from,
std::string const & from_format, std::string const & to_format,
- ErrorList & errorList, int conversionflags = none);
+ ErrorList & errorList, int conversionflags = none) const;
///
void update(Formats const & formats);
///
void updateLast(Formats const & formats);
///
- bool formatIsUsed(std::string const & format);
+ bool formatIsUsed(std::string const & format) const;
///
const_iterator begin() const { return converterlist_.begin(); }
///
const_iterator end() const { return converterlist_.end(); }
///
void buildGraph();
+
private:
///
std::vector<Format const *> const
- intToFormat(std::vector<int> const & input);
+ intToFormat(std::vector<int> const & input) const;
///
bool scanLog(Buffer const & buffer, std::string const & command,
- support::FileName const & filename, ErrorList & errorList);
+ support::FileName const & filename, ErrorList & errorList) const;
///
bool runLaTeX(Buffer const & buffer, std::string const & command,
- OutputParams const &, ErrorList & errorList);
+ OutputParams const &, ErrorList & errorList) const;
///
ConverterList converterlist_;
///
@@ -156,19 +157,27 @@
/// this method moves each /path/file*.ext file to /path2/file2*.ext2
bool move(std::string const & fmt,
support::FileName const & from, support::FileName const & to,
- bool copy);
+ bool copy) const;
///
Graph G_;
};
/// The global instance.
/// Implementation is in LyX.cpp.
-extern Converters & theConverters();
+extern Converters const theConverters();
+/// If the Converters need to be changed
+/// explicitly call a other function to get a reference
+extern Converters & refConverters();
+
/// The global copy after reading lyxrc.defaults.
/// Implementation is in LyX.cpp.
-extern Converters & theSystemConverters();
+extern Converters const theSystemConverters();
+/// If the Converters need to be changed
+/// explicitly call a other function to get a reference
+extern Converters & refSystemConverters();
+
} // namespace lyx
#endif //CONVERTER_H
Index: src/Converter.cpp
===================================================================
--- src/Converter.cpp (Revision 37226)
+++ src/Converter.cpp (Arbeitskopie)
@@ -252,7 +252,7 @@
}
-OutputParams::FLAVOR Converters::getFlavor(Graph::EdgePath const & path)
+OutputParams::FLAVOR Converters::getFlavor(Graph::EdgePath const & path) const
{
for (Graph::EdgePath::const_iterator cit = path.begin();
cit != path.end(); ++cit) {
@@ -275,7 +275,7 @@
FileName const & from_file, FileName const & to_file,
FileName const & orig_from,
string const & from_format, string const & to_format,
- ErrorList & errorList, int conversionflags)
+ ErrorList & errorList, int conversionflags) const
{
if (from_format == to_format)
return move(from_format, from_file, to_file, false);
@@ -513,7 +513,7 @@
bool Converters::move(string const & fmt,
- FileName const & from, FileName const & to, bool copy)
+ FileName const & from, FileName const & to, bool copy) const
{
if (from == to)
return true;
@@ -553,7 +553,7 @@
}
-bool Converters::formatIsUsed(string const & format)
+bool Converters::formatIsUsed(string const & format) const
{
ConverterList::const_iterator cit = converterlist_.begin();
ConverterList::const_iterator end = converterlist_.end();
@@ -566,7 +566,7 @@
bool Converters::scanLog(Buffer const & buffer, string const & /*command*/,
- FileName const & filename, ErrorList & errorList)
+ FileName const & filename, ErrorList & errorList) const
{
OutputParams runparams(0);
runparams.flavor = OutputParams::LATEX;
@@ -596,7 +596,7 @@
bool Converters::runLaTeX(Buffer const & buffer, string const & command,
- OutputParams const & runparams, ErrorList & errorList)
+ OutputParams const & runparams, ErrorList & errorList) const
{
buffer.setBusy(true);
buffer.message(_("Running LaTeX..."));
@@ -658,7 +658,7 @@
vector<Format const *> const
-Converters::intToFormat(vector<int> const & input)
+Converters::intToFormat(vector<int> const & input) const
{
vector<Format const *> result(input.size());
@@ -673,7 +673,7 @@
vector<Format const *> const
-Converters::getReachableTo(string const & target, bool const clear_visited)
+Converters::getReachableTo(string const & target, bool const clear_visited) const
{
vector<int> const & reachablesto =
G_.getReachableTo(formats.getNumber(target), clear_visited);
@@ -684,7 +684,7 @@
vector<Format const *> const
Converters::getReachable(string const & from, bool const only_viewable,
- bool const clear_visited)
+ bool const clear_visited) const
{
vector<int> const & reachables =
G_.getReachable(formats.getNumber(from),
@@ -695,21 +695,21 @@
}
-bool Converters::isReachable(string const & from, string const & to)
+bool Converters::isReachable(string const & from, string const & to) const
{
return G_.isReachable(formats.getNumber(from),
formats.getNumber(to));
}
-Graph::EdgePath Converters::getPath(string const & from, string const & to)
+Graph::EdgePath Converters::getPath(string const & from, string const & to) const
{
return G_.getPath(formats.getNumber(from),
formats.getNumber(to));
}
-vector<Format const *> Converters::importableFormats()
+vector<Format const *> Converters::importableFormats() const
{
vector<string> l = loaders();
vector<Format const *> result = getReachableTo(l[0], true);
@@ -722,7 +722,7 @@
}
-vector<Format const *> Converters::exportableFormats(bool only_viewable)
+vector<Format const *> Converters::exportableFormats(bool only_viewable) const
{
vector<string> s = savers();
vector<Format const *> result = getReachable(s[0], only_viewable, true);
Index: src/frontends/qt4/GuiPrefs.cpp
===================================================================
--- src/frontends/qt4/GuiPrefs.cpp (Revision 37226)
+++ src/frontends/qt4/GuiPrefs.cpp (Arbeitskopie)
@@ -1560,7 +1560,7 @@
void PrefConverters::updateGui()
{
form_->formats().sort();
- form_->converters().update(form_->formats());
+ form_->refConverters().update(form_->formats());
// save current selection
QString current = converterFromCO->currentText()
+ " -> " + converterToCO->currentText();
@@ -1667,10 +1667,10 @@
Converter const * old =
form_->converters().getConverter(from.name(), to.name());
- form_->converters().add(from.name(), to.name(), command, flags);
+ form_->refConverters().add(from.name(), to.name(), command, flags);
if (!old)
- form_->converters().updateLast(form_->formats());
+ form_->refConverters().updateLast(form_->formats());
updateGui();
@@ -1684,7 +1684,7 @@
{
Format const & from = form_->formats().get(converterFromCO->currentIndex());
Format const & to = form_->formats().get(converterToCO->currentIndex());
- form_->converters().erase(from.name(), to.name());
+ form_->refConverters().erase(from.name(), to.name());
updateGui();
@@ -3214,9 +3214,9 @@
lyx::formats = formats_;
- theConverters() = converters_;
- theConverters().update(lyx::formats);
- theConverters().buildGraph();
+ refConverters() = converters_;
+ refConverters().update(lyx::formats);
+ refConverters().buildGraph();
theMovers() = movers_;
Index: src/frontends/qt4/GuiPrefs.h
===================================================================
--- src/frontends/qt4/GuiPrefs.h (Revision 37226)
+++ src/frontends/qt4/GuiPrefs.h (Arbeitskopie)
@@ -109,7 +109,8 @@
int fromPaperSize(PAPER_SIZE papersize) const;
LyXRC & rc() { return rc_; }
- Converters & converters() { return converters_; }
+ Converters const converters() { return converters_; }
+ Converters & refConverters() { return converters_; }
Formats & formats() { return formats_; }
Movers & movers() { return movers_; }
Index: src/LyXRC.cpp
===================================================================
--- src/LyXRC.cpp (Revision 37226)
+++ src/LyXRC.cpp (Arbeitskopie)
@@ -1039,9 +1039,9 @@
if (lexrc.next())
flags = lexrc.getString();
if (command.empty())
- theConverters().erase(from, to);
+ refConverters().erase(from, to);
else
- theConverters().add(from, to, command, flags);
+ refConverters().add(from, to, command, flags);
break;
}
// compatibility with versions older than 1.4.0 only
@@ -1270,8 +1270,8 @@
}
/// Update converters data-structures
- theConverters().update(formats);
- theConverters().buildGraph();
+ refConverters().update(formats);
+ refConverters().buildGraph();
return 0;
}
Index: src/LyX.h
===================================================================
--- src/LyX.h (Revision 37226)
+++ src/LyX.h (Arbeitskopie)
@@ -139,8 +139,10 @@
friend BufferList & theBufferList();
friend Server & theServer();
friend ServerSocket & theServerSocket();
- friend Converters & theConverters();
- friend Converters & theSystemConverters();
+ friend Converters const theConverters();
+ friend Converters & refConverters();
+ friend Converters const theSystemConverters();
+ friend Converters & refSystemConverters();
friend Messages const & getMessages(std::string const & language);
friend Messages const & getGuiMessages();
friend KeyMap & theTopLevelKeymap();
Index: src/Graph.cpp
===================================================================
--- src/Graph.cpp (Revision 37227)
+++ src/Graph.cpp (Arbeitskopie)
@@ -19,12 +19,13 @@
#include <algorithm>
+
using namespace std;
namespace lyx {
-bool Graph::bfs_init(int s, bool clear_visited)
+bool Graph::bfs_init(int s, bool clear_visited) const
{
if (s < 0)
return false;
@@ -45,7 +46,7 @@
}
-void Graph::clearPaths()
+void Graph::clearPaths() const
{
vector<Vertex>::iterator it = vertices_.begin();
vector<Vertex>::iterator en = vertices_.end();
@@ -55,10 +56,8 @@
vector<int> const
- Graph::getReachableTo(int target, bool clear_visited)
+ Graph::getReachableTo(int target, bool clear_visited) const
{
- Mutex::Locker lock(&mutex_);
-
vector<int> result;
if (!bfs_init(target, clear_visited))
return result;
@@ -92,10 +91,8 @@
vector<int> const
Graph::getReachable(int from, bool only_viewable,
- bool clear_visited)
+ bool clear_visited) const
{
- Mutex::Locker lock(&mutex_);
-
vector<int> result;
if (!bfs_init(from, clear_visited))
return result;
@@ -130,10 +127,8 @@
}
-bool Graph::isReachable(int from, int to)
+bool Graph::isReachable(int from, int to) const
{
- Mutex::Locker lock(&mutex_);
-
if (from == to)
return true;
@@ -163,16 +158,15 @@
}
-Graph::EdgePath const Graph::getPath(int from, int to)
+Graph::EdgePath const Graph::getPath(int from, int to) const
{
- Mutex::Locker lock(&mutex_);
-
if (from == to)
return EdgePath();
if (to < 0 || !bfs_init(from))
return EdgePath();
+
clearPaths();
while (!Q_.empty()) {
int const current = Q_.front();
@@ -206,8 +200,6 @@
void Graph::init(int size)
{
- Mutex::Locker lock(&mutex_);
-
vertices_ = vector<Vertex>(size);
arrows_.clear();
numedges_ = 0;
@@ -216,8 +208,6 @@
void Graph::addEdge(int from, int to)
{
- Mutex::Locker lock(&mutex_);
-
arrows_.push_back(Arrow(from, to, numedges_));
numedges_++;
Arrow * ar = &(arrows_.back());
Index: src/Buffer.cpp
===================================================================
--- src/Buffer.cpp (Revision 37226)
+++ src/Buffer.cpp (Arbeitskopie)
@@ -3479,11 +3479,12 @@
vector<string> backs = backends();
if (find(backs.begin(), backs.end(), format) == backs.end()) {
// Get shortest path to format
- theConverters().buildGraph();
+ Converters converters = theConverters();
+ converters.buildGraph();
Graph::EdgePath path;
for (vector<string>::const_iterator it = backs.begin();
it != backs.end(); ++it) {
- Graph::EdgePath p = theConverters().getPath(*it, format);
+ Graph::EdgePath p = converters.getPath(*it, format);
if (!p.empty() && (path.empty() || p.size() < path.size())) {
backend_format = *it;
path = p;
