On 09/14/2011 04:50 AM, Dodji Seketeli wrote:
To comply with this invariant, the initial patch of Tom was cloning T
into T', and was using T'->src_loc to track the virtual location of T.
Which is close to what you are proposing, while respecting the
invariant.
Yes, that's what I had in mind. I didn't realize that there was only
one token for FOO.
But it turned out that was using too much memory :-(. So
we devised this scheme instead.
Ah. :(
+ void *macro;
This should be a union rather than an untyped pointer.
+ else if (context->tokens_kind == TOKENS_KIND_EXTENDED)
+ {
+ /* So we are in presence of an extended token context, which
+ means that each token in this context has a virtual
+ location attached to it. So let's not forget to update
+ the pointer to the current virtual location of the
+ current token when we update the pointer to the current
+ token */
+
+ rhs = *FIRST (context).ptoken++;
+ if (context->macro)
The other places that deal with TOKENS_KIND_EXTENDED don't test that
context->macro is non-null. Why is it needed here?
+ {
+ cpp_hashnode *macro;
+ if (context->tokens_kind == TOKENS_KIND_EXTENDED)
+ {
+ macro_context *mc = (macro_context *) context->macro;
+ macro = mc->macro_node;
+ /* If context->buff is set, it means the life time of tokens
+ is bound to the life time of this context; so we must
+ free the tokens; that means we must free the virtual
+ locations of these tokens too. */
+ if (context->buff && mc->virt_locs)
+ {
+ free (mc->virt_locs);
+ mc->virt_locs = NULL;
+ }
+ free (mc);
+ context->macro = NULL;
+ }
+ else
+ macro = (cpp_hashnode *) context->macro;
+
+ if (macro != NULL)
+ macro->flags &= ~NODE_DISABLED;
How can macro end up NULL if context->macro was set?
+/* In the traditionnal mode of the preprocessor, if we are currently
+ location if we are in the traditionnal mode, and just returns
"traditional"
I don't think we need to talk about virtual locations before
cpp_get_token_1; it's not an external interface, and it's redundant with
the description before cpp_get_token_with_location.
+ result = cpp_get_token_1 (pfile, loc);
if (pfile->context->macro)
- *loc = pfile->invocation_location;
+ {
+ if (!CPP_OPTION (pfile, track_macro_expansion))
+ *loc = pfile->invocation_location;
+ }
else
*loc = result->src_loc;
+ *loc = maybe_adjust_loc_for_trad_cpp (pfile, *loc);
Let's move this code into cpp_get_token_1 so that all the location
tweaking is in one place.
+ switch (it->kind)
+ {
+ case MACRO_ARG_TOKEN_NORMAL:
+ case MACRO_ARG_TOKEN_EXPANDED:
+ it->token_ptr++;
+ if (track_macro_exp_p)
+ it->location_ptr++;
+ break;
+ case MACRO_ARG_TOKEN_STRINGIFIED:
+#ifdef ENABLE_CHECKING
+ if (it->num_forwards > 0)
+ abort ();
+ it->num_forwards++;
+#endif
+ break;
+ }
Don't you want to increment num_forwards in the normal/expanded cases, too?
+tokens_buff_append_token (cpp_reader *pfile,
+ _cpp_buff *buffer,
+ source_location *virt_locs,
+ const cpp_token *token,
+ source_location def_loc,
+ source_location parm_def_loc,
+ const struct line_map *map,
+ unsigned int *macro_token_index)
Why is macro_token_index a pointer? Nothing seems to modify the referent.
+/* Appends a token to the end of the token buffer BUFFER. Note that
+ this function doesn't enlarge BUFFER; it overwrite the last memory
+ location of BUFFER that holds a token.
That doesn't sound like appending.
Jason
