bugGNU Octave - Bugs: bug #52542, Weird behaviour of vertical...

 
 

bug #52542: Weird behaviour of vertical concatenation with empty char array

Submitted by:  Luis Mendo <lmendo>
Submitted on:  Mon 27 Nov 2017 11:57:39 PM UTC  
 
Category: Octave FunctionSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Incorrect Result
Status: FixedAssigned to: None
Originator Name: Luis MendoOpen/Closed: Closed
Release: 4.2.1Operating System: Any

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Thu 30 Nov 2017 06:13:54 PM UTC, comment #8:

After some test results on the Octave Maintainer's list I committed the patch to the stable branch here (http://hg.savannah.gnu.org/hgweb/octave/rev/1212ffc13d24).

This will be a part of the 4.2.2 bug fix release expected by the end of the year.

Rik <rik5>
Project Administrator
Tue 28 Nov 2017 11:18:26 PM UTC, comment #7:

Of course, this turns out to be way, way more complicated. It appears the actual issue is in the parse tree and the parser's concatenation of string constants. I've attached a patch that works for me, but jwe should probably review as this is not my area. It seems like there were points in the parsing of

where the dimension vector (dv) of the first element was checked for emptiness with

However, there are other ways of having an empty matrix. It is possible that the code should be reviewed and the construct above replaced with the one below.

For the attached patch, there is another slight issue in that it doesn't flag a warning when the dimensions are different as is done for other data types.

(file #42512)

Rik <rik5>
Project Administrator
Tue 28 Nov 2017 06:19:31 AM UTC, comment #6:

convert_to_str is implemented by each octave_value type by the function convert_to_str_internal. For a real matrix like ones() the code should be in libinterp/octave-value/ov-re-mat.cc. The function convert_to_str internal in that file begins with

This looks alright, unless the constructor for charNDArray is somehow not doing the right thing.

Rik <rik5>
Project Administrator
Tue 28 Nov 2017 06:12:51 AM UTC, comment #5:

char() is located in libinterp/corefcn/strfcns.cc. The opening lines are

That suggests convert_to_str() has a problem, but it also means there is a third case where multiple arguments are passed to the char function. Indeed, this fails as well in a different manner.

Rik <rik5>
Project Administrator
Tue 28 Nov 2017 06:07:11 AM UTC, comment #4:

It is definitely the char() function call which is creating the problem. I can use string_fill_char() to set the fill character for the char() constructor to another value. This is then reflected in the output.

Offhand, it would seem that the constructor for char() is firing regardless of whether the dimensions indicate that it should be an empty matrix.

Rik <rik5>
Project Administrator
Tue 28 Nov 2017 06:03:05 AM UTC, comment #3:

I can sort of see how the parser might get to what you are suggesting. The examples below are somewhat confirmatory of your hypothesis that the array size is being set before the concatenation takes place.

Rik <rik5>
Project Administrator
Tue 28 Nov 2017 05:27:35 AM UTC, comment #2:

With the space after char, it's the same as if you wrote

Without looking at the actual code that builds the array, it seems to me that it is incorrectly calculating the size of the result array (filled with blanks, ASCII 32), then filling in elements. It appears to be counting the rows of the first empty array when computing the result size, but then when it inserts the empty array, there is nothing to insert, so the 'A' ends up in the first position and the second element is left with its initial value of SPC.

John W. Eaton <jwe>
Project Administrator
Tue 28 Nov 2017 01:07:17 AM UTC, comment #1:

It's even weirder than it first appears. Introduce a space after 'char' and it creates the correct output, although there is a warning.

Marking as confirmed. Seems like there is something very odd, possibly in the parser.

Rik <rik5>
Project Administrator
Mon 27 Nov 2017 11:57:39 PM UTC, original submission:

These three lines should give the same:

However, the results are respectively

(or see it online in this link: https://tio.run/##y08uSSxL/f8/Jb80KSdVIzo/L7VYw1DHQNNaQd1RPVaTCyaRnJFYpAGXRZcuSy0qSU4s0UBTpQNSpan5/z8A)

How does that space (character 32) crawl in at the end of the second result?

The output in Matlab R2017a is just `65` in all three cases.

Luis Mendo <lmendo>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #42512:  bug52542.cset added by rik5 (2KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by jwe (Posted a comment)
  • -unavailable- added by rik5 (Posted a comment)
  • -unavailable- added by lmendo (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 5 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Thu 30 Nov 2017 06:13:54 PM UTCrik5StatusPatch Submitted=>Fixed
      Open/ClosedOpen=>Closed
    Tue 28 Nov 2017 11:18:26 PM UTCrik5Attached File-=>Added bug52542.cset, #42512
      StatusConfirmed=>Patch Submitted
    Tue 28 Nov 2017 01:07:17 AM UTCrik5StatusNone=>Confirmed

    Back to the top


    Powered by Savane 3.1-cleanup1