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