Bug 134712 - Redim Preserve for existing arrays is accepted without any index range, but acts differently as if the unchange index ranges are made explicit.
Summary: Redim Preserve for existing arrays is accepted without any index range, but a...
Status: RESOLVED WONTFIX
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: BASIC (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-07-10 10:45 UTC by Wolfgang Jäger
Modified: 2020-07-11 10:32 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Wolfgang Jäger 2020-07-10 10:45:17 UTC
To get the clue to the subject, go stepwise through the following code and inspect the used array variables after each step:  

Sub directRedim()
REM Should closely be related to tdf#129256 .
Dim a(1 To 5, 1 To 3)
a(2, 2) = "§§"
b = a
Redim Preserve b
REM Should either be rejected (bad) or treated as if
REM the correct index ranges were explicitly given (good). See below!
a(2, 2) = "%%"
b(2, 2) = "Paragraphs"
a(2, 2) = "§§"
REM You see: You dont get it like with a dim c().
REM Basic applies the already set index ranges. 
The functional identity of b with a persists. 

Redim a(1 To 5, 1 To 3)
a(2, 2) = "§§"
b = a
Redim Preserve b(1 To 5, 1 To 3)
a(2, 2) = "%%"
b(2, 2) = "Paragraphs"
REM You see: b Now has an own identity disconnected from a
REM as if an assignment was following the the ByVal semantics. 

REM Actually the treatment in both variants is different in an error-prone way.
End Sub

For conceptual reasons the second variant is the only useful one.  
I'm not sure If there is much code relying on the first variant as is. 
Assuming it's rarely used because factually doing nothing, a unification of the behavior in the sense that the first variant would work like the second one should be safe enough. It would clearly simplify and make more efficient the creation of copies of arrays. Such an operation soetimes is actually needed.
Comment 1 Mike Kaganski 2020-07-10 16:57:06 UTC
The code like

> ReDim Preserve B

gets compiled out (so it can't even have a breakpoint). That happens in SbiParser::DefVar, where the call to VarDecl naturally sets pDim (the dimensions list) to nullptr, and then if( pDim ) conditions are false.

I don't think we should change that. To allow copying arrays, it's better to implement proper ByValue semantics for them (which is allowed in SB, but is not respected). Additionally, in other BASIC flavors, this is a syntax error (e.g., VBA); I don't think we also should make it an error: that could break something existing, while the status quo is harmless.

Closing WONTFIX. But possibly we should mention the noop variant in the help.
Comment 2 Wolfgang Jäger 2020-07-11 10:32:47 UTC
Spoken aside...  

I'm not happy with the WONTFIX.  

Redim Preserve b
is disregarded and will not even accept a breakpoit.
Yes. But that's inconsistent. 

It might be resolved like the case of the formerly accepted missing or additional trailing parentehses at the end of a BASIC line where you introduced your "dual strategy". 

LibO Basic actually *should* offer a simple and efficient way to get the ByVal semantics for arrays of any kind, dimensionality ... 
Well, there is the problematic case of the very special array types
.Data .DataArray .FormulaArray

The "Redim Preserve myArray" without index ranges looks like a simple solution applicable in hindsight. There may be reasons to exclude the "very special arrays".  

I don't think ther will be more than a handful (<=5) BASIC routines in the world containing such a disregared as-if-ststement, and *relying* on its being disregarded. There's a REM, after all for thinkable purposes.

(There are additional issues, of course, concerning .DataArray and thelike.)