CEDET

Hi David,

  The mechanism you use here for fully qualified names is different from
how the many other partially referenced namespaces are dereferenced,
which are usually handled via the "scope" object.  The logic there has
gotten a bit tricky, but will (in theory) work in other similarly
structured languages.  The function in question is
semantic-analyze-scoped-inherited-tag-map, around line 595 where it
calls back out to the analyzer to find the tag.  The SCOPE that is
passed in is the scope for where the completion is being done, while it
probably should be the scope for where the type TYPE is.  I have no idea
what would happen if you create a scope for some other location while
fabricating a local scope.

  What I like about your patch is the idea of converting things into a
fully qualified name.  It seems like it would simplify several problems.
Doing so, however, doesn't seem to make it possible to skip the task I
mention above, as there are many ways to say the same thing, and you
have to search each possibility as opposed to just guessing at one
particular solution.

  Anyway, that's what I know off the top of my head.  I didn't analyze
your patch as my kids keep interrupting and I can't seem to keep a
straight thought in my head today.

Eric

On Wed, 2009-07-22 at 19:19 +0200, David Engster wrote:
> I must say, the vmime library is really a goldmine of difficult-to-parse
> stuff. ;-)
> 
> Consider the following situation:
> 
> namespace deer {
>   class parent
>   {
>   public:
>     int aVar;
>     void aFunc() {};
>   };
> }
> namespace deer {
>   class theChild : public parent
>   {
>   public:
>     int anotherVar;
>     void anotherFunc() {};
>   };
> }
> 
> Now 'theChild' is used in a function, but outside the namespace with a
> fully qualified name:
> 
> void somefunction(void)
> {
>   deer::theChild test;
>   test. // complete here
> }
> 
> Currently, Semantic only returns "anotherVar" and "anotherFunc", because
> it cannot find the parent class. This is due to the tag of 'theChild'
> only containing "parent" instead of "deer::parent", because the
> inheritance was done inside the namespace.
> 
> I thought the best way to fix this was to write another "dereferencer"
> function. Please find the patch attached to this mail. I also included
> the above example in the Semantic test suite.
> 
> -David

Hi Eric,

Eric M. Ludlam <eric@...> writes:
> The function in question is semantic-analyze-scoped-inherited-tag-map,
> around line 595 where it calls back out to the analyzer to find the
> tag.  The SCOPE that is passed in is the scope for where the
> completion is being done, while it probably should be the scope for
> where the type TYPE is.  I have no idea what would happen if you
> create a scope for some other location while fabricating a local
> scope.

Ah, OK. I'm not so sure semantic-scope is the right place, since at
least to me this problem seems fairly specific to C++, and therefore
should better go into semantic-c?

Anyway, I played around with semantic-analyze-scoped-inherited-tag-map,
and if the namespaces are declared in the same file I can simply get
them from TYPE's scope:

--- a/semantic/semantic-scope.el
+++ b/semantic/semantic-scope.el
@@ -583,7 +583,7 @@ whose tags can be searched when needed, OR it may be a scope object."
         ;; interfaces.  Inheriting from an interfaces implies
         ;; you have a copy of all methods locally.  I think.
         (parents (semantic-tag-type-superclasses type))
-        ps pt
+        ps newps pt sc
         )
     (dolist (p parents)
       (setq ps (cond ((stringp p) p)
@@ -592,7 +592,21 @@ whose tags can be searched when needed, OR it may be a scope object."
                     ((and (listp p) (stringp (car p)))
                      p))
            pt (condition-case nil
-                  (semantic-analyze-find-tag ps 'type scope)
+                  (progn
+                    (setq newps ps)
+                    (when (and (stringp (semantic-analyze-split-name ps))
+                               (setq sc (semantic-scope-tag-get-scope type))
+                               (setq sc (reverse (semantic-find-tags-by-type "namespace" sc))))
+                      (while sc
+                        ;; Check if parent is member of that namespace.
+                        (when (semantic-deep-find-tags-by-name
+                               ps (semantic-tag-type-members (car sc)))
+                          (when (stringp (setq newps (semantic-analyze-split-name newps)))
+                            (setq newps (list newps)))
+                          ;; Make it fully qualified
+                          (setq newps (semantic-analyze-unsplit-name (append (list (semantic-tag-name (car sc))) newps))))
+                        (setq sc (cdr sc))))
+                    (semantic-analyze-find-tag newps 'type scope))
                 (error nil)))
       (when pt
        (funcall fcn pt)

However, I'm not sure what to do when the namespaces are declared in an
external header.

>   What I like about your patch is the idea of converting things into a
> fully qualified name.  It seems like it would simplify several problems.
> Doing so, however, doesn't seem to make it possible to skip the task I
> mention above, as there are many ways to say the same thing, and you
> have to search each possibility as opposed to just guessing at one
> particular solution.

Well, I'm not really guessing, since the namespace is given in the
type-declaration (which has a fully qualified name). Also, the
dereferencer was at least able to deal with the posted problem without
any change to semantic-scope.

>   Anyway, that's what I know off the top of my head.  I didn't analyze
> your patch as my kids keep interrupting and I can't seem to keep a
> straight thought in my head today.

Yet another problem with child classes... ;-)

-David

Hi David,

  Sorry for the delay in responding.

  An interesting tactic.  I hadn't thought of using the
`semantic-deep-find-tags-by-name' as a way of resolving this issue.
Very cool.

  To solve the problem of "what if it is in a header", perhaps what is
needed is a new utility such as "calculate-scope-for-tag".  It could
visit the tag (TYPE in this case) and calculate a scope there, but only
iff the first call to semantic-analyze-find-tag failed.  What would be
cool then is that the calculated scope would contain all the remainder
information actually needed for this mapping function due to recursion.

  Hmmm.  Complicated.  Nifty.

  A useful goal is keeping this inherited-tag-map function simple if
possible.

  Due to the iterative nature of this routine, using
semantic-elp-analyze on some test code for before/after effects will
help track performance issues early.

  And now you know why it takes me so long to fix bugs like this. :)

Eric

On Thu, 2009-07-23 at 16:14 +0200, David Engster wrote:
> Hi Eric,
> 
> Eric M. Ludlam <eric@...> writes:
> > The function in question is semantic-analyze-scoped-inherited-tag-map,
> > around line 595 where it calls back out to the analyzer to find the
> > tag.  The SCOPE that is passed in is the scope for where the
> > completion is being done, while it probably should be the scope for
> > where the type TYPE is.  I have no idea what would happen if you
> > create a scope for some other location while fabricating a local
> > scope.
> 
> Ah, OK. I'm not so sure semantic-scope is the right place, since at
> least to me this problem seems fairly specific to C++, and therefore
> should better go into semantic-c?
> 
> Anyway, I played around with semantic-analyze-scoped-inherited-tag-map,
> and if the namespaces are declared in the same file I can simply get
> them from TYPE's scope:
> 
> --- a/semantic/semantic-scope.el
> +++ b/semantic/semantic-scope.el
> @@ -583,7 +583,7 @@ whose tags can be searched when needed, OR it may be a scope object."
>          ;; interfaces.  Inheriting from an interfaces implies
>          ;; you have a copy of all methods locally.  I think.
>          (parents (semantic-tag-type-superclasses type))
> -        ps pt
> +        ps newps pt sc
>          )
>      (dolist (p parents)
>        (setq ps (cond ((stringp p) p)
> @@ -592,7 +592,21 @@ whose tags can be searched when needed, OR it may be a scope object."
>                      ((and (listp p) (stringp (car p)))
>                       p))
>             pt (condition-case nil
> -                  (semantic-analyze-find-tag ps 'type scope)
> +                  (progn
> +                    (setq newps ps)
> +                    (when (and (stringp (semantic-analyze-split-name ps))
> +                               (setq sc (semantic-scope-tag-get-scope type))
> +                               (setq sc (reverse (semantic-find-tags-by-type "namespace" sc))))
> +                      (while sc
> +                        ;; Check if parent is member of that namespace.
> +                        (when (semantic-deep-find-tags-by-name
> +                               ps (semantic-tag-type-members (car sc)))
> +                          (when (stringp (setq newps (semantic-analyze-split-name newps)))
> +                            (setq newps (list newps)))
> +                          ;; Make it fully qualified
> +                          (setq newps (semantic-analyze-unsplit-name (append (list (semantic-tag-name (car sc))) newps))))
> +                        (setq sc (cdr sc))))
> +                    (semantic-analyze-find-tag newps 'type scope))
>                  (error nil)))
>        (when pt
>         (funcall fcn pt)
> 
> However, I'm not sure what to do when the namespaces are declared in an
> external header.
> 
> >   What I like about your patch is the idea of converting things into a
> > fully qualified name.  It seems like it would simplify several problems.
> > Doing so, however, doesn't seem to make it possible to skip the task I
> > mention above, as there are many ways to say the same thing, and you
> > have to search each possibility as opposed to just guessing at one
> > particular solution.
> 
> Well, I'm not really guessing, since the namespace is given in the
> type-declaration (which has a fully qualified name). Also, the
> dereferencer was at least able to deal with the posted problem without
> any change to semantic-scope.
> 
> >   Anyway, that's what I know off the top of my head.  I didn't analyze
> > your patch as my kids keep interrupting and I can't seem to keep a
> > straight thought in my head today.
> 
> Yet another problem with child classes... ;-)
> 
> -David
> 
> ------------------------------------------------------------------------------
> _______________________________________________
> Cedet-devel mailing list
> Cedet-devel@...
> https://lists.sourceforge.net/lists/listinfo/cedet-devel

Hi Eric,

Eric M. Ludlam <eric@...> writes:
>   To solve the problem of "what if it is in a header", perhaps what is
> needed is a new utility such as "calculate-scope-for-tag".  It could
> visit the tag (TYPE in this case) and calculate a scope there, but only
> iff the first call to semantic-analyze-find-tag failed.  What would be
> cool then is that the calculated scope would contain all the remainder
> information actually needed for this mapping function due to recursion.
>
>   Hmmm.  Complicated.  Nifty.
>
>   A useful goal is keeping this inherited-tag-map function simple if
> possible.
>
>   Due to the iterative nature of this routine, using
> semantic-elp-analyze on some test code for before/after effects will
> help track performance issues early.

Maybe you could take another look at the dereferencer I initially wrote
for this problem. I'm inclined to say that this is the better approach,
simply because I see this as a problem strictly regarding C++. While I
tried to keep the code general, I wouldn't know of any other languages
profiting from these extensions to inherited-tag-map, and as you say,
there's the danger that it just slows down parsing. I could also think
of languages where it might even return the wrong result, depending on
how analyze-split/unsplit work.

You mentioned that a dereferencing step would not be enough, since
inherited-tag-map must also be able to resolve the parent's
name. However, in my tests everything worked well with the only using
the dereferencer. I guess this might be because the changes the it does
to the tag names are permanent; the parent's name will stay qualified
when the dereferencer has done its job once; at least that's what I
observed.

-David

Ok, I'll go back to your other patch.

Thanks
Eric

On Wed, 2009-07-29 at 14:28 +0200, David Engster wrote:
> Hi Eric,
> 
> Eric M. Ludlam <eric@...> writes:
> >   To solve the problem of "what if it is in a header", perhaps what is
> > needed is a new utility such as "calculate-scope-for-tag".  It could
> > visit the tag (TYPE in this case) and calculate a scope there, but only
> > iff the first call to semantic-analyze-find-tag failed.  What would be
> > cool then is that the calculated scope would contain all the remainder
> > information actually needed for this mapping function due to recursion.
> >
> >   Hmmm.  Complicated.  Nifty.
> >
> >   A useful goal is keeping this inherited-tag-map function simple if
> > possible.
> >
> >   Due to the iterative nature of this routine, using
> > semantic-elp-analyze on some test code for before/after effects will
> > help track performance issues early.
> 
> Maybe you could take another look at the dereferencer I initially wrote
> for this problem. I'm inclined to say that this is the better approach,
> simply because I see this as a problem strictly regarding C++. While I
> tried to keep the code general, I wouldn't know of any other languages
> profiting from these extensions to inherited-tag-map, and as you say,
> there's the danger that it just slows down parsing. I could also think
> of languages where it might even return the wrong result, depending on
> how analyze-split/unsplit work.
> 
> You mentioned that a dereferencing step would not be enough, since
> inherited-tag-map must also be able to resolve the parent's
> name. However, in my tests everything worked well with the only using
> the dereferencer. I guess this might be because the changes the it does
> to the tag names are permanent; the parent's name will stay qualified
> when the dereferencer has done its job once; at least that's what I
> observed.
> 
> -David
> 
> ------------------------------------------------------------------------------
> Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
> trial. Simplify your report design, integration and deployment - and focus on 
> what you do best, core application coding. Discover what's new with 
> Crystal Reports now.  http://p.sf.net/sfu/bobj-july
> _______________________________________________
> Cedet-devel mailing list
> Cedet-devel@...
> https://lists.sourceforge.net/lists/listinfo/cedet-devel

Hi David,

  Sorry for the long delay.  I chased the solution I was looking for
regarding lookup up parent names in a scope derived from the location of
the class definition.  This is a bit tricky in terms of getting only the
parts of the mini-scope necessary for the lookup.  I think there is
definitely a performance hit here, but when running the unit tests the
new code takes ~2.84 seconds.  From an older copy w/out this
modification it took 2.89 seconds, so clearly it is not a measurable
difference.

Eric

On Wed, 2009-07-22 at 19:19 +0200, David Engster wrote:
> I must say, the vmime library is really a goldmine of difficult-to-parse
> stuff. ;-)
> 
> Consider the following situation:
> 
> namespace deer {
>   class parent
>   {
>   public:
>     int aVar;
>     void aFunc() {};
>   };
> }
> namespace deer {
>   class theChild : public parent
>   {
>   public:
>     int anotherVar;
>     void anotherFunc() {};
>   };
> }
> 
> Now 'theChild' is used in a function, but outside the namespace with a
> fully qualified name:
> 
> void somefunction(void)
> {
>   deer::theChild test;
>   test. // complete here
> }
> 
> Currently, Semantic only returns "anotherVar" and "anotherFunc", because
> it cannot find the parent class. This is due to the tag of 'theChild'
> only containing "parent" instead of "deer::parent", because the
> inheritance was done inside the namespace.
> 
> I thought the best way to fix this was to write another "dereferencer"
> function. Please find the patch attached to this mail. I also included
> the above example in the Semantic test suite.
> 
> -David
> 
> ------------------------------------------------------------------------------
> _______________________________________________
> Cedet-devel mailing list
> Cedet-devel@...
> https://lists.sourceforge.net/lists/listinfo/cedet-devel