BruceDLong/ProteusCore

agent::addIDs() incorrectly assumes a list of items in an LValue are "alternatives"

Opened this issue · 0 comments

Description

I am working with this infon:

{ [5, 7, 3, 8] {[?]. | ...}} :: {...}

It's a little strange because the arguments to the function [5, 7, 3, 8] are hard-coded and the normal argument list is 'unknown': {...}
Nevertheless, while working with this I found another issue. Since the infon in question won't (yet) produce a valid output I cannot easily express the error in terms of expected results from normalizing it. Instead I'll talk about the expected results of debugging.

Expected Result

When stepping through the code with ddd, I expect the current item (CI) to progress like this:

  1. topInfon
  2. topInfon->spec2
  3. topInfon->spec2->value // here, this item's wrklist gets set as expected.
  4. topInfon->spec2->value->value // here, this item should get a single ident in its wrklist.

Item 4 represents the '5' in the argument list [5, 7, 3, 8]. It's next fields are the 7, 3, and 8.

Because these items are already filled in I don't expect normalizing them to do anything even though they have something in their wrkList.

Actual result

Item 4 above (the '5') gets two items added to its wrklist. They are both an infon with size=-1 and value=5. Also, quite unexpectedly, item 4's next field (the '7') also receives two items in it's wrklist. It isn't even that items turn to be normalized.

Partial explanation

In agent::addID(), there is a possibility of alternate scenarios ('alts') all needing to have idents added to their wrkLists. Somehow the two items generated for the unknown argument list are interpreted as alternates instead of as siblings in a list.

(Note, the two items come from the code {...}. At some point, because it is a list, initList gets called on it. This generates two items in the list. They are both marked 'tentative' because it isn't yet know whether this list has any elements in it. However, the 'alt-processing-code' in addIDs is seeing that and thinking that the tentative siblings are therefore alts.

Discussion of solution

As a first thought it seems that we need to figure out a better way of detecting whether items processed in addIDs() are siblings or alts. See my just-added comments around lines 6-10 in the following code:

void agent::addIDs(infon* Lvals, infon* Rvals, int asAlt){
    const int maxAlternates=100;
    infon* RvlLst[maxAlternates]; infon* crntAlt=Rvals; infon *pred, *Rval, *prev=0; infNode* IDp; UInt size;
    int altCnt=1; RvlLst[0]=crntAlt;
    while(crntAlt && (crntAlt->flags&isTentative)){  // This condition is mistaking siblings for alts.
        getFollower(&crntAlt, getTop(crntAlt));
        RvlLst[altCnt++]=crntAlt;  // altCnt is incremented causing two items to be set (the 5 and the 7).
        if(altCnt>=maxAlternates) throw "Too many nested alternates";
    }
    size=((UInt)Lvals->next->size)-2; crntAlt=Lvals; pred=Lvals->pred;
    while(crntAlt){  // Lvals
        for(int i=0; i<altCnt; ++i){   // Rvals
            if (!asAlt && altCnt==1 && crntAlt==Lvals){
                insertID(&Lvals->wrkList,Rvals,0); recAlts(Lvals,Rvals);
            } else {
                Rval=RvlLst[i];
                AddSizeAlternate(crntAlt, Rval, pred, size, (prev)?prev->prev:0);
            }
        }
        if(crntAlt->flags&isTentative) {prev=crntAlt; size=((UInt)crntAlt->next->size)-2; crntAlt=getTop(crntAlt); if(crntAlt) crntAlt->flags|=hasAlts;}
        else crntAlt=0;
    }
}

Reproducing the problem in git commit ed587c2

  • $cd proteus/core
  • $make
  • $ddd ptest &
  • In ddd (or gdb if you're brave) set a breakpoint at line 275: while(crntAlt && (crntAlt->flags&isTentative)){
  • Run <altTest.pr
  • Step through the while loop. You'll notice it is mis-identifying the RVals as alternates instead of dropping out the the loop because the 'next' item is a sibling.