On 11/02/2017 06:21 PM, David Malcolm wrote: > Jeff: You previously had concerns about the refcounting used in v1 > of this patch; this avoids that in favor of using gnu::unique_ptr. > Joseph already approved the C frontend parts of v2 of this > patch. I had to go back and find my original message to remember what I was concerned about. It was the "delete this" that caught my eye in conjunction with reference counting which brought up issues of ensuring the object was always heap allocated and such.
Otherwise the patch was reasonable. Given you've changed the concerning code to use the blessed gnu::unique_ptr those concerns should be fully addressed now. > > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. > OK for trunk? > > Changed in v3: > - We can't directly include "unique-ptr.h" due to the fix for > PR bootstrap/82610; see: > https://gcc.gnu.org/ml/gcc-patches/2017-10/msg01289.html > The fix is to define INCLUDE_UNIQUE_PTR before including system.h. > This version of the patch moves the usage of gnu::unique_ptr from > c-common.h to a new name-hint.h header, to avoid having to define > INCLUDE_UNIQUE_PTR everywhere that uses c-common.h. > - Updated for *_at_rich_loc renaming > > Changed in v2: > - dropped refcounting in favor of using gnu::unique_ptr. One > wart with this is that the handling of suppressed diagnostics > has to happen in every deferred_diagnostic subclass, rather > than in the name_hint class. It would be possible to fix this > by introducing another dynamically-allocated object to manage > this concern, but adding another dynamic allocation seemed like > overkill. > > Blurb from v1: > > 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 (enum lookup_name_fuzzy_kind): Move to name-hint.h. > (lookup_name_fuzzy): Likewise. Convert return type from > const char * to name_hint. Add location_t param. > * name-hint.h: New header. > > gcc/c/ChangeLog: > * c-decl.c: Define INCLUDE_UNIQUE_PTR before including system.h. > Include "c-family/name-hint.h" > (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: Define INCLUDE_UNIQUE_PTR before including system.h. > Include "c-family/name-hint.h" > (c_parser_declaration_or_fndef): Convert "hint" from > const char * to name_hint. Pass location to lookup_name_fuzzy. > (c_parser_parameter_declaration): Likewise. > > gcc/cp/ChangeLog: > * name-lookup.c: Define INCLUDE_UNIQUE_PTR before including system.h. > Include "c-family/name-hint.h" > (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: Define INCLUDE_UNIQUE_PTR before including system.h. > Include "c-family/name-hint.h" > (cp_parser_diagnose_invalid_type_name): Convert > "suggestion" from const char * to name_hint, and rename to "hint". > Pass location to lookup_name_fuzzy. OK. jeff
