https://svn.lrde.epita.fr/svn/nolimips/trunk
ChangeLog | 14 ++++++++++++++ src/inst/label.hh | 6 +++--- src/inst/label_exp.hh | 2 +- src/inst/section.hh | 4 ++-- src/misc/test-unique_string.cc | 6 +++--- src/misc/unique_string.cc | 13 +++++++------ src/misc/unique_string.hh | 29 +++++++++++++---------------- src/parse/asm-parse.yy.gen.py | 5 ++++- src/parse/asm-scan.ll.gen.py | 6 +++--- src/shell/shell.cc | 3 +-- src/vm/virtual_machine.cc | 2 +- 11 files changed, 52 insertions(+), 38 deletions(-)
Index: ChangeLog from Benoît Perrot benoit@lrde.epita.fr
Simplify unique_string construction.
* src/misc/unique_string.hh, src/misc/unique_string.cc (create): Remove. Move its implementation to unique_string's constructor, hence make unique_string aggregate a pointer-to-string instead of a reference. * src/misc/test-unique_string.cc, src/vm/virtual_machine.cc, * src/shell/shell.cc, src/parse/asm-parse.yy.gen.py, * src/parse/asm-scan.ll.gen.py, src/inst/label_exp.hh, * src/inst/section.hh, src/inst/label.hh: Adjust.
Index: src/vm/virtual_machine.cc --- src/vm/virtual_machine.cc (revision 187) +++ src/vm/virtual_machine.cc (revision 188) @@ -35,7 +35,7 @@ { reset();
- const misc::unique_string &main = misc::unique_string::create("main"); + misc::unique_string main("main"); if (! program.text_section ().has_label(main)) { std::cerr << "No `main' label in assembly file." << std::endl; Index: src/misc/unique_string.cc --- src/misc/unique_string.cc (revision 187) +++ src/misc/unique_string.cc (revision 188) @@ -18,23 +18,24 @@ //
#include "misc/unique_string.hh" +#include "misc/contract.hh"
namespace misc {
unique_string::string_to_unique_type unique_string::pool_;
- const unique_string & - unique_string::create(const std::string &str) + unique_string::unique_string(const std::string &str) { unique_string::string_to_unique_type::iterator it = pool_.find(&str); - if (it == pool_.end() || it->first->compare(str)) + if (it == pool_.end() || (*it)->compare(str)) { std::string *s = new std::string(str); - it = pool_.insert(it, unique_string::string_to_unique_type:: - value_type(s, unique_string(*s))); + it = pool_.insert(it, s); } - return it->second; + + str_ = *it; + assertion(str_); }
} // namespace misc Index: src/misc/unique_string.hh --- src/misc/unique_string.hh (revision 187) +++ src/misc/unique_string.hh (revision 188) @@ -26,7 +26,7 @@ the implementation is quite different. */
#include <string> -#include <map> +#include <set> #include <iostream>
namespace misc @@ -44,32 +44,28 @@ } };
- typedef std::map<const std::string*, unique_string, + typedef std::set<const std::string*, string_ptr_less> string_to_unique_type;
- private: - unique_string(const std::string &str): - str_(str) - {} public: - static const unique_string &create(const std::string &str); + unique_string(const std::string &str);
// Accessors public: const std::string &get() const { - return str_; + return *str_; } operator const std::string &() const { - return str_; + return *str_; } const std::string & operator* () const { - return str_; + return *str_; }
- static unsigned pool_size() + static unsigned pool_size() const { return pool_.size(); } @@ -78,24 +74,25 @@ public: bool operator==(const unique_string &rhs) const { - return &str_ == &rhs.str_; + return str_ == rhs.str_; } bool operator!=(const unique_string &rhs) const { - return &str_ != &rhs.str_; + return str_ != rhs.str_; } bool operator<(const unique_string &rhs) const { - if (&str_ == &rhs.str_) + if (str_ == rhs.str_) return false; - return str_ < rhs.str_; + + return *str_ < *(rhs.str_); }
private: static string_to_unique_type pool_;
private: - const std::string &str_; + const std::string *str_; };
inline std::ostream & Index: src/misc/test-unique_string.cc --- src/misc/test-unique_string.cc (revision 187) +++ src/misc/test-unique_string.cc (revision 188) @@ -22,9 +22,9 @@ int main() { - const misc::unique_string &foo0 = misc::unique_string::create("foo"); - const misc::unique_string &foo1 = misc::unique_string::create("foo"); - const misc::unique_string &bar = misc::unique_string::create("bar"); + misc::unique_string foo0("foo"); + misc::unique_string foo1("foo"); + misc::unique_string bar("bar");
assertion(misc::unique_string::pool_size() == 2); assertion(*foo0 == "foo"); Index: src/shell/shell.cc --- src/shell/shell.cc (revision 187) +++ src/shell/shell.cc (revision 188) @@ -470,8 +470,7 @@ std::cerr << "No program loaded." << std::endl; else { - const misc::unique_string &label = - misc::unique_string::create(*cmd.get_args().begin()); + misc::unique_string label(*cmd.get_args().begin());
if (!program_->text_section().has_label(label)) std::cerr << "Label " << label Index: src/parse/asm-parse.yy.gen.py --- src/parse/asm-parse.yy.gen.py (revision 187) +++ src/parse/asm-parse.yy.gen.py (revision 188) @@ -75,7 +75,7 @@ { int i; std::string *s; - const misc::unique_string *id; + misc::unique_string *id; inst::Label *label; inst::Register *reg; inst::Exp *exp; @@ -92,6 +92,9 @@
%destructor { delete $$; } "string"
+%destructor { delete $$; } "label" +%destructor { delete $$; } "label definition" + %printer { debug_stream () << *$$; } "string" "register" %printer { debug_stream () << $$; } "integer"
Index: src/parse/asm-scan.ll.gen.py --- src/parse/asm-scan.ll.gen.py (revision 187) +++ src/parse/asm-scan.ll.gen.py (revision 188) @@ -145,13 +145,13 @@
{id} { - yylval->id = &misc::unique_string::create(yytext); + yylval->id = new misc::unique_string(yytext); return LABEL; } {id}":" { - std::string label = yytext; + std::string label(yytext); label.resize(yyleng - 1); - yylval->id = &misc::unique_string::create(label); + yylval->id = new misc::unique_string(label); return LABEL_DEF; }
Index: src/inst/label_exp.hh --- src/inst/label_exp.hh (revision 187) +++ src/inst/label_exp.hh (revision 188) @@ -49,7 +49,7 @@ virtual void print(std::ostream& ostr) const;
protected: - const misc::unique_string &name_; + const misc::unique_string name_; };
} // namespace inst Index: src/inst/section.hh --- src/inst/section.hh (revision 187) +++ src/inst/section.hh (revision 188) @@ -38,7 +38,7 @@ public: /// Construct an abstract section `\a name' Section(const std::string &name): - name_(misc::unique_string::create(name)) + name_(name) {} virtual ~Section() {} @@ -70,7 +70,7 @@ virtual void print(std::ostream& ostr) const = 0;
protected: - const misc::unique_string &name_; + const misc::unique_string name_;
label_set_type sorted_labels_; }; Index: src/inst/label.hh --- src/inst/label.hh (revision 187) +++ src/inst/label.hh (revision 188) @@ -45,11 +45,11 @@
protected: Label(const std::string &s): - ustr_(misc::unique_string::create(s)) + ustr_(s) {} public: Label(const std::string &s, int offset): - ustr_(misc::unique_string::create(s)), offset_(offset) + ustr_(s), offset_(offset) { lock(); } @@ -81,7 +81,7 @@ }
protected: - const misc::unique_string &ustr_; + const misc::unique_string ustr_; int offset_; };
"Benoît" == Benoît Perrot benoit@lrde.epita.fr writes:
Index: src/vm/virtual_machine.cc --- src/vm/virtual_machine.cc (revision 187) +++ src/vm/virtual_machine.cc (revision 188) @@ -35,7 +35,7 @@ { reset();
- const misc::unique_string &main = misc::unique_string::create("main");
- misc::unique_string main("main"); if (! program.text_section ().has_label(main))
I don't even think you need to create the variable main. The whole point is to be able to write
if (! program.text_section ().has_label("main"))
(unless you need the variable afterward, of course).
Then you should (IMHO) also introduce a conversion operator to std::string.
Index: src/misc/unique_string.hh --- src/misc/unique_string.hh (revision 187) +++ src/misc/unique_string.hh (revision 188) @@ -26,7 +26,7 @@ the implementation is quite different. */
#include <string> -#include <map> +#include <set> #include <iostream>
namespace misc @@ -44,32 +44,28 @@ } };
- typedef std::map<const std::string*, unique_string,
- typedef std::set<const std::string*,
string_ptr_less> string_to_unique_type;
Why a set of pointers instead of a set of strings?
private: static string_to_unique_type pool_;
private:
- const std::string &str_;
- const std::string *str_;
This guy should be a set::const_iterator, no?
{id}":" {
- std::string label = yytext;
- std::string label(yytext); label.resize(yyleng - 1);
- yylval->id = &misc::unique_string::create(label);
- yylval->id = new misc::unique_string(label); return LABEL_DEF;
You don't need to introduce label IMHO.
The following message is a courtesy copy of an article that has been posted to lrde.proj as well.
"Benoît" == Benoît Perrot benoit@lrde.epita.fr writes:
Index: src/vm/virtual_machine.cc --- src/vm/virtual_machine.cc (revision 187) +++ src/vm/virtual_machine.cc (revision 188) @@ -35,7 +35,7 @@ { reset();
- const misc::unique_string &main = misc::unique_string::create("main");
- misc::unique_string main("main"); if (! program.text_section ().has_label(main))
I don't even think you need to create the variable main. The whole point is to be able to write
if (! program.text_section ().has_label("main"))
(unless you need the variable afterward, of course).
Then you should (IMHO) also introduce a conversion operator to std::string.
Index: src/misc/unique_string.hh --- src/misc/unique_string.hh (revision 187) +++ src/misc/unique_string.hh (revision 188) @@ -26,7 +26,7 @@ the implementation is quite different. */
#include <string> -#include <map> +#include <set> #include <iostream>
namespace misc @@ -44,32 +44,28 @@ } };
- typedef std::map<const std::string*, unique_string,
- typedef std::set<const std::string*,
string_ptr_less> string_to_unique_type;
Why a set of pointers instead of a set of strings?
private: static string_to_unique_type pool_;
private:
- const std::string &str_;
- const std::string *str_;
This guy should be a set::const_iterator, no?
{id}":" {
- std::string label = yytext;
- std::string label(yytext); label.resize(yyleng - 1);
- yylval->id = &misc::unique_string::create(label);
- yylval->id = new misc::unique_string(label); return LABEL_DEF;
You don't need to introduce label IMHO.