On Mon, 2018-05-14 at 14:51 +0200, Martin Liška wrote: > Second part refactors function from gcc.c into a new class > option_proposer. > > Martin
[...snip...] diff --git a/gcc/c-family/cppspec.c b/gcc/c-family/cppspec.c index 1e0a8bcd294..66540239f53 100644 --- a/gcc/c-family/cppspec.c +++ b/gcc/c-family/cppspec.c @@ -21,6 +21,7 @@ along with GCC; see the file COPYING3. If not see #include "system.h" #include "coretypes.h" #include "tm.h" +#include "opt-suggestions.h" #include "gcc.h" I'm not a reviewer for this, and I don't know what our policy here is, but please can we make "gcc.h" self-contained, and have it include this the header it needs, rather than have every file that includes "gcc.h" need to know what includes gcc.h might need. Seems like a violation of the DRY principle. [...snip more examples of added headers...] [...snip changes to gcc.c and gcc.h which I can't approve but which look sane to me...] [...snip more examples of added headers...] diff --git a/gcc/opt-suggestions.c b/gcc/opt-suggestions.c [...snip...] +option_proposer::~option_proposer () +{ + if (m_option_suggestions) + { + int i; + char *str; + FOR_EACH_VEC_ELT (*m_option_suggestions, i, str) + free (str); + delete m_option_suggestions; + } +} Could m_option_suggestions be a auto_string_vec *m_option_suggestions; rather than a: auto_vec <char *> *m_option_suggestions; ? If so, then this dtor's body could simply be: option_proposer::~option_proposer () { delete m_option_suggestions; } [...snip...] Everything else looks sane to me (I'm not a reviewer, but I wrote much of the code you're touching). Dave