On 05/05/2017 11:51 AM, David Malcolm wrote:
> In various places we use lookup_name_fuzzy to provide a hint,
> and can report messages of the form:
> error: unknown foo named 'bar'
> or:
> error: unknown foo named 'bar'; did you mean 'SUGGESTION?
>
> This patch provides a way for lookup_name_fuzzy to provide
> both the suggestion above, and (optionally) additional hints
> that can be printed e.g.
>
> note: did you forget to include <SOME_HEADER.h>?
>
> This patch provides the mechanism and ports existing users
> of lookup_name_fuzzy to the new return type.
> There are no uses of such hints in this patch, but followup
> patches provide various front-end specific uses of this.
>
> gcc/c-family/ChangeLog:
> * c-common.h (class deferred_diagnostic): New class.
> (class name_hint): New class.
> (lookup_name_fuzzy): Convert return type from const char *
> to name_hint. Add location_t param.
>
> gcc/c/ChangeLog:
> * c-decl.c (implicit_decl_warning): Convert "hint" from
> const char * to name_hint. Pass location to
> lookup_name_fuzzy. Suppress any deferred diagnostic if the
> warning was not printed.
> (undeclared_variable): Likewise for "guessed_id".
> (lookup_name_fuzzy): Convert return type from const char *
> to name_hint. Add location_t param.
> * c-parser.c (c_parser_declaration_or_fndef): Convert "hint" from
> const char * to name_hint. Pass location to lookup_name_fuzzy.
> (c_parser_parameter_declaration): Pass location to
> lookup_name_fuzzy.
>
> gcc/cp/ChangeLog:
> * name-lookup.c (suggest_alternatives_for): Convert "fuzzy_name" from
> const char * to name_hint, and rename to "hint". Pass location to
> lookup_name_fuzzy.
> (lookup_name_fuzzy): Convert return type from const char *
> to name_hint. Add location_t param.
> * parser.c (cp_parser_diagnose_invalid_type_name): Convert
> "suggestion" from const char * to name_hint, and rename to "hint".
> Pass location to lookup_name_fuzzy.
> ---
> gcc/c-family/c-common.h | 121
> +++++++++++++++++++++++++++++++++++++++++++++++-
> gcc/c/c-decl.c | 35 +++++++-------
> gcc/c/c-parser.c | 16 ++++---
> gcc/cp/name-lookup.c | 17 +++----
> gcc/cp/parser.c | 12 ++---
> 5 files changed, 163 insertions(+), 38 deletions(-)
>
> diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
> index 138a0a6..83c1a68 100644
> --- a/gcc/c-family/c-common.h
> +++ b/gcc/c-family/c-common.h
> @@ -1009,7 +1009,126 @@ enum lookup_name_fuzzy_kind {
> /* Any name. */
> FUZZY_LOOKUP_NAME
> };
> -extern const char *lookup_name_fuzzy (tree, enum lookup_name_fuzzy_kind);
> +
> +/* A deferred_diagnostic is a wrapper around optional extra diagnostics
> + that we may want to bundle into a name_hint.
> +
> + The emit method is called when no name_hint instances reference
> + the deferred_diagnostic. In the simple case this is when the name_hint
> + goes out of scope, but a reference-counting scheme is used to allow
> + name_hint instances to be copied. */
> +
> +class deferred_diagnostic
> +{
> + public:
> + virtual ~deferred_diagnostic () {}
> + virtual void emit () = 0;
> +
> + void incref () { m_refcnt++; }
> + void decref ()
> + {
> + if (--m_refcnt == 0)
> + {
> + if (!m_suppress)
> + emit ();
> + delete this;
> + }
> + }
> +
> + location_t get_location () const { return m_loc; }
> +
> + /* Call this if the corresponding warning was not emitted,
> + in which case we should also not emit the deferred_diagnostic. */
> + void suppress ()
> + {
> + m_suppress = true;
> + }
> +
> + protected:
> + deferred_diagnostic (location_t loc)
> + : m_refcnt (0), m_loc (loc), m_suppress (false) {}
> +
> + private:
> + int m_refcnt;
> + location_t m_loc;
> + bool m_suppress;
> +};
So what stands out here is "delete this" and the need for explicit
reference counting. Also doesn't that imply that deferred_diagnostic
objects must be allocated on the heap? Is there another way to get the
behavior you want without resorting to something like this?
Or is your argument that deferred_diagnostic is only used from within
class name_hint and thus the concerns around heap vs stack, explicit
counting, etc are all buried inside the name_hint class? If so, is
there any reasonable way to restrict the use of deferred_disagnostic to
within the name_hint class?
The rest of the changes seem non-controversial, so I think if we can
sort out the issues with those classes then this will be fine to move
forward.
jeff
