Skip to content

Commit

Permalink
Preprocessor: refactor streambuf management of preprocessor objects
Browse files Browse the repository at this point in the history 
Instead of using this complex system where new processor objects registered themselves as the
current preprocessor object in the base class ctor (this led to freestanding `new` statements
that looked like memory leaks), we now use a deque of unique_ptrs in the streambuf class. This
makes the code a lot clearer since all the memory management is is one place.
  • Loading branch information
@Vultraz
Vultraz committed Aug 29, 2017
1 parent 5ab2ec5 commit 15084ef
Showing 1 changed file with 79 additions and 60 deletions.
139 changes: 79 additions & 60 deletions src/serialization/preprocessor.cpp
View file
Expand Up @@ -30,6 +30,7 @@
#include "wesconfig.h"

#include <stdexcept>
#include <deque>

static lg::log_domain log_preprocessor("preprocessor");
#define ERR_PREPROC LOG_STREAM(err, log_preprocessor)
Expand Down Expand Up @@ -235,11 +236,6 @@ class preprocessor
{
}

preprocessor* get_old_preprocessor()
{
return old_preprocessor_;
}

/**
* Preprocesses and sends some text to the #target_ buffer.
* @return false when the input has no data left.
Expand All @@ -256,8 +252,6 @@ class preprocessor
}

private:
preprocessor* const old_preprocessor_;

std::string old_textdomain_;
std::string old_location_;

Expand All @@ -280,7 +274,7 @@ class preprocessor_streambuf : public streambuf
: streambuf()
, out_buffer_("")
, buffer_()
, current_(nullptr)
, preprocessor_queue_()
, defines_(def)
, default_defines_()
, textdomain_(PACKAGE)
Expand All @@ -297,12 +291,23 @@ class preprocessor_streambuf : public streambuf
void error(const std::string&, int);
void warning(const std::string&, int);

template<typename T, typename... A>
void push_preprocessor(A&&... args)
{
preprocessor_queue_.emplace_back(new T(*this, std::forward<A>(args)...));
}

preprocessor* current() const
{
return preprocessor_queue_.empty() ? nullptr : preprocessor_queue_.back().get();
}

private:
preprocessor_streambuf(const preprocessor_streambuf& t)
: streambuf()
, out_buffer_("")
, buffer_()
, current_(nullptr)
, preprocessor_queue_()
, defines_(t.defines_)
, default_defines_()
, textdomain_(PACKAGE)
Expand All @@ -315,16 +320,16 @@ class preprocessor_streambuf : public streambuf

virtual int underflow();

void restore_old_preprocessor(const preprocessor& current);
void restore_old_preprocessor();

/** Buffer read by the STL stream. */
std::string out_buffer_;

/** Buffer filled by the #current_ preprocessor. */
std::stringstream buffer_;

/** Input preprocessor. */
preprocessor* current_;
/** Input preprocessor queue. */
std::deque<std::unique_ptr<preprocessor>> preprocessor_queue_;

preproc_map* defines_;
preproc_map default_defines_;
Expand All @@ -349,14 +354,12 @@ class preprocessor_streambuf : public streambuf

/** Preprocessor constructor. */
preprocessor::preprocessor(preprocessor_streambuf& t)
: old_preprocessor_(t.current_)
, old_textdomain_(t.textdomain_)
: old_textdomain_(t.textdomain_)
, old_location_(t.location_)
, old_linenum_(t.linenum_)
, target_(t)
{
++target_.depth_;
target_.current_ = this;
}

/**
Expand Down Expand Up @@ -392,17 +395,14 @@ int preprocessor_streambuf::underflow()

const int desired_fill_amount = 2000;

while(current_ && buffer_.rdbuf()->in_avail() < desired_fill_amount) {
while(current() && buffer_.rdbuf()->in_avail() < desired_fill_amount) {
// Process files and data chunks until the desired buffer size is reached
if(!current_->get_chunk()) {
// Delete the current preprocessor item to restore its predecessor
preprocessor* temp = current_;
restore_old_preprocessor(*temp);
delete temp;
if(!current()->get_chunk()) {
// Drop the current preprocessor item from the queue.
restore_old_preprocessor();
}
}


// Update the internal state and data pointers
out_buffer_ = buffer_.str();
if(out_buffer_.empty()) {
Expand All @@ -426,20 +426,26 @@ int preprocessor_streambuf::underflow()
* Appends location and domain directives to the buffer, so that the parser
* notices these changes.
*/
void preprocessor_streambuf::restore_old_preprocessor(const preprocessor& current)
void preprocessor_streambuf::restore_old_preprocessor()
{
if(!current.old_location_.empty()) {
buffer_ << "\376line " << current.old_linenum_ << ' ' << current.old_location_ << '\n';
preprocessor* current = this->current();

if(!current->old_location_.empty()) {
buffer_ << "\376line " << current->old_linenum_ << ' ' << current->old_location_ << '\n';
}

if(!current.old_textdomain_.empty() && textdomain_ != current.old_textdomain_) {
buffer_ << "\376textdomain " << current.old_textdomain_ << '\n';
if(!current->old_textdomain_.empty() && textdomain_ != current->old_textdomain_) {
buffer_ << "\376textdomain " << current->old_textdomain_ << '\n';
}

current_ = current.old_preprocessor_;
location_ = current.old_location_;
linenum_ = current.old_linenum_;
textdomain_ = current.old_textdomain_;
location_ = current->old_location_;
linenum_ = current->old_linenum_;
textdomain_ = current->old_textdomain_;

current = nullptr;

// Drop the preprocessor from the queue.
preprocessor_queue_.pop_back();

--depth_;
}
Expand All @@ -448,14 +454,19 @@ std::string preprocessor_streambuf::get_current_file()
{
unsigned nested_level = 0;

preprocessor* pre = current_;
preprocessor* pre = current();

// Iterate backwards over queue to get the last non-macro preprocessor.
for(auto& p = preprocessor_queue_.rbegin(); p != preprocessor_queue_.rend(); ++p) {
if(!pre || !pre->is_macro()) {
break;
}

while(pre && pre->is_macro()) {
if(pre->is_macro() == 1) {
++nested_level;
}

pre = pre->get_old_preprocessor();
pre = p->get();

This comment has been minimized.

Sign in to view

Copy link
@jyrkive

jyrkive Aug 29, 2017

Member

This loop no longer processes the last preprocessor. In the last iteration it assigns the last preprocessor to pre, then it does ++p and notices that it has reached the end of the deque. And therefore the last preprocessor is never processed.

You need to move this line to the beginning of the loop body.
}

std::string res;
Expand Down Expand Up @@ -533,7 +544,7 @@ void preprocessor_streambuf::warning(const std::string& warning_type, int l)
* A preprocessor_file object is created when a preprocessor encounters an
* inclusion directive that resolves to a file or directory, e.g. '{themes/}'.
*/
class preprocessor_file : preprocessor
class preprocessor_file : public preprocessor
{
public:
/** Constructor. It relies on preprocessor_data so it's implemented after that class is declared. */
Expand All @@ -554,7 +565,7 @@ class preprocessor_file : preprocessor
continue;
}

new preprocessor_file(target_, name);
target_.push_preprocessor<preprocessor_file>(name);
return true;
}

Expand All @@ -575,7 +586,7 @@ class preprocessor_file : preprocessor
* Specialized preprocessor for handling any kind of input stream.
* This is the core of the preprocessor.
*/
class preprocessor_data : preprocessor
class preprocessor_data : public preprocessor
{
/** Description of a preprocessing chunk. */
struct token_desc
Expand Down Expand Up @@ -738,8 +749,10 @@ preprocessor_file::preprocessor_file(preprocessor_streambuf& t, const std::strin
if(!file_stream->good()) {
ERR_PREPROC << "Could not open file " << name << std::endl;
} else {
new preprocessor_data(t, file_stream.release(), "", filesystem::get_short_wml_path(name), 1,
filesystem::directory_name(name), t.textdomain_, nullptr);
t.push_preprocessor<preprocessor_data>(file_stream.release(), "",
filesystem::get_short_wml_path(name), 1,
filesystem::directory_name(name), t.textdomain_, nullptr
);
}
}

Expand Down Expand Up @@ -1439,27 +1452,26 @@ bool preprocessor_data::get_chunk()
if(val.optional_arguments.size() > 0) {
for(const auto& argument : val.optional_arguments) {
if(defines->find(argument.first) == defines->end()) {
std::ostringstream res;
preprocessor_streambuf* buf = new preprocessor_streambuf(target_);
std::unique_ptr<preprocessor_streambuf> buf(new preprocessor_streambuf(target_));

buf->textdomain_ = target_.textdomain_;
std::istream in(buf);
std::istream in(buf.get());

std::istringstream* buffer = new std::istringstream(argument.second);

std::map<std::string, std::string>* temp_defines = new std::map<std::string, std::string>;
temp_defines->insert(defines->begin(), defines->end());

new preprocessor_data(*buf, buffer, val.location, "", val.linenum, dir, val.textdomain,
temp_defines, false);
buf->push_preprocessor<preprocessor_data>(
buffer, val.location, "", val.linenum, dir, val.textdomain, temp_defines, false);

std::ostringstream res;
res << in.rdbuf();

DBG_PREPROC << "Setting default for optional argument " << argument.first << " in macro "
<< symbol << "\n";

(*defines)[argument.first] = res.str();

delete buf;
}
}
}
Expand All @@ -1479,22 +1491,27 @@ bool preprocessor_data::get_chunk()

if(!slowpath_) {
DBG_PREPROC << "substituting macro " << symbol << '\n';
new preprocessor_data(
target_, buffer, val.location, "", val.linenum, dir, val.textdomain, defines, true);

target_.push_preprocessor<preprocessor_data>(
buffer, val.location, "", val.linenum, dir, val.textdomain, defines, true);
} else {
DBG_PREPROC << "substituting (slow) macro " << symbol << '\n';
std::ostringstream res;
preprocessor_streambuf* buf = new preprocessor_streambuf(target_);

std::unique_ptr<preprocessor_streambuf> buf(new preprocessor_streambuf(target_));

// Make the nested preprocessor_data responsible for
// restoring our current textdomain if needed.
buf->textdomain_ = target_.textdomain_;

std::ostringstream res;
{
std::istream in(buf);
new preprocessor_data(
*buf, buffer, val.location, "", val.linenum, dir, val.textdomain, defines, true);
std::istream in(buf.get());
buf->push_preprocessor<preprocessor_data>(
buffer, val.location, "", val.linenum, dir, val.textdomain, defines, true);

res << in.rdbuf();
}
delete buf;

put(res.str());
}
} else if(target_.depth_ < 40) {
Expand All @@ -1506,16 +1523,18 @@ bool preprocessor_data::get_chunk()
if(!slowpath_)
// nfname.size() - symbol.size() gives you an index into nfname
// This does not necessarily match the symbol though, as it can start with ~ or ./
new preprocessor_file(target_, nfname, nfname.size() - symbol.size());
target_.push_preprocessor<preprocessor_file>(nfname, nfname.size() - symbol.size());
else {
std::unique_ptr<preprocessor_streambuf> buf(new preprocessor_streambuf(target_));

std::ostringstream res;
preprocessor_streambuf* buf = new preprocessor_streambuf(target_);
{
std::istream in(buf);
new preprocessor_file(*buf, nfname, nfname.size() - symbol.size());
std::istream in(buf.get());
buf->push_preprocessor<preprocessor_file>(nfname, nfname.size() - symbol.size());

res << in.rdbuf();
}
delete buf;

put(res.str());
}
} else {
Expand Down Expand Up @@ -1592,7 +1611,7 @@ filesystem::scoped_istream preprocess_file(const std::string& fname, preproc_map

preprocessor_streambuf* buf = new preprocessor_streambuf(defines);

new preprocessor_file(*buf, fname);
buf->push_preprocessor<preprocessor_file>(fname);

return filesystem::scoped_istream(new preprocessor_deleter(buf, owned_defines));
}
Expand Down

0 comments on commit 15084ef

Please sign in to comment.