[rfa] linespec.c, part 3

[rfa] linespec.c, part 3

[David Carlton]

[ Sorry about that almost-empty message a few minutes ago. ]

This part of the decode_line_1 refactoring moves some code
initializing some flags into a separate function set_flags.  Some
comments:

* The current code sets a flag 'has_parens' to indicate whether or not
  there is a paren, and a variable 'pp' to store the location of that
  paren.  This is redundant; we might as well have 'pp' indicate both
  parts of that data, where pp is NULL if has_parens would be 0 and pp
  isn't NULL if has_parens would be one.  Given that I'd rather pass
  as few variables by reference as possible, I decided to go that way;
  I've named the resulting variable 'paren_pointer', and updated
  references to either 'has_parens' or 'pp' accordingly.

* The current code uses the variable 'ii' both within the code that
  I've extracted and elsewhere.  But those uses are distinct: in fact,
  the value of 'ii' is changed immediately after the code that this
  patch extracts.  

David Carlton
carlton@math.stanford.edu

2002-11-11  David Carlton  <carlton@math.stanford.edu>

	* linespec.c (set_flags): New function.
	(decode_line_1): Move code into set_flags.

Index: linespec.c
===================================================================
RCS file: /cvs/src/src/gdb/linespec.c,v
retrieving revision 1.27
diff -u -p -r1.27 linespec.c
--- linespec.c	11 Nov 2002 21:18:55 -0000	1.27
+++ linespec.c	11 Nov 2002 21:29:39 -0000
@@ -42,6 +42,8 @@ extern char *operator_chars (char *, cha
 static void initialize_defaults (struct symtab **default_symtab,
 				 int *default_line);
 
+static void set_flags (char *arg, int *is_quoted, char **paren_pointer);
+
 static struct symtabs_and_lines decode_indirect (char **argptr);
 
 static void cplusplus_error (const char *name, const char *fmt, ...) ATTR_FORMAT (printf, 2, 3);
@@ -528,7 +530,7 @@ decode_line_1 (char **argptr, int funfir
   struct symtabs_and_lines values;
   struct symtab_and_line val;
   register char *p, *p1;
-  char *q, *pp, *ii, *p2;
+  char *q, *ii, *p2;
 #if 0
   char *q1;
 #endif
@@ -542,10 +544,13 @@ decode_line_1 (char **argptr, int funfir
   char *copy;
   struct symbol *sym_class;
   int i1;
+  /* This is NULL if there are no parens in *ARGPTR, or a pointer to
+     the closing parenthesis if there are parens.  */
+  char *paren_pointer;
+  /* This says whether or not something in *ARGPTR is quoted with
+     completer_quotes (i.e. with single quotes).  */
   int is_quoted;
   int is_quote_enclosed;
-  int has_parens;
-  int has_if = 0;
   int has_comma = 0;
   struct symbol **sym_arr;
   struct type *t;
@@ -563,45 +568,13 @@ decode_line_1 (char **argptr, int funfir
   if (**argptr == '*')
     return decode_indirect (argptr);
 
-  /* 'has_if' is for the syntax:
-   *     (gdb) break foo if (a==b)
-   */
-  if ((ii = strstr (*argptr, " if ")) != NULL ||
-      (ii = strstr (*argptr, "\tif ")) != NULL ||
-      (ii = strstr (*argptr, " if\t")) != NULL ||
-      (ii = strstr (*argptr, "\tif\t")) != NULL ||
-      (ii = strstr (*argptr, " if(")) != NULL ||
-      (ii = strstr (*argptr, "\tif( ")) != NULL)
-    has_if = 1;
-  /* Temporarily zap out "if (condition)" to not
-   * confuse the parenthesis-checking code below.
-   * This is undone below. Do not change ii!!
-   */
-  if (has_if)
-    {
-      *ii = '\0';
-    }
-
   /* Set various flags.
-   * 'has_parens' is important for overload checking, where
+   * 'paren_pointer' is important for overload checking, where
    * we allow things like: 
    *     (gdb) break c::f(int)
    */
 
-  /* Maybe arg is FILE : LINENUM or FILE : FUNCTION */
-
-  is_quoted = (**argptr
-	       && strchr (get_gdb_completer_quote_characters (),
-			  **argptr) != NULL);
-
-  has_parens = ((pp = strchr (*argptr, '(')) != NULL
-		&& (pp = strrchr (pp, ')')) != NULL);
-
-  /* Now that we're safely past the has_parens check,
-   * put back " if (condition)" so outer layers can see it 
-   */
-  if (has_if)
-    *ii = ' ';
+  set_flags (*argptr, &is_quoted, &paren_pointer);
 
   /* Maybe we were called with a line range FILENAME:LINENUM,FILENAME:LINENUM
      and we must isolate the first half.  Outer layers will call again later
@@ -682,7 +655,7 @@ decode_line_1 (char **argptr, int funfir
   if (has_comma)
     *ii = ',';
 
-  if ((p[0] == ':' || p[0] == '.') && !has_parens)
+  if ((p[0] == ':' || p[0] == '.') && paren_pointer == NULL)
     {
       /*  C++ */
       /*  ... or Java */
@@ -1075,9 +1048,9 @@ decode_line_1 (char **argptr, int funfir
       if (p[-1] != '\'')
 	error ("Unmatched single quote.");
     }
-  else if (has_parens)
+  else if (paren_pointer != NULL)
     {
-      p = pp + 1;
+      p = paren_pointer + 1;
     }
   else
     {
@@ -1214,6 +1187,46 @@ initialize_defaults (struct symtab **def
       *default_symtab = cursal.symtab;
       *default_line = cursal.line;
     }
+}
+
+static void
+set_flags (char *arg, int *is_quoted, char **paren_pointer)
+{
+  char *ii;
+  int has_if = 0;
+
+  /* 'has_if' is for the syntax:
+   *     (gdb) break foo if (a==b)
+   */
+  if ((ii = strstr (arg, " if ")) != NULL ||
+      (ii = strstr (arg, "\tif ")) != NULL ||
+      (ii = strstr (arg, " if\t")) != NULL ||
+      (ii = strstr (arg, "\tif\t")) != NULL ||
+      (ii = strstr (arg, " if(")) != NULL ||
+      (ii = strstr (arg, "\tif( ")) != NULL)
+    has_if = 1;
+  /* Temporarily zap out "if (condition)" to not
+   * confuse the parenthesis-checking code below.
+   * This is undone below. Do not change ii!!
+   */
+  if (has_if)
+    {
+      *ii = '\0';
+    }
+
+  *is_quoted = (*arg
+		&& strchr (get_gdb_completer_quote_characters (),
+			   *arg) != NULL);
+
+  *paren_pointer = strchr (arg, '(');
+  if (*paren_pointer != NULL)
+    *paren_pointer = strrchr (*paren_pointer, ')');
+
+  /* Now that we're safely past the paren_pointer check,
+   * put back " if (condition)" so outer layers can see it 
+   */
+  if (has_if)
+    *ii = ' ';
 }
 
 \f

Re: [rfa] linespec.c, part 3

[Elena Zannoni]

David Carlton writes:
 > [ Sorry about that almost-empty message a few minutes ago. ]
 > 
 > This part of the decode_line_1 refactoring moves some code
 > initializing some flags into a separate function set_flags.  Some
 > comments:
 > 

Looks ok, but I have a question. Wouldn't the code that sets up
has_comma and is_quote_enclosed also belong to this set_flags
function? (Ideally a struct of flags could be set up). I see that
has_comma plays the same trick with changing the char before parsing
and restoring it afterwards. :-( I wonder if this could be changed by
using a comma_ptr pointer and changing the for loop to continue until
the comma_ptr instead of up to '\0'.  Or just duplicate the first half
of the string and play with that, it's not that we would be using a
lot more memory, and it would seem less fragile than relying on a
comment pleading you not to change 'ii'. Smae could be said for the
current use of 'ii' with 'if'. But this is for a separate patch, you
are just moving code around now.

 > * The current code sets a flag 'has_parens' to indicate whether or not
 >   there is a paren, and a variable 'pp' to store the location of that
 >   paren.  This is redundant; we might as well have 'pp' indicate both
 >   parts of that data, where pp is NULL if has_parens would be 0 and pp
 >   isn't NULL if has_parens would be one.  Given that I'd rather pass
 >   as few variables by reference as possible, I decided to go that way;
 >   I've named the resulting variable 'paren_pointer', and updated
 >   references to either 'has_parens' or 'pp' accordingly.
 > 

good move.

 > * The current code uses the variable 'ii' both within the code that
 >   I've extracted and elsewhere.  But those uses are distinct: in fact,
 >   the value of 'ii' is changed immediately after the code that this
 >   patch extracts.  
 > 

ok.

go ahead, thanks!

Elena


 > David Carlton
 > carlton@math.stanford.edu
 > 
 > 2002-11-11  David Carlton  <carlton@math.stanford.edu>
 > 
 > 	* linespec.c (set_flags): New function.
 > 	(decode_line_1): Move code into set_flags.
 > 
 > Index: linespec.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/linespec.c,v
 > retrieving revision 1.27
 > diff -u -p -r1.27 linespec.c
 > --- linespec.c	11 Nov 2002 21:18:55 -0000	1.27
 > +++ linespec.c	11 Nov 2002 21:29:39 -0000
 > @@ -42,6 +42,8 @@ extern char *operator_chars (char *, cha
 >  static void initialize_defaults (struct symtab **default_symtab,
 >  				 int *default_line);
 >  
 > +static void set_flags (char *arg, int *is_quoted, char **paren_pointer);
 > +
 >  static struct symtabs_and_lines decode_indirect (char **argptr);
 >  
 >  static void cplusplus_error (const char *name, const char *fmt, ...) ATTR_FORMAT (printf, 2, 3);
 > @@ -528,7 +530,7 @@ decode_line_1 (char **argptr, int funfir
 >    struct symtabs_and_lines values;
 >    struct symtab_and_line val;
 >    register char *p, *p1;
 > -  char *q, *pp, *ii, *p2;
 > +  char *q, *ii, *p2;
 >  #if 0
 >    char *q1;
 >  #endif
 > @@ -542,10 +544,13 @@ decode_line_1 (char **argptr, int funfir
 >    char *copy;
 >    struct symbol *sym_class;
 >    int i1;
 > +  /* This is NULL if there are no parens in *ARGPTR, or a pointer to
 > +     the closing parenthesis if there are parens.  */
 > +  char *paren_pointer;
 > +  /* This says whether or not something in *ARGPTR is quoted with
 > +     completer_quotes (i.e. with single quotes).  */
 >    int is_quoted;
 >    int is_quote_enclosed;
 > -  int has_parens;
 > -  int has_if = 0;
 >    int has_comma = 0;
 >    struct symbol **sym_arr;
 >    struct type *t;
 > @@ -563,45 +568,13 @@ decode_line_1 (char **argptr, int funfir
 >    if (**argptr == '*')
 >      return decode_indirect (argptr);
 >  
 > -  /* 'has_if' is for the syntax:
 > -   *     (gdb) break foo if (a==b)
 > -   */
 > -  if ((ii = strstr (*argptr, " if ")) != NULL ||
 > -      (ii = strstr (*argptr, "\tif ")) != NULL ||
 > -      (ii = strstr (*argptr, " if\t")) != NULL ||
 > -      (ii = strstr (*argptr, "\tif\t")) != NULL ||
 > -      (ii = strstr (*argptr, " if(")) != NULL ||
 > -      (ii = strstr (*argptr, "\tif( ")) != NULL)
 > -    has_if = 1;
 > -  /* Temporarily zap out "if (condition)" to not
 > -   * confuse the parenthesis-checking code below.
 > -   * This is undone below. Do not change ii!!
 > -   */
 > -  if (has_if)
 > -    {
 > -      *ii = '\0';
 > -    }
 > -
 >    /* Set various flags.
 > -   * 'has_parens' is important for overload checking, where
 > +   * 'paren_pointer' is important for overload checking, where
 >     * we allow things like: 
 >     *     (gdb) break c::f(int)
 >     */
 >  
 > -  /* Maybe arg is FILE : LINENUM or FILE : FUNCTION */
 > -
 > -  is_quoted = (**argptr
 > -	       && strchr (get_gdb_completer_quote_characters (),
 > -			  **argptr) != NULL);
 > -
 > -  has_parens = ((pp = strchr (*argptr, '(')) != NULL
 > -		&& (pp = strrchr (pp, ')')) != NULL);
 > -
 > -  /* Now that we're safely past the has_parens check,
 > -   * put back " if (condition)" so outer layers can see it 
 > -   */
 > -  if (has_if)
 > -    *ii = ' ';
 > +  set_flags (*argptr, &is_quoted, &paren_pointer);
 >  
 >    /* Maybe we were called with a line range FILENAME:LINENUM,FILENAME:LINENUM
 >       and we must isolate the first half.  Outer layers will call again later
 > @@ -682,7 +655,7 @@ decode_line_1 (char **argptr, int funfir
 >    if (has_comma)
 >      *ii = ',';
 >  
 > -  if ((p[0] == ':' || p[0] == '.') && !has_parens)
 > +  if ((p[0] == ':' || p[0] == '.') && paren_pointer == NULL)
 >      {
 >        /*  C++ */
 >        /*  ... or Java */
 > @@ -1075,9 +1048,9 @@ decode_line_1 (char **argptr, int funfir
 >        if (p[-1] != '\'')
 >  	error ("Unmatched single quote.");
 >      }
 > -  else if (has_parens)
 > +  else if (paren_pointer != NULL)
 >      {
 > -      p = pp + 1;
 > +      p = paren_pointer + 1;
 >      }
 >    else
 >      {
 > @@ -1214,6 +1187,46 @@ initialize_defaults (struct symtab **def
 >        *default_symtab = cursal.symtab;
 >        *default_line = cursal.line;
 >      }
 > +}
 > +
 > +static void
 > +set_flags (char *arg, int *is_quoted, char **paren_pointer)
 > +{
 > +  char *ii;
 > +  int has_if = 0;
 > +
 > +  /* 'has_if' is for the syntax:
 > +   *     (gdb) break foo if (a==b)
 > +   */
 > +  if ((ii = strstr (arg, " if ")) != NULL ||
 > +      (ii = strstr (arg, "\tif ")) != NULL ||
 > +      (ii = strstr (arg, " if\t")) != NULL ||
 > +      (ii = strstr (arg, "\tif\t")) != NULL ||
 > +      (ii = strstr (arg, " if(")) != NULL ||
 > +      (ii = strstr (arg, "\tif( ")) != NULL)
 > +    has_if = 1;
 > +  /* Temporarily zap out "if (condition)" to not
 > +   * confuse the parenthesis-checking code below.
 > +   * This is undone below. Do not change ii!!
 > +   */
 > +  if (has_if)
 > +    {
 > +      *ii = '\0';
 > +    }
 > +
 > +  *is_quoted = (*arg
 > +		&& strchr (get_gdb_completer_quote_characters (),
 > +			   *arg) != NULL);
 > +
 > +  *paren_pointer = strchr (arg, '(');
 > +  if (*paren_pointer != NULL)
 > +    *paren_pointer = strrchr (*paren_pointer, ')');
 > +
 > +  /* Now that we're safely past the paren_pointer check,
 > +   * put back " if (condition)" so outer layers can see it 
 > +   */
 > +  if (has_if)
 > +    *ii = ' ';
 >  }
 >  
 >  \f

Re: [rfa] linespec.c, part 3

[David Carlton]

On Mon, 11 Nov 2002 17:28:45 -0500, Elena Zannoni <ezannoni@redhat.com> said:
> David Carlton writes:

>> This part of the decode_line_1 refactoring moves some code
>> initializing some flags into a separate function set_flags.  Some
>> comments:

> Looks ok, but I have a question. Wouldn't the code that sets up
> has_comma and is_quote_enclosed also belong to this set_flags
> function?

It seems entirely plausible to me that that is the case.  For now,
that will come in my next patch, which won't quite do what you ask:
the problem is that that part of the code is also updating 'p' to
figure out where a compound linespec separator is (for filenames or
C++/Java objects).  Teasing this apart is a bit of a mess; my current
"final" version of these changes has 'has_comma' buried in an internal
function, and 'is_quote_enclosed' doesn't last for very long.  But
it's messy, so I think that, once I'm done submitting patches that
just break off code into separate functions, we should revisit this
issue.

> (Ideally a struct of flags could be set up).

Yes, that would be a much better idea than passing the flag variables
by reference.

> But this is for a separate patch, you are just moving code around
> now.

Right.

> go ahead, thanks!

Committed.

(Now you've caught up to me!  Eek!  I'll have to get my grading done
quickly so I can keep up my end of things. :-) )

David Carlton
carlton@math.stanford.edu

Re: [rfa] linespec.c, part 3

[Elena Zannoni]

David Carlton writes:
 > On Mon, 11 Nov 2002 17:28:45 -0500, Elena Zannoni <ezannoni@redhat.com> said:
 > > David Carlton writes:
 > 
 > >> This part of the decode_line_1 refactoring moves some code
 > >> initializing some flags into a separate function set_flags.  Some
 > >> comments:
 > 
 > > Looks ok, but I have a question. Wouldn't the code that sets up
 > > has_comma and is_quote_enclosed also belong to this set_flags
 > > function?
 > 
 > It seems entirely plausible to me that that is the case.  For now,
 > that will come in my next patch, which won't quite do what you ask:
 > the problem is that that part of the code is also updating 'p' to
 > figure out where a compound linespec separator is (for filenames or
 > C++/Java objects).  Teasing this apart is a bit of a mess; my current
 > "final" version of these changes has 'has_comma' buried in an internal
 > function, and 'is_quote_enclosed' doesn't last for very long.  But
 > it's messy, so I think that, once I'm done submitting patches that
 > just break off code into separate functions, we should revisit this
 > issue.
 > 

Yes, I didn't really think the issue through.

 > > (Ideally a struct of flags could be set up).
 > 
 > Yes, that would be a much better idea than passing the flag variables
 > by reference.
 > 
 > > But this is for a separate patch, you are just moving code around
 > > now.
 > 
 > Right.
 > 
 > > go ahead, thanks!
 > 
 > Committed.
 > 
 > (Now you've caught up to me!  Eek!  I'll have to get my grading done
 > quickly so I can keep up my end of things. :-) )
 > 

Don't worry, I have plenty of other things to do myself!

Elena


 > David Carlton
 > carlton@math.stanford.edu

Re: [rfa] linespec.c, part 3

[Klee Dienes]

[-- Attachment #1: Type: text/plain, Size: 1230 bytes --]

I'm not sure if you want to deal with this now, or if you'd rather I 
wait until you're done with merging your changes, but I figured I'd 
mention it now while you were looking at set_flags stuff:

The linespec.c changes you posted looked so cool, we couldn't resist 
using them.  So rather than try to deal with the merge conflicts from 
the upcoming stream of linespec.c patches, we decided to re-add our 
Objective-C support to the linespec.c from your branch and use that as 
our linespec.c.

One issue that came up was that the part of our code that decodes an 
Objective-C function needs to have the if-clause removed from the 
breakpoint expression (since Objective-C functions can contain spaces). 
We did this by extending set_flags to pass back a pointer to the 
if-clause, if there was one, and by splitting decode_line_1 into two 
functions, one of which sets the defaults, calls set_flags, strips off 
the if-clause and calls decode_line_2, which does the actual parsing.

The downside to this is that there's yet one more function to be called 
in the parsing process; the upside is that the actual parsing code 
doesn't have to worry about the presence of the if-clause, and that 
decode_line_1 gets even shorter.


[-- Attachment #2: linespec.txt --]
[-- Type: text/plain, Size: 4146 bytes --]

--- /Volumes/Storage/Users/kdienes/source/cygnus.cygnus/src/gdb/linespec.c	Mon Nov 11 16:47:45 2002
+++ linespec.c	Mon Nov 11 17:00:39 2002
@@ -39,7 +39,8 @@
 static void initialize_defaults (struct symtab **default_symtab,
 				 int *default_line);
 
-static void set_flags (char *arg, int *is_quoted, char **paren_pointer);
+static void set_flags (char *arg, int *is_quoted, char **paren_pointer,
+		       char **if_pointer);
 
 static struct symtabs_and_lines decode_indirect (char **argptr);
 
@@ -203,38 +204,33 @@
    from char ** to const char **.  */
 
 struct symtabs_and_lines
-decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
-	       int default_line, char ***canonical)
+decode_line_2 (char **argptr, int funfirstline, struct symtab *default_symtab,
+	       int default_line, char ***canonical,
+	       int is_quoted, char *paren_pointer)
 {
-  /* This is NULL if there are no parens in *ARGPTR, or a pointer to
-     the closing parenthesis if there are parens.  */
-  char *paren_pointer;
-  /* This says whether or not something in *ARGPTR is quoted with
-     completer_quotes (i.e. with single quotes).  */
-  int is_quoted;
   /* If a file name is specified, this is its symtab.  */
   struct symtab *file_symtab = NULL;
   /* This function advances *ARGPTR, but, for error messages, we want
      to remember what it pointed to initially.  */
   char *saved_arg = *argptr;
 
-  /* Defaults have defaults.  */
-
-  initialize_defaults (&default_symtab, &default_line);
-  
-  /* Set various flags.
-   * 'paren_pointer' is important for overload checking, where
-   * we allow things like: 
-   *     (gdb) break c::f(int)
-   */
-
-  set_flags (*argptr, &is_quoted, &paren_pointer);
-
   /* See if arg is *PC.  */
 
   if (**argptr == '*')
     return decode_indirect (argptr);
 
+  /* Is it an Objective-C selector? */
+
+#if 0
+  {
+    struct symtabs_and_lines values;
+    values = decode_objc (argptr, funfirstline, file_symtab,
+			  canonical, saved_arg);
+    if (values.sals != NULL)
+      return values;
+  }
+#endif
+
   /* Check to see if it's a multipart linespec (with colons or
      periods).  */
   {
@@ -289,6 +285,48 @@
 			  paren_pointer, file_symtab);
 }
 
+struct symtabs_and_lines
+decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
+	       int default_line, char ***canonical)
+{
+  /* This says whether or not something in *ARGPTR is quoted with
+     completer_quotes (i.e. with single quotes).  */
+  int is_quoted;
+
+  /* This is NULL if there are no parens in *ARGPTR, or a pointer to
+     the closing parenthesis if there are parens.  */
+  char *paren_pointer;
+
+  /* This is NULL if there is no if-clause at the end of *ARGPTR, or a
+     pointer to the whitespace before the 'if' if there is. */
+  char *if_pointer;
+
+  struct symtabs_and_lines values;
+
+  /* Defaults have defaults.  */
+
+  initialize_defaults (&default_symtab, &default_line);
+  
+  /* Set various flags.
+   * 'paren_pointer' is important for overload checking, where
+   * we allow things like: 
+   *     (gdb) break c::f(int)
+   */
+  
+  set_flags (*argptr, &is_quoted, &paren_pointer, &if_pointer);
+
+  if (if_pointer != NULL)
+    *if_pointer = '\0';
+
+  values = decode_line_2 (argptr, funfirstline, default_symtab, default_line,
+			  canonical, is_quoted, paren_pointer);
+
+  if (if_pointer != NULL)
+    *if_pointer = ' ';
+
+  return values;
+}
+
 \f
 
 /* Now, the helper functions.  */
@@ -326,7 +364,7 @@
 }
 
 static void
-set_flags (char *arg, int *is_quoted, char **paren_pointer)
+set_flags (char *arg, int *is_quoted, char **paren_pointer, char **if_pointer)
 {
   char *if_index;
   char *paren_start;
@@ -372,7 +410,12 @@
   /* Now that we're safely past the has_parens check, put back " if
      (condition)" so outer layers can see it.  */
   if (has_if)
-    *if_index = ' ';
+    {
+      *if_pointer = if_index;
+      *if_index = ' ';
+    }
+  else
+    *if_pointer = NULL;
 }
 
 \f

Re: [rfa] linespec.c, part 3

[David Carlton]

On Mon, 11 Nov 2002 17:30:41 -0500, Klee Dienes <klee@apple.com> said:

> The linespec.c changes you posted looked so cool, we couldn't resist
> using them.

Great!  I would seem to have fans everywhere. :-)

> One issue that came up was that the part of our code that decodes an
> Objective-C function needs to have the if-clause removed from the
> breakpoint expression (since Objective-C functions can contain
> spaces). We did this by extending set_flags to pass back a pointer
> to the if-clause, if there was one, and by splitting decode_line_1
> into two functions, one of which sets the defaults, calls set_flags,
> strips off the if-clause and calls decode_line_2, which does the
> actual parsing.

I think that's pretty sensible.  I don't care too much one way or
another as to whether or not there's a separate 'decode_line_2'
function to do the parsing, but the general idea of setting various
necessary flags first before doing any decoding sounds good to me.

For now, I want to worry about breaking up the existing code into
separate functions first (I'm already giving Elena enough to worry
about as it is, I think), but it sounds like, once that phase is done
and we can actually understand what decode_line_1 is doing, we'll want
to start rewriting functions, changing the existing reorganization.
So hold off on your patch until then, and then we can talk.

David Carlton
carlton@math.stanford.edu

Re: [rfa] linespec.c, part 3

[Daniel Jacobowitz]

On Mon, Nov 11, 2002 at 05:30:41PM -0500, Klee Dienes wrote:
> I'm not sure if you want to deal with this now, or if you'd rather I 
> wait until you're done with merging your changes, but I figured I'd 
> mention it now while you were looking at set_flags stuff:
> 
> The linespec.c changes you posted looked so cool, we couldn't resist 
> using them.  So rather than try to deal with the merge conflicts from 
> the upcoming stream of linespec.c patches, we decided to re-add our 
> Objective-C support to the linespec.c from your branch and use that as 
> our linespec.c.
> 
> One issue that came up was that the part of our code that decodes an 
> Objective-C function needs to have the if-clause removed from the 
> breakpoint expression (since Objective-C functions can contain spaces). 
> We did this by extending set_flags to pass back a pointer to the 
> if-clause, if there was one, and by splitting decode_line_1 into two 
> functions, one of which sets the defaults, calls set_flags, strips off 
> the if-clause and calls decode_line_2, which does the actual parsing.

Dare I ask what you did with the existing decode_line_2, also in that
file?  Other than that, it looks reasonable...

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [rfa] linespec.c, part 3

[Klee Dienes]

The patch I posted was against your branched linespec.c, in which you'd 
renamed decode_line_2 to select_symbols.  If you like, I'd be happy to 
submit the patch as a patch against the TOT linespec.c assuming your 
patch 3 has been applied.

On Monday, November 11, 2002, at 05:56 PM, Daniel Jacobowitz wrote:
>
> Dare I ask what you did with the existing decode_line_2, also in that
> file?  Other than that, it looks reasonable...

[rfa] linespec.c, part 3

[David Carlton]

Here's the third part.