* C++ PATCH: use identifier_p instead of naked TREE_CODE == IDENTIFIER_NODE
@ 2013-03-22 3:52 Gabriel Dos Reis
2013-03-22 4:27 ` Miles Bader
2013-03-22 9:22 ` Richard Biener
0 siblings, 2 replies; 10+ messages in thread
From: Gabriel Dos Reis @ 2013-03-22 3:52 UTC (permalink / raw)
To: gcc-patches; +Cc: jason
This patch introduces identified_p (t) in lieu of
TREE_CODE (t) == IDENTIFIER_NODE
in the C++ front-end. identifier_p is effectively LANG_IDENTIFIER_CAST
except that it returns a typed pointer instead of a boolean value.
There is NO change in functionality. With this patch, I measured that
bootsrapping takes slightly less time than without, and the object size
of cc1plus decreases by more than 5K.
Bootstrapped and texted on an x86_64-suse-linux.
Applying to trunk. Follow up patches will exploit the typed pointer.
-- Gaby
2013-03-21 Gabriel Dos Reis <gdr@integrable-solutions.net>
* cp-tree.h (identifier_p): New.
* call.c: Throughout, call identifier_p insstead of direct
comparaison of TREE_CODE against IDENTIFIER_NODE.
* decl.c: Likewisse.
* decl2.c: Likewise.
* init.c: Likewise.
* mangle.c: Likewise.
* name-lookup.c: Likewise.
* parser.c: Likewise.
* pt.c: Likewise.
* search.c: Likewise.
* semantics.c: Likewise.
* tree.c: Likewise.
* typeck.c: Likewise.
* typeck2.c: Likewise.
Index: call.c
===================================================================
--- call.c (revision 196891)
+++ call.c (working copy)
@@ -233,7 +233,7 @@
name = TREE_TYPE (name);
else if (TYPE_P (name))
/* OK */;
- else if (TREE_CODE (name) == IDENTIFIER_NODE)
+ else if (identifier_p (name))
{
if ((MAYBE_CLASS_TYPE_P (basetype)
&& name == constructor_name (basetype))
@@ -3147,7 +3147,7 @@
: ACONCAT ((msgstr, " ", NULL)));
location_t cloc = location_of (candidate->fn);
- if (TREE_CODE (candidate->fn) == IDENTIFIER_NODE)
+ if (identifier_p (candidate->fn))
{
cloc = loc;
if (candidate->num_convs == 3)
@@ -8563,8 +8563,7 @@
- do not have the same parameter type list as any non-template
non-member candidate. */
- if (TREE_CODE (cand1->fn) == IDENTIFIER_NODE
- || TREE_CODE (cand2->fn) == IDENTIFIER_NODE)
+ if (identifier_p (cand1->fn) || identifier_p (cand2->fn))
{
for (i = 0; i < len; ++i)
if (!same_type_p (cand1->convs[i]->type,
@@ -8575,7 +8574,7 @@
if (cand1->fn == cand2->fn)
/* Two built-in candidates; arbitrarily pick one. */
return 1;
- else if (TREE_CODE (cand1->fn) == IDENTIFIER_NODE)
+ else if (identifier_p (cand1->fn))
/* cand1 is built-in; prefer cand2. */
return -1;
else
Index: cp-tree.h
===================================================================
--- cp-tree.h (revision 196891)
+++ cp-tree.h (working copy)
@@ -241,6 +241,16 @@
tree label_value;
};
+/* Return a typed pointer version of T if it designates a
+ C++ front-end identifier. */
+inline lang_identifier*
+identifier_p (tree t)
+{
+ if (TREE_CODE (t) == IDENTIFIER_NODE)
+ return (lang_identifier*) t;
+ return NULL;
+}
+
/* In an IDENTIFIER_NODE, nonzero if this identifier is actually a
keyword. C_RID_CODE (node) is then the RID_* value of the keyword,
and C_RID_YYCODE is the token number wanted by Yacc. */
Index: decl.c
===================================================================
--- decl.c (revision 196891)
+++ decl.c (working copy)
@@ -3296,7 +3296,7 @@
error ("%qD used without template parameters", name);
return error_mark_node;
}
- gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
+ gcc_assert (identifier_p (name));
gcc_assert (TYPE_P (context));
if (!MAYBE_CLASS_TYPE_P (context))
@@ -3402,7 +3402,7 @@
name = TYPE_IDENTIFIER (name);
else if (DECL_P (name))
name = DECL_NAME (name);
- gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
+ gcc_assert (identifier_p (name));
if (!dependent_type_p (context)
|| currently_open_class (context))
@@ -4781,7 +4781,7 @@
}
else
{
- gcc_assert (TREE_CODE (ce->index) == IDENTIFIER_NODE);
+ gcc_assert (identifier_p (ce->index));
error ("name %qD used in a GNU-style designated "
"initializer for an array", ce->index);
}
@@ -7413,8 +7413,7 @@
== current_class_type);
fns = TREE_OPERAND (fns, 1);
}
- gcc_assert (TREE_CODE (fns) == IDENTIFIER_NODE
- || TREE_CODE (fns) == OVERLOAD);
+ gcc_assert (identifier_p (fns) || TREE_CODE (fns) == OVERLOAD);
DECL_TEMPLATE_INFO (decl) = build_template_info (fns, args);
for (t = TYPE_ARG_TYPES (TREE_TYPE (decl)); t; t = TREE_CHAIN (t))
@@ -7772,7 +7771,7 @@
tree decl;
tree explicit_scope;
- gcc_assert (!name || TREE_CODE (name) == IDENTIFIER_NODE);
+ gcc_assert (!name || identifier_p (name));
/* Compute the scope in which to place the variable, but remember
whether or not that scope was explicitly specified by the user. */
@@ -8509,7 +8508,7 @@
{
if (!identifier)
error ("unnamed variable or field declared void");
- else if (TREE_CODE (identifier) == IDENTIFIER_NODE)
+ else if (identifier_p (identifier))
{
gcc_assert (!IDENTIFIER_OPNAME_P (identifier));
error ("variable or field %qE declared void", identifier);
@@ -8778,7 +8777,7 @@
tree fns = TREE_OPERAND (decl, 0);
dname = fns;
- if (TREE_CODE (dname) != IDENTIFIER_NODE)
+ if (!identifier_p (dname))
{
gcc_assert (is_overloaded_fn (dname));
dname = DECL_NAME (get_first_fn (dname));
@@ -8787,7 +8786,7 @@
/* Fall through. */
case IDENTIFIER_NODE:
- if (TREE_CODE (decl) == IDENTIFIER_NODE)
+ if (identifier_p (decl))
dname = decl;
if (C_IS_RESERVED_WORD (dname))
@@ -8852,7 +8851,7 @@
}
if (dname
- && TREE_CODE (dname) == IDENTIFIER_NODE
+ && identifier_p (dname)
&& UDLIT_OPER_P (dname)
&& innermost_code != cdk_function)
{
@@ -8977,7 +8976,7 @@
common. With no options, it is allowed. With -Wreturn-type,
it is a warning. It is only an error with -pedantic-errors. */
is_main = (funcdef_flag
- && dname && TREE_CODE (dname) == IDENTIFIER_NODE
+ && dname && identifier_p (dname)
&& MAIN_NAME_P (dname)
&& ctype == NULL_TREE
&& in_namespace == NULL_TREE
@@ -11896,7 +11895,7 @@
tree context = NULL_TREE;
tag_scope scope;
- gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
+ gcc_assert (identifier_p (name));
switch (tag_code)
{
@@ -12323,7 +12322,7 @@
bool scoped_enum_p, bool *is_new)
{
tree prevtype = NULL_TREE;
- gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
+ gcc_assert (identifier_p (name));
if (is_new)
*is_new = false;
Index: decl2.c
===================================================================
--- decl2.c (revision 196891)
+++ decl2.c (working copy)
@@ -1121,7 +1121,7 @@
second and following arguments. Attributes like mode, format,
cleanup and several target specific attributes aren't late
just because they have an IDENTIFIER_NODE as first argument. */
- if (arg == args && TREE_CODE (t) == IDENTIFIER_NODE)
+ if (arg == args && identifier_p (t))
continue;
if (value_dependent_expression_p (t)
Index: init.c
===================================================================
--- init.c (revision 196891)
+++ init.c (working copy)
@@ -1416,7 +1416,7 @@
}
else
{
- if (TREE_CODE (name) == IDENTIFIER_NODE)
+ if (identifier_p (name))
field = lookup_field (current_class_type, name, 1, false);
else
field = name;
Index: mangle.c
===================================================================
--- mangle.c (revision 196891)
+++ mangle.c (working copy)
@@ -1189,7 +1189,7 @@
{
MANGLE_TRACE_TREE ("unqualified-name", decl);
- if (TREE_CODE (decl) == IDENTIFIER_NODE)
+ if (identifier_p (decl))
{
write_unqualified_id (decl);
return;
@@ -2519,7 +2519,7 @@
static void
write_member_name (tree member)
{
- if (TREE_CODE (member) == IDENTIFIER_NODE)
+ if (identifier_p (member))
write_unqualified_id (member);
else if (DECL_P (member))
write_unqualified_name (member);
@@ -2697,7 +2697,7 @@
{
write_expression (TREE_OPERAND (expr, 0));
}
- else if (TREE_CODE (expr) == IDENTIFIER_NODE)
+ else if (identifier_p (expr))
{
/* An operator name appearing as a dependent name needs to be
specially marked to disambiguate between a use of the operator
Index: name-lookup.c
===================================================================
--- name-lookup.c (revision 196891)
+++ name-lookup.c (working copy)
@@ -1959,7 +1959,7 @@
if (!name)
return false;
- if (TREE_CODE (name) != IDENTIFIER_NODE)
+ if (!identifier_p (name))
return false;
/* These don't have names. */
@@ -2073,7 +2073,7 @@
gcc_assert (function && TREE_CODE (function) == FUNCTION_DECL);
name = DECL_NAME (function);
- gcc_assert (name && TREE_CODE (name) == IDENTIFIER_NODE);
+ gcc_assert (name && identifier_p (name));
for (iter = IDENTIFIER_NAMESPACE_BINDINGS (name);
iter;
@@ -2136,7 +2136,7 @@
tree decl;
gcc_assert (TREE_CODE (scope) == NAMESPACE_DECL);
- gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
+ gcc_assert (identifier_p (name));
for (decl = current_binding_level->usings; decl; decl = DECL_CHAIN (decl))
if (USING_DECL_SCOPE (decl) == scope && DECL_NAME (decl) == name)
break;
@@ -5724,7 +5724,7 @@
|| COMPLETE_TYPE_P (b->this_entity))))
b = b->level_chain;
- gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
+ gcc_assert (identifier_p (name));
/* Do C++ gratuitous typedefing. */
if (identifier_type_value_1 (name) != type)
Index: parser.c
===================================================================
--- parser.c (revision 196891)
+++ parser.c (working copy)
@@ -1110,7 +1110,7 @@
case CPP_KEYWORD:
/* Some keywords have a value that is not an IDENTIFIER_NODE.
For example, `struct' is mapped to an INTEGER_CST. */
- if (TREE_CODE (token->u.value) != IDENTIFIER_NODE)
+ if (!identifier_p (token->u.value))
break;
/* else fall through */
case CPP_NAME:
@@ -1259,7 +1259,7 @@
if (qualifying_scope && TYPE_P (qualifying_scope))
qualifying_scope = TYPE_MAIN_VARIANT (qualifying_scope);
- gcc_assert (TREE_CODE (unqualified_name) == IDENTIFIER_NODE
+ gcc_assert (identifier_p (unqualified_name)
|| TREE_CODE (unqualified_name) == BIT_NOT_EXPR
|| TREE_CODE (unqualified_name) == TEMPLATE_ID_EXPR);
@@ -2587,7 +2587,7 @@
{
if (TYPE_P (type))
error_at (location, "%qT is not a template", type);
- else if (TREE_CODE (type) == IDENTIFIER_NODE)
+ else if (identifier_p (type))
{
if (tag_type != none_type)
error_at (location, "%qE is not a class template", type);
@@ -3193,7 +3193,7 @@
tree id, location_t id_location)
{
tree result;
- if (TREE_CODE (id) == IDENTIFIER_NODE)
+ if (identifier_p (id))
{
result = make_typename_type (scope, id, typename_type,
/*complain=*/tf_none);
@@ -5654,7 +5654,7 @@
while (true)
{
if (idk == CP_ID_KIND_UNQUALIFIED
- && TREE_CODE (postfix_expression) == IDENTIFIER_NODE
+ && identifier_p (postfix_expression)
&& cp_lexer_next_token_is_not (parser->lexer, CPP_OPEN_PAREN))
/* It is not a Koenig lookup function call. */
postfix_expression
@@ -5741,7 +5741,7 @@
if (idk == CP_ID_KIND_UNQUALIFIED
|| idk == CP_ID_KIND_TEMPLATE_ID)
{
- if (TREE_CODE (postfix_expression) == IDENTIFIER_NODE)
+ if (identifier_p (postfix_expression))
{
if (!args->is_empty ())
{
@@ -11310,7 +11310,7 @@
cp_id_kind idk;
const char *error_msg;
- if (TREE_CODE (expr) == IDENTIFIER_NODE)
+ if (identifier_p (expr))
/* Lookup the name we got back from the id-expression. */
expr = cp_parser_lookup_name (parser, expr,
none_type,
@@ -12759,7 +12759,7 @@
}
/* Build a representation of the specialization. */
- if (TREE_CODE (templ) == IDENTIFIER_NODE)
+ if (identifier_p (templ))
template_id = build_min_nt_loc (next_token->location,
TEMPLATE_ID_EXPR,
templ, arguments);
@@ -13980,7 +13980,7 @@
&& !global_p
&& !qualified_p
&& TREE_CODE (type) == TYPE_DECL
- && TREE_CODE (DECL_NAME (type)) == IDENTIFIER_NODE)
+ && identifier_p (DECL_NAME (type)))
maybe_note_name_used_in_class (DECL_NAME (type), type);
/* If it didn't work out, we don't have a TYPE. */
if ((flags & CP_PARSER_FLAGS_OPTIONAL)
@@ -15279,7 +15279,7 @@
depending on what scope we are in. */
if (qscope == error_mark_node || identifier == error_mark_node)
;
- else if (TREE_CODE (identifier) != IDENTIFIER_NODE
+ else if (!identifier_p (identifier)
&& TREE_CODE (identifier) != BIT_NOT_EXPR)
/* [namespace.udecl]
@@ -16562,8 +16562,7 @@
unqualified_name = error_mark_node;
else if (unqualified_name
&& (qualifying_scope
- || (TREE_CODE (unqualified_name)
- != IDENTIFIER_NODE)))
+ || (!identifier_p (unqualified_name))))
{
cp_parser_error (parser, "expected unqualified-id");
unqualified_name = error_mark_node;
@@ -18226,7 +18225,7 @@
/* Check to see that it is really the name of a class. */
if (TREE_CODE (decl) == TEMPLATE_ID_EXPR
- && TREE_CODE (TREE_OPERAND (decl, 0)) == IDENTIFIER_NODE
+ && identifier_p (TREE_OPERAND (decl, 0))
&& cp_lexer_next_token_is (parser->lexer, CPP_SCOPE))
/* Situations like this:
@@ -21120,7 +21119,7 @@
/* By this point, the NAME should be an ordinary identifier. If
the id-expression was a qualified name, the qualifying scope is
stored in PARSER->SCOPE at this point. */
- gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
+ gcc_assert (identifier_p (name));
/* Perform the lookup. */
if (parser->scope)
@@ -24290,7 +24289,7 @@
node = token->u.value;
- while (node && TREE_CODE (node) == IDENTIFIER_NODE
+ while (node && identifier_p (node)
&& (node == ridpointers [(int) RID_IN]
|| node == ridpointers [(int) RID_OUT]
|| node == ridpointers [(int) RID_INOUT]
Index: pt.c
===================================================================
--- pt.c (revision 196891)
+++ pt.c (working copy)
@@ -2467,7 +2467,7 @@
{
tree fns;
- gcc_assert (TREE_CODE (declarator) == IDENTIFIER_NODE);
+ gcc_assert (identifier_p (declarator));
if (ctype)
fns = dname;
else
@@ -2528,8 +2528,7 @@
return decl;
}
else if (ctype != NULL_TREE
- && (TREE_CODE (TREE_OPERAND (declarator, 0)) ==
- IDENTIFIER_NODE))
+ && (identifier_p (TREE_OPERAND (declarator, 0))))
{
/* Find the list of functions in ctype that have the same
name as the declared function. */
@@ -6955,7 +6954,7 @@
gcc_assert (!arglist || TREE_CODE (arglist) == TREE_VEC);
- if (!is_overloaded_fn (fns) && TREE_CODE (fns) != IDENTIFIER_NODE)
+ if (!is_overloaded_fn (fns) && !identifier_p (fns))
{
error ("%q#D is not a function template", fns);
return error_mark_node;
@@ -7058,7 +7057,7 @@
spec_entry elt;
hashval_t hash;
- if (TREE_CODE (d1) == IDENTIFIER_NODE)
+ if (identifier_p (d1))
{
tree value = innermost_non_namespace_value (d1);
if (value && DECL_TEMPLATE_TEMPLATE_PARM_P (value))
@@ -7853,7 +7852,7 @@
|| TREE_CODE (t) == TEMPLATE_PARM_INDEX
|| TREE_CODE (t) == OVERLOAD
|| BASELINK_P (t)
- || TREE_CODE (t) == IDENTIFIER_NODE
+ || identifier_p (t)
|| TREE_CODE (t) == TRAIT_EXPR
|| TREE_CODE (t) == CONSTRUCTOR
|| CONSTANT_CLASS_P (t))
@@ -8482,8 +8481,7 @@
if (TREE_VALUE (t)
&& TREE_CODE (TREE_VALUE (t)) == TREE_LIST
&& TREE_VALUE (TREE_VALUE (t))
- && (TREE_CODE (TREE_VALUE (TREE_VALUE (t)))
- == IDENTIFIER_NODE))
+ && (identifier_p (TREE_VALUE (TREE_VALUE (t)))))
{
tree chain
= tsubst_expr (TREE_CHAIN (TREE_VALUE (t)), args, complain,
@@ -13480,7 +13478,7 @@
input_location);
if (error_msg)
error (error_msg);
- if (!function_p && TREE_CODE (decl) == IDENTIFIER_NODE)
+ if (!function_p && identifier_p (decl))
{
if (complain & tf_error)
unqualified_name_lookup_error (decl);
@@ -13907,7 +13905,7 @@
/*done=*/false,
/*address_p=*/false);
}
- else if (koenig_p && TREE_CODE (function) == IDENTIFIER_NODE)
+ else if (koenig_p && identifier_p (function))
{
/* Do nothing; calling tsubst_copy_and_build on an identifier
would incorrectly perform unqualified lookup again.
@@ -13991,7 +13989,7 @@
not appropriate, even if an unqualified-name was used
to denote the function. */
&& !DECL_FUNCTION_MEMBER_P (get_first_fn (function)))
- || TREE_CODE (function) == IDENTIFIER_NODE)
+ || identifier_p (function))
/* Only do this when substitution turns a dependent call
into a non-dependent call. */
&& type_dependent_expression_p_push (t)
@@ -13999,7 +13997,7 @@
function = perform_koenig_lookup (function, call_args, false,
tf_none);
- if (TREE_CODE (function) == IDENTIFIER_NODE
+ if (identifier_p (function)
&& !any_type_dependent_arguments_p (call_args))
{
if (koenig_p && (complain & tf_warning_or_error))
@@ -14049,7 +14047,7 @@
function = unq;
}
}
- if (TREE_CODE (function) == IDENTIFIER_NODE)
+ if (identifier_p (function))
{
if (complain & tf_error)
unqualified_name_lookup_error (function);
@@ -19781,8 +19779,7 @@
return false;
/* An unresolved name is always dependent. */
- if (TREE_CODE (expression) == IDENTIFIER_NODE
- || TREE_CODE (expression) == USING_DECL)
+ if (identifier_p (expression) || TREE_CODE (expression) == USING_DECL)
return true;
/* Some expression forms are never type-dependent. */
@@ -19887,7 +19884,7 @@
if (type_dependent_expression_p (TREE_OPERAND (expression, 0)))
return true;
expression = TREE_OPERAND (expression, 1);
- if (TREE_CODE (expression) == IDENTIFIER_NODE)
+ if (identifier_p (expression))
return false;
}
/* SCOPE_REF with non-null TREE_TYPE is always non-dependent. */
@@ -19978,7 +19975,7 @@
return NULL_TREE;
case COMPONENT_REF:
- if (TREE_CODE (TREE_OPERAND (*tp, 1)) == IDENTIFIER_NODE)
+ if (identifier_p (TREE_OPERAND (*tp, 1)))
/* In a template, finish_class_member_access_expr creates a
COMPONENT_REF with an IDENTIFIER_NODE for op1 even if it isn't
type-dependent, so that we can check access control at
@@ -20222,8 +20219,7 @@
|| TREE_CODE (tmpl) == TEMPLATE_TEMPLATE_PARM)
return true;
/* So are names that have not been looked up. */
- if (TREE_CODE (tmpl) == SCOPE_REF
- || TREE_CODE (tmpl) == IDENTIFIER_NODE)
+ if (TREE_CODE (tmpl) == SCOPE_REF || identifier_p (tmpl))
return true;
/* So are member templates of dependent classes. */
if (TYPE_P (CP_DECL_CONTEXT (tmpl)))
@@ -20380,7 +20376,7 @@
find a TEMPLATE_DECL. Otherwise, we want to find a TYPE_DECL. */
if (!decl)
/*nop*/;
- else if (TREE_CODE (TYPENAME_TYPE_FULLNAME (type)) == IDENTIFIER_NODE
+ else if (identifier_p (TYPENAME_TYPE_FULLNAME (type))
&& TREE_CODE (decl) == TYPE_DECL)
{
result = TREE_TYPE (decl);
Index: search.c
===================================================================
--- search.c (revision 196891)
+++ search.c (working copy)
@@ -381,7 +381,7 @@
{
tree field;
- gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
+ gcc_assert (identifier_p (name));
if (TREE_CODE (type) == TEMPLATE_TYPE_PARM
|| TREE_CODE (type) == BOUND_TEMPLATE_TEMPLATE_PARM
@@ -1190,7 +1190,7 @@
|| xbasetype == error_mark_node)
return NULL_TREE;
- gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
+ gcc_assert (identifier_p (name));
if (TREE_CODE (xbasetype) == TREE_BINFO)
{
Index: semantics.c
===================================================================
--- semantics.c (revision 196891)
+++ semantics.c (working copy)
@@ -536,7 +536,7 @@
tree
finish_goto_stmt (tree destination)
{
- if (TREE_CODE (destination) == IDENTIFIER_NODE)
+ if (identifier_p (destination))
destination = lookup_label (destination);
/* We warn about unused labels with -Wunused. That means we have to
@@ -1999,7 +1999,7 @@
}
/* Find the name of the overloaded function. */
- if (TREE_CODE (fn) == IDENTIFIER_NODE)
+ if (identifier_p (fn))
identifier = fn;
else if (is_overloaded_fn (fn))
{
@@ -2966,7 +2966,7 @@
if (scope
&& (!TYPE_P (scope)
|| (!dependent_type_p (scope)
- && !(TREE_CODE (id_expression) == IDENTIFIER_NODE
+ && !(identifier_p (id_expression)
&& IDENTIFIER_TYPENAME_P (id_expression)
&& dependent_type_p (TREE_TYPE (id_expression))))))
{
@@ -2995,8 +2995,7 @@
the current class so that we can check later to see if
the meaning would have been different after the class
was entirely defined. */
- if (!scope && decl != error_mark_node
- && TREE_CODE (id_expression) == IDENTIFIER_NODE)
+ if (!scope && decl != error_mark_node && identifier_p (id_expression))
maybe_note_name_used_in_class (id_expression, decl);
/* Disallow uses of local variables from containing functions, except
@@ -3169,8 +3168,7 @@
/* A template-id where the name of the template was not resolved
is definitely dependent. */
else if (TREE_CODE (decl) == TEMPLATE_ID_EXPR
- && (TREE_CODE (TREE_OPERAND (decl, 0))
- == IDENTIFIER_NODE))
+ && (identifier_p (TREE_OPERAND (decl, 0))))
dependent_p = true;
/* For anything except an overloaded function, just check its
type. */
@@ -5301,7 +5299,7 @@
[expr.ref]), decltype(e) is defined as the type of the entity
named by e. If there is no such entity, or e names a set of
overloaded functions, the program is ill-formed. */
- if (TREE_CODE (expr) == IDENTIFIER_NODE)
+ if (identifier_p (expr))
expr = lookup_name (expr);
if (TREE_CODE (expr) == INDIRECT_REF)
Index: tree.c
===================================================================
--- tree.c (revision 196891)
+++ tree.c (working copy)
@@ -1734,7 +1734,7 @@
tree
dependent_name (tree x)
{
- if (TREE_CODE (x) == IDENTIFIER_NODE)
+ if (identifier_p (x))
return x;
if (TREE_CODE (x) != COMPONENT_REF
&& TREE_CODE (x) != OFFSET_REF
Index: typeck.c
===================================================================
--- typeck.c (revision 196891)
+++ typeck.c (working copy)
@@ -2467,7 +2467,7 @@
scope, dtor_type);
return error_mark_node;
}
- if (TREE_CODE (dtor_type) == IDENTIFIER_NODE)
+ if (identifier_p (dtor_type))
{
/* In a template, names we can't find a match for are still accepted
destructor names, and we check them here. */
@@ -2588,7 +2588,7 @@
dependent_type_p (object_type)
/* If NAME is just an IDENTIFIER_NODE, then the expression
is dependent. */
- || TREE_CODE (object) == IDENTIFIER_NODE
+ || identifier_p (object)
/* If NAME is "f<args>", where either 'f' or 'args' is
dependent, then the expression is dependent. */
|| (TREE_CODE (name) == TEMPLATE_ID_EXPR
@@ -2604,7 +2604,7 @@
object = build_non_dependent_expr (object);
}
else if (c_dialect_objc ()
- && TREE_CODE (name) == IDENTIFIER_NODE
+ && identifier_p (name)
&& (expr = objc_maybe_build_component_ref (object, name)))
return expr;
@@ -2671,8 +2671,7 @@
}
gcc_assert (CLASS_TYPE_P (scope));
- gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE
- || TREE_CODE (name) == BIT_NOT_EXPR);
+ gcc_assert (identifier_p (name) || TREE_CODE (name) == BIT_NOT_EXPR);
if (constructor_name_p (name, scope))
{
@@ -5067,8 +5066,7 @@
arg = mark_lvalue_use (arg);
argtype = lvalue_type (arg);
- gcc_assert (TREE_CODE (arg) != IDENTIFIER_NODE
- || !IDENTIFIER_OPNAME_P (arg));
+ gcc_assert (!identifier_p (arg) || !IDENTIFIER_OPNAME_P (arg));
if (TREE_CODE (arg) == COMPONENT_REF && type_unknown_p (arg)
&& !really_overloaded_fn (TREE_OPERAND (arg, 1)))
Index: typeck2.c
===================================================================
--- typeck2.c (revision 196891)
+++ typeck2.c (working copy)
@@ -278,8 +278,7 @@
void **slot;
struct pending_abstract_type *pat;
- gcc_assert (!decl || DECL_P (decl)
- || TREE_CODE (decl) == IDENTIFIER_NODE);
+ gcc_assert (!decl || DECL_P (decl) || identifier_p (decl));
if (!abstract_pending_vars)
abstract_pending_vars = htab_create_ggc (31, &pat_calc_hash,
@@ -336,7 +335,7 @@
error ("invalid abstract return type for member function %q+#D", decl);
else if (TREE_CODE (decl) == FUNCTION_DECL)
error ("invalid abstract return type for function %q+#D", decl);
- else if (TREE_CODE (decl) == IDENTIFIER_NODE)
+ else if (identifier_p (decl))
/* Here we do not have location information. */
error ("invalid abstract type %qT for %qE", type, decl);
else
@@ -1241,7 +1240,7 @@
latter case can happen in templates where lookup has to be
deferred. */
gcc_assert (TREE_CODE (ce->index) == FIELD_DECL
- || TREE_CODE (ce->index) == IDENTIFIER_NODE);
+ || identifier_p (ce->index));
if (ce->index != field
&& ce->index != DECL_NAME (field))
{
@@ -1367,7 +1366,7 @@
{
if (TREE_CODE (ce->index) == FIELD_DECL)
;
- else if (TREE_CODE (ce->index) == IDENTIFIER_NODE)
+ else if (identifier_p (ce->index))
{
/* This can happen within a cast, see g++.dg/opt/cse2.C. */
tree name = ce->index;
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: C++ PATCH: use identifier_p instead of naked TREE_CODE == IDENTIFIER_NODE
2013-03-22 3:52 C++ PATCH: use identifier_p instead of naked TREE_CODE == IDENTIFIER_NODE Gabriel Dos Reis
@ 2013-03-22 4:27 ` Miles Bader
2013-03-22 4:36 ` Gabriel Dos Reis
2013-03-22 9:22 ` Richard Biener
1 sibling, 1 reply; 10+ messages in thread
From: Miles Bader @ 2013-03-22 4:27 UTC (permalink / raw)
To: Gabriel Dos Reis; +Cc: gcc-patches, jason
Gabriel Dos Reis <gdr@axiomatics.org> writes:
> in the C++ front-end. identifier_p is effectively LANG_IDENTIFIER_CAST
> except that it returns a typed pointer instead of a boolean value.
What's the point of returning a pointer when the name (and apparently,
use, judging from the patch) suggest a boolean...?
[If it's intended to be used in non-boolean contexts as well, using
"_p" for the name seems vaguely ill-considered...]
-miles
--
Never miss a good chance to shut up. -- Will Rogers
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: C++ PATCH: use identifier_p instead of naked TREE_CODE == IDENTIFIER_NODE
2013-03-22 4:27 ` Miles Bader
@ 2013-03-22 4:36 ` Gabriel Dos Reis
2013-03-22 4:58 ` Miles Bader
0 siblings, 1 reply; 10+ messages in thread
From: Gabriel Dos Reis @ 2013-03-22 4:36 UTC (permalink / raw)
To: Miles Bader; +Cc: Gabriel Dos Reis, gcc-patches, jason
On Thu, Mar 21, 2013 at 11:25 PM, Miles Bader <miles@gnu.org> wrote:
>
> Gabriel Dos Reis <gdr@axiomatics.org> writes:
> > in the C++ front-end. identifier_p is effectively LANG_IDENTIFIER_CAST
> > except that it returns a typed pointer instead of a boolean value.
>
> What's the point of returning a pointer when the name (and apparently,
> use, judging from the patch) suggest a boolean...?
consider it to be a generalized boolean, nothing out of ordinary.
In many places, we do thinks like:
1. test that we have a identifier.
2. immediately follow that with access to parts of the
tree as identifiers, but check again that we really
an identifier, etc.
Something best written as
if (lang_identifier *id = identifier_p (t))
access members of id with no more check please.
There is nothing silly about that.
-- Gaby
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: C++ PATCH: use identifier_p instead of naked TREE_CODE == IDENTIFIER_NODE
2013-03-22 4:36 ` Gabriel Dos Reis
@ 2013-03-22 4:58 ` Miles Bader
0 siblings, 0 replies; 10+ messages in thread
From: Miles Bader @ 2013-03-22 4:58 UTC (permalink / raw)
To: Gabriel Dos Reis; +Cc: Gabriel Dos Reis, gcc-patches, jason
Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
> In many places, we do thinks like:
> 1. test that we have a identifier.
> 2. immediately follow that with access to parts of the
> tree as identifiers, but check again that we really
> an identifier, etc.
>
> There is nothing silly about that.
Sure, it's a common and useful pattern. I'm just saying it's silly to
call it "..._p" in that case...
-miles
--
雨上がり に歩いて、風 柔らかい
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: C++ PATCH: use identifier_p instead of naked TREE_CODE == IDENTIFIER_NODE
2013-03-22 3:52 C++ PATCH: use identifier_p instead of naked TREE_CODE == IDENTIFIER_NODE Gabriel Dos Reis
2013-03-22 4:27 ` Miles Bader
@ 2013-03-22 9:22 ` Richard Biener
2013-03-22 9:31 ` Jakub Jelinek
2013-03-22 12:44 ` Gabriel Dos Reis
1 sibling, 2 replies; 10+ messages in thread
From: Richard Biener @ 2013-03-22 9:22 UTC (permalink / raw)
To: Gabriel Dos Reis; +Cc: gcc-patches, jason
On Fri, Mar 22, 2013 at 4:50 AM, Gabriel Dos Reis <gdr@axiomatics.org> wrote:
>
> This patch introduces identified_p (t) in lieu of
>
> TREE_CODE (t) == IDENTIFIER_NODE
Generally we have macros like IDENTIFIER_P for this kind of checks.
> in the C++ front-end. identifier_p is effectively LANG_IDENTIFIER_CAST
> except that it returns a typed pointer instead of a boolean value.
Hm? So you are replacing TREE_CODE (t) == IDENTIFIER_NODE
with kind-of dynamic_cast<identifier> (t) (in C++ terms)? Then
naming it identifier_p is bad. We have is-a.h now, so please try to use
a single style of C++-style casting throughout GCC.
Thanks,
Richard.
> There is NO change in functionality. With this patch, I measured that
> bootsrapping takes slightly less time than without, and the object size
> of cc1plus decreases by more than 5K.
>
>
> Bootstrapped and texted on an x86_64-suse-linux.
> Applying to trunk. Follow up patches will exploit the typed pointer.
>
> -- Gaby
>
> 2013-03-21 Gabriel Dos Reis <gdr@integrable-solutions.net>
>
> * cp-tree.h (identifier_p): New.
> * call.c: Throughout, call identifier_p insstead of direct
> comparaison of TREE_CODE against IDENTIFIER_NODE.
> * decl.c: Likewisse.
> * decl2.c: Likewise.
> * init.c: Likewise.
> * mangle.c: Likewise.
> * name-lookup.c: Likewise.
> * parser.c: Likewise.
> * pt.c: Likewise.
> * search.c: Likewise.
> * semantics.c: Likewise.
> * tree.c: Likewise.
> * typeck.c: Likewise.
> * typeck2.c: Likewise.
>
> Index: call.c
> ===================================================================
> --- call.c (revision 196891)
> +++ call.c (working copy)
> @@ -233,7 +233,7 @@
> name = TREE_TYPE (name);
> else if (TYPE_P (name))
> /* OK */;
> - else if (TREE_CODE (name) == IDENTIFIER_NODE)
> + else if (identifier_p (name))
> {
> if ((MAYBE_CLASS_TYPE_P (basetype)
> && name == constructor_name (basetype))
> @@ -3147,7 +3147,7 @@
> : ACONCAT ((msgstr, " ", NULL)));
> location_t cloc = location_of (candidate->fn);
>
> - if (TREE_CODE (candidate->fn) == IDENTIFIER_NODE)
> + if (identifier_p (candidate->fn))
> {
> cloc = loc;
> if (candidate->num_convs == 3)
> @@ -8563,8 +8563,7 @@
> - do not have the same parameter type list as any non-template
> non-member candidate. */
>
> - if (TREE_CODE (cand1->fn) == IDENTIFIER_NODE
> - || TREE_CODE (cand2->fn) == IDENTIFIER_NODE)
> + if (identifier_p (cand1->fn) || identifier_p (cand2->fn))
> {
> for (i = 0; i < len; ++i)
> if (!same_type_p (cand1->convs[i]->type,
> @@ -8575,7 +8574,7 @@
> if (cand1->fn == cand2->fn)
> /* Two built-in candidates; arbitrarily pick one. */
> return 1;
> - else if (TREE_CODE (cand1->fn) == IDENTIFIER_NODE)
> + else if (identifier_p (cand1->fn))
> /* cand1 is built-in; prefer cand2. */
> return -1;
> else
> Index: cp-tree.h
> ===================================================================
> --- cp-tree.h (revision 196891)
> +++ cp-tree.h (working copy)
> @@ -241,6 +241,16 @@
> tree label_value;
> };
>
> +/* Return a typed pointer version of T if it designates a
> + C++ front-end identifier. */
> +inline lang_identifier*
> +identifier_p (tree t)
> +{
> + if (TREE_CODE (t) == IDENTIFIER_NODE)
> + return (lang_identifier*) t;
> + return NULL;
> +}
> +
> /* In an IDENTIFIER_NODE, nonzero if this identifier is actually a
> keyword. C_RID_CODE (node) is then the RID_* value of the keyword,
> and C_RID_YYCODE is the token number wanted by Yacc. */
> Index: decl.c
> ===================================================================
> --- decl.c (revision 196891)
> +++ decl.c (working copy)
> @@ -3296,7 +3296,7 @@
> error ("%qD used without template parameters", name);
> return error_mark_node;
> }
> - gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
> + gcc_assert (identifier_p (name));
> gcc_assert (TYPE_P (context));
>
> if (!MAYBE_CLASS_TYPE_P (context))
> @@ -3402,7 +3402,7 @@
> name = TYPE_IDENTIFIER (name);
> else if (DECL_P (name))
> name = DECL_NAME (name);
> - gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
> + gcc_assert (identifier_p (name));
>
> if (!dependent_type_p (context)
> || currently_open_class (context))
> @@ -4781,7 +4781,7 @@
> }
> else
> {
> - gcc_assert (TREE_CODE (ce->index) == IDENTIFIER_NODE);
> + gcc_assert (identifier_p (ce->index));
> error ("name %qD used in a GNU-style designated "
> "initializer for an array", ce->index);
> }
> @@ -7413,8 +7413,7 @@
> == current_class_type);
> fns = TREE_OPERAND (fns, 1);
> }
> - gcc_assert (TREE_CODE (fns) == IDENTIFIER_NODE
> - || TREE_CODE (fns) == OVERLOAD);
> + gcc_assert (identifier_p (fns) || TREE_CODE (fns) == OVERLOAD);
> DECL_TEMPLATE_INFO (decl) = build_template_info (fns, args);
>
> for (t = TYPE_ARG_TYPES (TREE_TYPE (decl)); t; t = TREE_CHAIN (t))
> @@ -7772,7 +7771,7 @@
> tree decl;
> tree explicit_scope;
>
> - gcc_assert (!name || TREE_CODE (name) == IDENTIFIER_NODE);
> + gcc_assert (!name || identifier_p (name));
>
> /* Compute the scope in which to place the variable, but remember
> whether or not that scope was explicitly specified by the user. */
> @@ -8509,7 +8508,7 @@
> {
> if (!identifier)
> error ("unnamed variable or field declared void");
> - else if (TREE_CODE (identifier) == IDENTIFIER_NODE)
> + else if (identifier_p (identifier))
> {
> gcc_assert (!IDENTIFIER_OPNAME_P (identifier));
> error ("variable or field %qE declared void", identifier);
> @@ -8778,7 +8777,7 @@
> tree fns = TREE_OPERAND (decl, 0);
>
> dname = fns;
> - if (TREE_CODE (dname) != IDENTIFIER_NODE)
> + if (!identifier_p (dname))
> {
> gcc_assert (is_overloaded_fn (dname));
> dname = DECL_NAME (get_first_fn (dname));
> @@ -8787,7 +8786,7 @@
> /* Fall through. */
>
> case IDENTIFIER_NODE:
> - if (TREE_CODE (decl) == IDENTIFIER_NODE)
> + if (identifier_p (decl))
> dname = decl;
>
> if (C_IS_RESERVED_WORD (dname))
> @@ -8852,7 +8851,7 @@
> }
>
> if (dname
> - && TREE_CODE (dname) == IDENTIFIER_NODE
> + && identifier_p (dname)
> && UDLIT_OPER_P (dname)
> && innermost_code != cdk_function)
> {
> @@ -8977,7 +8976,7 @@
> common. With no options, it is allowed. With -Wreturn-type,
> it is a warning. It is only an error with -pedantic-errors. */
> is_main = (funcdef_flag
> - && dname && TREE_CODE (dname) == IDENTIFIER_NODE
> + && dname && identifier_p (dname)
> && MAIN_NAME_P (dname)
> && ctype == NULL_TREE
> && in_namespace == NULL_TREE
> @@ -11896,7 +11895,7 @@
> tree context = NULL_TREE;
> tag_scope scope;
>
> - gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
> + gcc_assert (identifier_p (name));
>
> switch (tag_code)
> {
> @@ -12323,7 +12322,7 @@
> bool scoped_enum_p, bool *is_new)
> {
> tree prevtype = NULL_TREE;
> - gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
> + gcc_assert (identifier_p (name));
>
> if (is_new)
> *is_new = false;
> Index: decl2.c
> ===================================================================
> --- decl2.c (revision 196891)
> +++ decl2.c (working copy)
> @@ -1121,7 +1121,7 @@
> second and following arguments. Attributes like mode, format,
> cleanup and several target specific attributes aren't late
> just because they have an IDENTIFIER_NODE as first argument. */
> - if (arg == args && TREE_CODE (t) == IDENTIFIER_NODE)
> + if (arg == args && identifier_p (t))
> continue;
>
> if (value_dependent_expression_p (t)
> Index: init.c
> ===================================================================
> --- init.c (revision 196891)
> +++ init.c (working copy)
> @@ -1416,7 +1416,7 @@
> }
> else
> {
> - if (TREE_CODE (name) == IDENTIFIER_NODE)
> + if (identifier_p (name))
> field = lookup_field (current_class_type, name, 1, false);
> else
> field = name;
> Index: mangle.c
> ===================================================================
> --- mangle.c (revision 196891)
> +++ mangle.c (working copy)
> @@ -1189,7 +1189,7 @@
> {
> MANGLE_TRACE_TREE ("unqualified-name", decl);
>
> - if (TREE_CODE (decl) == IDENTIFIER_NODE)
> + if (identifier_p (decl))
> {
> write_unqualified_id (decl);
> return;
> @@ -2519,7 +2519,7 @@
> static void
> write_member_name (tree member)
> {
> - if (TREE_CODE (member) == IDENTIFIER_NODE)
> + if (identifier_p (member))
> write_unqualified_id (member);
> else if (DECL_P (member))
> write_unqualified_name (member);
> @@ -2697,7 +2697,7 @@
> {
> write_expression (TREE_OPERAND (expr, 0));
> }
> - else if (TREE_CODE (expr) == IDENTIFIER_NODE)
> + else if (identifier_p (expr))
> {
> /* An operator name appearing as a dependent name needs to be
> specially marked to disambiguate between a use of the operator
> Index: name-lookup.c
> ===================================================================
> --- name-lookup.c (revision 196891)
> +++ name-lookup.c (working copy)
> @@ -1959,7 +1959,7 @@
> if (!name)
> return false;
>
> - if (TREE_CODE (name) != IDENTIFIER_NODE)
> + if (!identifier_p (name))
> return false;
>
> /* These don't have names. */
> @@ -2073,7 +2073,7 @@
> gcc_assert (function && TREE_CODE (function) == FUNCTION_DECL);
>
> name = DECL_NAME (function);
> - gcc_assert (name && TREE_CODE (name) == IDENTIFIER_NODE);
> + gcc_assert (name && identifier_p (name));
>
> for (iter = IDENTIFIER_NAMESPACE_BINDINGS (name);
> iter;
> @@ -2136,7 +2136,7 @@
> tree decl;
>
> gcc_assert (TREE_CODE (scope) == NAMESPACE_DECL);
> - gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
> + gcc_assert (identifier_p (name));
> for (decl = current_binding_level->usings; decl; decl = DECL_CHAIN (decl))
> if (USING_DECL_SCOPE (decl) == scope && DECL_NAME (decl) == name)
> break;
> @@ -5724,7 +5724,7 @@
> || COMPLETE_TYPE_P (b->this_entity))))
> b = b->level_chain;
>
> - gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
> + gcc_assert (identifier_p (name));
>
> /* Do C++ gratuitous typedefing. */
> if (identifier_type_value_1 (name) != type)
> Index: parser.c
> ===================================================================
> --- parser.c (revision 196891)
> +++ parser.c (working copy)
> @@ -1110,7 +1110,7 @@
> case CPP_KEYWORD:
> /* Some keywords have a value that is not an IDENTIFIER_NODE.
> For example, `struct' is mapped to an INTEGER_CST. */
> - if (TREE_CODE (token->u.value) != IDENTIFIER_NODE)
> + if (!identifier_p (token->u.value))
> break;
> /* else fall through */
> case CPP_NAME:
> @@ -1259,7 +1259,7 @@
> if (qualifying_scope && TYPE_P (qualifying_scope))
> qualifying_scope = TYPE_MAIN_VARIANT (qualifying_scope);
>
> - gcc_assert (TREE_CODE (unqualified_name) == IDENTIFIER_NODE
> + gcc_assert (identifier_p (unqualified_name)
> || TREE_CODE (unqualified_name) == BIT_NOT_EXPR
> || TREE_CODE (unqualified_name) == TEMPLATE_ID_EXPR);
>
> @@ -2587,7 +2587,7 @@
> {
> if (TYPE_P (type))
> error_at (location, "%qT is not a template", type);
> - else if (TREE_CODE (type) == IDENTIFIER_NODE)
> + else if (identifier_p (type))
> {
> if (tag_type != none_type)
> error_at (location, "%qE is not a class template", type);
> @@ -3193,7 +3193,7 @@
> tree id, location_t id_location)
> {
> tree result;
> - if (TREE_CODE (id) == IDENTIFIER_NODE)
> + if (identifier_p (id))
> {
> result = make_typename_type (scope, id, typename_type,
> /*complain=*/tf_none);
> @@ -5654,7 +5654,7 @@
> while (true)
> {
> if (idk == CP_ID_KIND_UNQUALIFIED
> - && TREE_CODE (postfix_expression) == IDENTIFIER_NODE
> + && identifier_p (postfix_expression)
> && cp_lexer_next_token_is_not (parser->lexer, CPP_OPEN_PAREN))
> /* It is not a Koenig lookup function call. */
> postfix_expression
> @@ -5741,7 +5741,7 @@
> if (idk == CP_ID_KIND_UNQUALIFIED
> || idk == CP_ID_KIND_TEMPLATE_ID)
> {
> - if (TREE_CODE (postfix_expression) == IDENTIFIER_NODE)
> + if (identifier_p (postfix_expression))
> {
> if (!args->is_empty ())
> {
> @@ -11310,7 +11310,7 @@
> cp_id_kind idk;
> const char *error_msg;
>
> - if (TREE_CODE (expr) == IDENTIFIER_NODE)
> + if (identifier_p (expr))
> /* Lookup the name we got back from the id-expression. */
> expr = cp_parser_lookup_name (parser, expr,
> none_type,
> @@ -12759,7 +12759,7 @@
> }
>
> /* Build a representation of the specialization. */
> - if (TREE_CODE (templ) == IDENTIFIER_NODE)
> + if (identifier_p (templ))
> template_id = build_min_nt_loc (next_token->location,
> TEMPLATE_ID_EXPR,
> templ, arguments);
> @@ -13980,7 +13980,7 @@
> && !global_p
> && !qualified_p
> && TREE_CODE (type) == TYPE_DECL
> - && TREE_CODE (DECL_NAME (type)) == IDENTIFIER_NODE)
> + && identifier_p (DECL_NAME (type)))
> maybe_note_name_used_in_class (DECL_NAME (type), type);
> /* If it didn't work out, we don't have a TYPE. */
> if ((flags & CP_PARSER_FLAGS_OPTIONAL)
> @@ -15279,7 +15279,7 @@
> depending on what scope we are in. */
> if (qscope == error_mark_node || identifier == error_mark_node)
> ;
> - else if (TREE_CODE (identifier) != IDENTIFIER_NODE
> + else if (!identifier_p (identifier)
> && TREE_CODE (identifier) != BIT_NOT_EXPR)
> /* [namespace.udecl]
>
> @@ -16562,8 +16562,7 @@
> unqualified_name = error_mark_node;
> else if (unqualified_name
> && (qualifying_scope
> - || (TREE_CODE (unqualified_name)
> - != IDENTIFIER_NODE)))
> + || (!identifier_p (unqualified_name))))
> {
> cp_parser_error (parser, "expected unqualified-id");
> unqualified_name = error_mark_node;
> @@ -18226,7 +18225,7 @@
>
> /* Check to see that it is really the name of a class. */
> if (TREE_CODE (decl) == TEMPLATE_ID_EXPR
> - && TREE_CODE (TREE_OPERAND (decl, 0)) == IDENTIFIER_NODE
> + && identifier_p (TREE_OPERAND (decl, 0))
> && cp_lexer_next_token_is (parser->lexer, CPP_SCOPE))
> /* Situations like this:
>
> @@ -21120,7 +21119,7 @@
> /* By this point, the NAME should be an ordinary identifier. If
> the id-expression was a qualified name, the qualifying scope is
> stored in PARSER->SCOPE at this point. */
> - gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
> + gcc_assert (identifier_p (name));
>
> /* Perform the lookup. */
> if (parser->scope)
> @@ -24290,7 +24289,7 @@
>
> node = token->u.value;
>
> - while (node && TREE_CODE (node) == IDENTIFIER_NODE
> + while (node && identifier_p (node)
> && (node == ridpointers [(int) RID_IN]
> || node == ridpointers [(int) RID_OUT]
> || node == ridpointers [(int) RID_INOUT]
> Index: pt.c
> ===================================================================
> --- pt.c (revision 196891)
> +++ pt.c (working copy)
> @@ -2467,7 +2467,7 @@
> {
> tree fns;
>
> - gcc_assert (TREE_CODE (declarator) == IDENTIFIER_NODE);
> + gcc_assert (identifier_p (declarator));
> if (ctype)
> fns = dname;
> else
> @@ -2528,8 +2528,7 @@
> return decl;
> }
> else if (ctype != NULL_TREE
> - && (TREE_CODE (TREE_OPERAND (declarator, 0)) ==
> - IDENTIFIER_NODE))
> + && (identifier_p (TREE_OPERAND (declarator, 0))))
> {
> /* Find the list of functions in ctype that have the same
> name as the declared function. */
> @@ -6955,7 +6954,7 @@
>
> gcc_assert (!arglist || TREE_CODE (arglist) == TREE_VEC);
>
> - if (!is_overloaded_fn (fns) && TREE_CODE (fns) != IDENTIFIER_NODE)
> + if (!is_overloaded_fn (fns) && !identifier_p (fns))
> {
> error ("%q#D is not a function template", fns);
> return error_mark_node;
> @@ -7058,7 +7057,7 @@
> spec_entry elt;
> hashval_t hash;
>
> - if (TREE_CODE (d1) == IDENTIFIER_NODE)
> + if (identifier_p (d1))
> {
> tree value = innermost_non_namespace_value (d1);
> if (value && DECL_TEMPLATE_TEMPLATE_PARM_P (value))
> @@ -7853,7 +7852,7 @@
> || TREE_CODE (t) == TEMPLATE_PARM_INDEX
> || TREE_CODE (t) == OVERLOAD
> || BASELINK_P (t)
> - || TREE_CODE (t) == IDENTIFIER_NODE
> + || identifier_p (t)
> || TREE_CODE (t) == TRAIT_EXPR
> || TREE_CODE (t) == CONSTRUCTOR
> || CONSTANT_CLASS_P (t))
> @@ -8482,8 +8481,7 @@
> if (TREE_VALUE (t)
> && TREE_CODE (TREE_VALUE (t)) == TREE_LIST
> && TREE_VALUE (TREE_VALUE (t))
> - && (TREE_CODE (TREE_VALUE (TREE_VALUE (t)))
> - == IDENTIFIER_NODE))
> + && (identifier_p (TREE_VALUE (TREE_VALUE (t)))))
> {
> tree chain
> = tsubst_expr (TREE_CHAIN (TREE_VALUE (t)), args, complain,
> @@ -13480,7 +13478,7 @@
> input_location);
> if (error_msg)
> error (error_msg);
> - if (!function_p && TREE_CODE (decl) == IDENTIFIER_NODE)
> + if (!function_p && identifier_p (decl))
> {
> if (complain & tf_error)
> unqualified_name_lookup_error (decl);
> @@ -13907,7 +13905,7 @@
> /*done=*/false,
> /*address_p=*/false);
> }
> - else if (koenig_p && TREE_CODE (function) == IDENTIFIER_NODE)
> + else if (koenig_p && identifier_p (function))
> {
> /* Do nothing; calling tsubst_copy_and_build on an identifier
> would incorrectly perform unqualified lookup again.
> @@ -13991,7 +13989,7 @@
> not appropriate, even if an unqualified-name was used
> to denote the function. */
> && !DECL_FUNCTION_MEMBER_P (get_first_fn (function)))
> - || TREE_CODE (function) == IDENTIFIER_NODE)
> + || identifier_p (function))
> /* Only do this when substitution turns a dependent call
> into a non-dependent call. */
> && type_dependent_expression_p_push (t)
> @@ -13999,7 +13997,7 @@
> function = perform_koenig_lookup (function, call_args, false,
> tf_none);
>
> - if (TREE_CODE (function) == IDENTIFIER_NODE
> + if (identifier_p (function)
> && !any_type_dependent_arguments_p (call_args))
> {
> if (koenig_p && (complain & tf_warning_or_error))
> @@ -14049,7 +14047,7 @@
> function = unq;
> }
> }
> - if (TREE_CODE (function) == IDENTIFIER_NODE)
> + if (identifier_p (function))
> {
> if (complain & tf_error)
> unqualified_name_lookup_error (function);
> @@ -19781,8 +19779,7 @@
> return false;
>
> /* An unresolved name is always dependent. */
> - if (TREE_CODE (expression) == IDENTIFIER_NODE
> - || TREE_CODE (expression) == USING_DECL)
> + if (identifier_p (expression) || TREE_CODE (expression) == USING_DECL)
> return true;
>
> /* Some expression forms are never type-dependent. */
> @@ -19887,7 +19884,7 @@
> if (type_dependent_expression_p (TREE_OPERAND (expression, 0)))
> return true;
> expression = TREE_OPERAND (expression, 1);
> - if (TREE_CODE (expression) == IDENTIFIER_NODE)
> + if (identifier_p (expression))
> return false;
> }
> /* SCOPE_REF with non-null TREE_TYPE is always non-dependent. */
> @@ -19978,7 +19975,7 @@
> return NULL_TREE;
>
> case COMPONENT_REF:
> - if (TREE_CODE (TREE_OPERAND (*tp, 1)) == IDENTIFIER_NODE)
> + if (identifier_p (TREE_OPERAND (*tp, 1)))
> /* In a template, finish_class_member_access_expr creates a
> COMPONENT_REF with an IDENTIFIER_NODE for op1 even if it isn't
> type-dependent, so that we can check access control at
> @@ -20222,8 +20219,7 @@
> || TREE_CODE (tmpl) == TEMPLATE_TEMPLATE_PARM)
> return true;
> /* So are names that have not been looked up. */
> - if (TREE_CODE (tmpl) == SCOPE_REF
> - || TREE_CODE (tmpl) == IDENTIFIER_NODE)
> + if (TREE_CODE (tmpl) == SCOPE_REF || identifier_p (tmpl))
> return true;
> /* So are member templates of dependent classes. */
> if (TYPE_P (CP_DECL_CONTEXT (tmpl)))
> @@ -20380,7 +20376,7 @@
> find a TEMPLATE_DECL. Otherwise, we want to find a TYPE_DECL. */
> if (!decl)
> /*nop*/;
> - else if (TREE_CODE (TYPENAME_TYPE_FULLNAME (type)) == IDENTIFIER_NODE
> + else if (identifier_p (TYPENAME_TYPE_FULLNAME (type))
> && TREE_CODE (decl) == TYPE_DECL)
> {
> result = TREE_TYPE (decl);
> Index: search.c
> ===================================================================
> --- search.c (revision 196891)
> +++ search.c (working copy)
> @@ -381,7 +381,7 @@
> {
> tree field;
>
> - gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
> + gcc_assert (identifier_p (name));
>
> if (TREE_CODE (type) == TEMPLATE_TYPE_PARM
> || TREE_CODE (type) == BOUND_TEMPLATE_TEMPLATE_PARM
> @@ -1190,7 +1190,7 @@
> || xbasetype == error_mark_node)
> return NULL_TREE;
>
> - gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
> + gcc_assert (identifier_p (name));
>
> if (TREE_CODE (xbasetype) == TREE_BINFO)
> {
> Index: semantics.c
> ===================================================================
> --- semantics.c (revision 196891)
> +++ semantics.c (working copy)
> @@ -536,7 +536,7 @@
> tree
> finish_goto_stmt (tree destination)
> {
> - if (TREE_CODE (destination) == IDENTIFIER_NODE)
> + if (identifier_p (destination))
> destination = lookup_label (destination);
>
> /* We warn about unused labels with -Wunused. That means we have to
> @@ -1999,7 +1999,7 @@
> }
>
> /* Find the name of the overloaded function. */
> - if (TREE_CODE (fn) == IDENTIFIER_NODE)
> + if (identifier_p (fn))
> identifier = fn;
> else if (is_overloaded_fn (fn))
> {
> @@ -2966,7 +2966,7 @@
> if (scope
> && (!TYPE_P (scope)
> || (!dependent_type_p (scope)
> - && !(TREE_CODE (id_expression) == IDENTIFIER_NODE
> + && !(identifier_p (id_expression)
> && IDENTIFIER_TYPENAME_P (id_expression)
> && dependent_type_p (TREE_TYPE (id_expression))))))
> {
> @@ -2995,8 +2995,7 @@
> the current class so that we can check later to see if
> the meaning would have been different after the class
> was entirely defined. */
> - if (!scope && decl != error_mark_node
> - && TREE_CODE (id_expression) == IDENTIFIER_NODE)
> + if (!scope && decl != error_mark_node && identifier_p (id_expression))
> maybe_note_name_used_in_class (id_expression, decl);
>
> /* Disallow uses of local variables from containing functions, except
> @@ -3169,8 +3168,7 @@
> /* A template-id where the name of the template was not resolved
> is definitely dependent. */
> else if (TREE_CODE (decl) == TEMPLATE_ID_EXPR
> - && (TREE_CODE (TREE_OPERAND (decl, 0))
> - == IDENTIFIER_NODE))
> + && (identifier_p (TREE_OPERAND (decl, 0))))
> dependent_p = true;
> /* For anything except an overloaded function, just check its
> type. */
> @@ -5301,7 +5299,7 @@
> [expr.ref]), decltype(e) is defined as the type of the entity
> named by e. If there is no such entity, or e names a set of
> overloaded functions, the program is ill-formed. */
> - if (TREE_CODE (expr) == IDENTIFIER_NODE)
> + if (identifier_p (expr))
> expr = lookup_name (expr);
>
> if (TREE_CODE (expr) == INDIRECT_REF)
> Index: tree.c
> ===================================================================
> --- tree.c (revision 196891)
> +++ tree.c (working copy)
> @@ -1734,7 +1734,7 @@
> tree
> dependent_name (tree x)
> {
> - if (TREE_CODE (x) == IDENTIFIER_NODE)
> + if (identifier_p (x))
> return x;
> if (TREE_CODE (x) != COMPONENT_REF
> && TREE_CODE (x) != OFFSET_REF
> Index: typeck.c
> ===================================================================
> --- typeck.c (revision 196891)
> +++ typeck.c (working copy)
> @@ -2467,7 +2467,7 @@
> scope, dtor_type);
> return error_mark_node;
> }
> - if (TREE_CODE (dtor_type) == IDENTIFIER_NODE)
> + if (identifier_p (dtor_type))
> {
> /* In a template, names we can't find a match for are still accepted
> destructor names, and we check them here. */
> @@ -2588,7 +2588,7 @@
> dependent_type_p (object_type)
> /* If NAME is just an IDENTIFIER_NODE, then the expression
> is dependent. */
> - || TREE_CODE (object) == IDENTIFIER_NODE
> + || identifier_p (object)
> /* If NAME is "f<args>", where either 'f' or 'args' is
> dependent, then the expression is dependent. */
> || (TREE_CODE (name) == TEMPLATE_ID_EXPR
> @@ -2604,7 +2604,7 @@
> object = build_non_dependent_expr (object);
> }
> else if (c_dialect_objc ()
> - && TREE_CODE (name) == IDENTIFIER_NODE
> + && identifier_p (name)
> && (expr = objc_maybe_build_component_ref (object, name)))
> return expr;
>
> @@ -2671,8 +2671,7 @@
> }
>
> gcc_assert (CLASS_TYPE_P (scope));
> - gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE
> - || TREE_CODE (name) == BIT_NOT_EXPR);
> + gcc_assert (identifier_p (name) || TREE_CODE (name) == BIT_NOT_EXPR);
>
> if (constructor_name_p (name, scope))
> {
> @@ -5067,8 +5066,7 @@
> arg = mark_lvalue_use (arg);
> argtype = lvalue_type (arg);
>
> - gcc_assert (TREE_CODE (arg) != IDENTIFIER_NODE
> - || !IDENTIFIER_OPNAME_P (arg));
> + gcc_assert (!identifier_p (arg) || !IDENTIFIER_OPNAME_P (arg));
>
> if (TREE_CODE (arg) == COMPONENT_REF && type_unknown_p (arg)
> && !really_overloaded_fn (TREE_OPERAND (arg, 1)))
> Index: typeck2.c
> ===================================================================
> --- typeck2.c (revision 196891)
> +++ typeck2.c (working copy)
> @@ -278,8 +278,7 @@
> void **slot;
> struct pending_abstract_type *pat;
>
> - gcc_assert (!decl || DECL_P (decl)
> - || TREE_CODE (decl) == IDENTIFIER_NODE);
> + gcc_assert (!decl || DECL_P (decl) || identifier_p (decl));
>
> if (!abstract_pending_vars)
> abstract_pending_vars = htab_create_ggc (31, &pat_calc_hash,
> @@ -336,7 +335,7 @@
> error ("invalid abstract return type for member function %q+#D", decl);
> else if (TREE_CODE (decl) == FUNCTION_DECL)
> error ("invalid abstract return type for function %q+#D", decl);
> - else if (TREE_CODE (decl) == IDENTIFIER_NODE)
> + else if (identifier_p (decl))
> /* Here we do not have location information. */
> error ("invalid abstract type %qT for %qE", type, decl);
> else
> @@ -1241,7 +1240,7 @@
> latter case can happen in templates where lookup has to be
> deferred. */
> gcc_assert (TREE_CODE (ce->index) == FIELD_DECL
> - || TREE_CODE (ce->index) == IDENTIFIER_NODE);
> + || identifier_p (ce->index));
> if (ce->index != field
> && ce->index != DECL_NAME (field))
> {
> @@ -1367,7 +1366,7 @@
> {
> if (TREE_CODE (ce->index) == FIELD_DECL)
> ;
> - else if (TREE_CODE (ce->index) == IDENTIFIER_NODE)
> + else if (identifier_p (ce->index))
> {
> /* This can happen within a cast, see g++.dg/opt/cse2.C. */
> tree name = ce->index;
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: C++ PATCH: use identifier_p instead of naked TREE_CODE == IDENTIFIER_NODE
2013-03-22 9:22 ` Richard Biener
@ 2013-03-22 9:31 ` Jakub Jelinek
2013-03-22 12:37 ` Gabriel Dos Reis
2013-03-22 12:44 ` Gabriel Dos Reis
1 sibling, 1 reply; 10+ messages in thread
From: Jakub Jelinek @ 2013-03-22 9:31 UTC (permalink / raw)
To: Richard Biener; +Cc: Gabriel Dos Reis, gcc-patches, jason
On Fri, Mar 22, 2013 at 10:21:05AM +0100, Richard Biener wrote:
> On Fri, Mar 22, 2013 at 4:50 AM, Gabriel Dos Reis <gdr@axiomatics.org> wrote:
> >
> > This patch introduces identified_p (t) in lieu of
> >
> > TREE_CODE (t) == IDENTIFIER_NODE
>
> Generally we have macros like IDENTIFIER_P for this kind of checks.
>
> > in the C++ front-end. identifier_p is effectively LANG_IDENTIFIER_CAST
> > except that it returns a typed pointer instead of a boolean value.
>
> Hm? So you are replacing TREE_CODE (t) == IDENTIFIER_NODE
> with kind-of dynamic_cast<identifier> (t) (in C++ terms)? Then
> naming it identifier_p is bad. We have is-a.h now, so please try to use
> a single style of C++-style casting throughout GCC.
Anyway, if you see better code generated with your change compared to the
old style, I'd say you should file a PR with some preferrably short
preprocessed routine where it makes a difference, because that would show
that either VRP or other optimization pass just isn't doing good job.
It is better code for --enable-checking=yes anyway, right?
Jakub
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: C++ PATCH: use identifier_p instead of naked TREE_CODE == IDENTIFIER_NODE
2013-03-22 9:31 ` Jakub Jelinek
@ 2013-03-22 12:37 ` Gabriel Dos Reis
0 siblings, 0 replies; 10+ messages in thread
From: Gabriel Dos Reis @ 2013-03-22 12:37 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Richard Biener, gcc-patches, jason
Jakub Jelinek <jakub@redhat.com> writes:
| On Fri, Mar 22, 2013 at 10:21:05AM +0100, Richard Biener wrote:
| > On Fri, Mar 22, 2013 at 4:50 AM, Gabriel Dos Reis <gdr@axiomatics.org> wrote:
| > >
| > > This patch introduces identified_p (t) in lieu of
| > >
| > > TREE_CODE (t) == IDENTIFIER_NODE
| >
| > Generally we have macros like IDENTIFIER_P for this kind of checks.
I know we have macros for this kind of test, and what what I can tell
looking at the source code, nobody actually bothered using it.
The point of the change isn't to do that test exactly. As I explained
earlier, the change was prompted by looking at patterns of usage inf the
existing source code: it is very frequent that we would test XXX_P (t)
and immediately do XXX_GET_YYY (t) when XXX_GET_YYY will do a XXX_CHECK (t),
which morally tests again XXX_P (t), and this is all over the places.
The reason why XXX_GET_YYY (t) does XXX_CHECK (t) is because it could be
mistakenly used on a different node since t isn't "typed".
| >
| > > in the C++ front-end. identifier_p is effectively LANG_IDENTIFIER_CAST
| > > except that it returns a typed pointer instead of a boolean value.
| >
| > Hm? So you are replacing TREE_CODE (t) == IDENTIFIER_NODE
| > with kind-of dynamic_cast<identifier> (t) (in C++ terms)? Then
| > naming it identifier_p is bad. We have is-a.h now, so please try to use
| > a single style of C++-style casting throughout GCC.
|
| Anyway, if you see better code generated with your change compared to the
| old style, I'd say you should file a PR with some preferrably short
| preprocessed routine where it makes a difference, because that would show
| that either VRP or other optimization pass just isn't doing good job.
We are talking about files in cp/ right? They don't know what "short"
means:-)
With the next change, I'll try to find a short file, although currently
I am working on cp/name-lookup.c
| It is better code for --enable-checking=yes anyway, right?
Yes.
-- Gaby
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: C++ PATCH: use identifier_p instead of naked TREE_CODE == IDENTIFIER_NODE
2013-03-22 9:22 ` Richard Biener
2013-03-22 9:31 ` Jakub Jelinek
@ 2013-03-22 12:44 ` Gabriel Dos Reis
2013-03-22 12:54 ` Richard Biener
1 sibling, 1 reply; 10+ messages in thread
From: Gabriel Dos Reis @ 2013-03-22 12:44 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches, jason
Richard Biener <richard.guenther@gmail.com> writes:
[...]
| > in the C++ front-end. identifier_p is effectively LANG_IDENTIFIER_CAST
| > except that it returns a typed pointer instead of a boolean value.
|
| Hm? So you are replacing TREE_CODE (t) == IDENTIFIER_NODE
| with kind-of dynamic_cast<identifier> (t) (in C++ terms)?
It would be a mistake to name it dynamic_cast or anything like cast (I
know it is popular in certain C++ circles to name everything xxx_cast),
because dynamic_cast is an implementation detail. I should probably
have called it "give_me_identifier_pointer_if_operand_points_to_a_cxx_identifier_object"
| Then
| naming it identifier_p is bad. We have is-a.h now, so please try to use
| a single style of C++-style casting throughout GCC.
I strongly agree with consistent style, On the other hand, isn't someone going
to object that is_xxx has a predicate connotation therefore a bad naming
because it isn't returning a bool?
I think a naming that focuses too much on implementation detail is no good,
-- Gaby
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: C++ PATCH: use identifier_p instead of naked TREE_CODE == IDENTIFIER_NODE
2013-03-22 12:44 ` Gabriel Dos Reis
@ 2013-03-22 12:54 ` Richard Biener
2013-03-22 14:52 ` Gabriel Dos Reis
0 siblings, 1 reply; 10+ messages in thread
From: Richard Biener @ 2013-03-22 12:54 UTC (permalink / raw)
To: Gabriel Dos Reis; +Cc: gcc-patches, jason
On Fri, Mar 22, 2013 at 1:44 PM, Gabriel Dos Reis <gdr@axiomatics.org> wrote:
> Richard Biener <richard.guenther@gmail.com> writes:
>
> [...]
>
> | > in the C++ front-end. identifier_p is effectively LANG_IDENTIFIER_CAST
> | > except that it returns a typed pointer instead of a boolean value.
> |
> | Hm? So you are replacing TREE_CODE (t) == IDENTIFIER_NODE
> | with kind-of dynamic_cast<identifier> (t) (in C++ terms)?
>
> It would be a mistake to name it dynamic_cast or anything like cast (I
> know it is popular in certain C++ circles to name everything xxx_cast),
> because dynamic_cast is an implementation detail. I should probably
> have called it "give_me_identifier_pointer_if_operand_points_to_a_cxx_identifier_object"
>
> | Then
> | naming it identifier_p is bad. We have is-a.h now, so please try to use
> | a single style of C++-style casting throughout GCC.
>
> I strongly agree with consistent style, On the other hand, isn't someone going
> to object that is_xxx has a predicate connotation therefore a bad naming
> because it isn't returning a bool?
>
> I think a naming that focuses too much on implementation detail is no good,
Neither is one that is confusing ;)
That said - your specific identifier case should be generalized. The cgraph
people had exactly the same issue, given a symtab * return a cgraph *
if the symbol is a function, otherwise NULL, combining the previous
if (symbol == function) fn = get-me-a-function (symbol)
And they invented is-a.h as we settled for a template approach which
more readily mimics what the C++ language provides (in form of
static_cast<>, dynamic_cast<>, etc.).
The tree node case is subtly different because we are not actually casting
anything (you are not returning a more specific type than tree) but still
you provide a more C++-ish form to the standard tree.h interfaces.
That is, plain use of is-a.h is possible for your case:
template <>
inline lang_identifier *
as_a (tree p)
{
if (TREE_CODE (p) == IDENTIFIER_NODE)
return (lang_identifier *)p;
return NULL;
}
following existing GCC style (and yes, we've bikeshedded over that
already).
Thus you'd write
as_a<lang_identifier> (id)
instead of your
identifier_p (id)
(which should have been lang_identifier_p instead).
Richard.
> -- Gaby
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: C++ PATCH: use identifier_p instead of naked TREE_CODE == IDENTIFIER_NODE
2013-03-22 12:54 ` Richard Biener
@ 2013-03-22 14:52 ` Gabriel Dos Reis
0 siblings, 0 replies; 10+ messages in thread
From: Gabriel Dos Reis @ 2013-03-22 14:52 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches, jason
Richard Biener <richard.guenther@gmail.com> writes:
| On Fri, Mar 22, 2013 at 1:44 PM, Gabriel Dos Reis <gdr@axiomatics.org> wrote:
| > Richard Biener <richard.guenther@gmail.com> writes:
| >
| > [...]
| >
| > | > in the C++ front-end. identifier_p is effectively LANG_IDENTIFIER_CAST
| > | > except that it returns a typed pointer instead of a boolean value.
| > |
| > | Hm? So you are replacing TREE_CODE (t) == IDENTIFIER_NODE
| > | with kind-of dynamic_cast<identifier> (t) (in C++ terms)?
| >
| > It would be a mistake to name it dynamic_cast or anything like cast (I
| > know it is popular in certain C++ circles to name everything xxx_cast),
| > because dynamic_cast is an implementation detail. I should probably
| > have called it "give_me_identifier_pointer_if_operand_points_to_a_cxx_identifier_object"
| >
| > | Then
| > | naming it identifier_p is bad. We have is-a.h now, so please try to use
| > | a single style of C++-style casting throughout GCC.
| >
| > I strongly agree with consistent style, On the other hand, isn't someone going
| > to object that is_xxx has a predicate connotation therefore a bad naming
| > because it isn't returning a bool?
| >
| > I think a naming that focuses too much on implementation detail is no good,
|
| Neither is one that is confusing ;)
You make good points below.
I have one quibble -- since I was provoked into bike shedding :-)
|
| That said - your specific identifier case should be generalized. The cgraph
| people had exactly the same issue, given a symtab * return a cgraph *
| if the symbol is a function, otherwise NULL, combining the previous
| if (symbol == function) fn = get-me-a-function (symbol)
|
| And they invented is-a.h as we settled for a template approach which
| more readily mimics what the C++ language provides (in form of
| static_cast<>, dynamic_cast<>, etc.).
|
| The tree node case is subtly different because we are not actually casting
| anything (you are not returning a more specific type than tree) but still
| you provide a more C++-ish form to the standard tree.h interfaces.
|
| That is, plain use of is-a.h is possible for your case:
|
| template <>
| inline lang_identifier *
| as_a (tree p)
| {
| if (TREE_CODE (p) == IDENTIFIER_NODE)
| return (lang_identifier *)p;
| return NULL;
| }
|
| following existing GCC style (and yes, we've bikeshedded over that
| already).
I would have dropped the suffix "_a" on both "is_a" and "as_a", did
I get a chance to bike shed when is-a.h was introduced.
I will add the specialization and use it the appropriate places.
|
| Thus you'd write
|
| as_a<lang_identifier> (id)
|
| instead of your
|
| identifier_p (id)
|
| (which should have been lang_identifier_p instead).
-- Gaby
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-03-22 14:52 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-22 3:52 C++ PATCH: use identifier_p instead of naked TREE_CODE == IDENTIFIER_NODE Gabriel Dos Reis
2013-03-22 4:27 ` Miles Bader
2013-03-22 4:36 ` Gabriel Dos Reis
2013-03-22 4:58 ` Miles Bader
2013-03-22 9:22 ` Richard Biener
2013-03-22 9:31 ` Jakub Jelinek
2013-03-22 12:37 ` Gabriel Dos Reis
2013-03-22 12:44 ` Gabriel Dos Reis
2013-03-22 12:54 ` Richard Biener
2013-03-22 14:52 ` Gabriel Dos Reis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).