Project

General

Profile

Slug #31

theValue makes copy

Added by Anna Maria Bigatti over 12 years ago. Updated about 2 years ago.

Status:
In Progress
Priority:
Normal
Assignee:
Anna Maria Bigatti
Category:
Parser/Interpreter
Target version:
CoCoA-5.4.2
Start date:
15 Nov 2011
Due date:
% Done:

60%

Estimated time:
10.00 h
Spent time:
6.05 h

Description

in Interpreter.H

template <typename T> typename CoCoALibType<T>::type theValue(boost::intrusive_ptr<T> x);

template <> BigInt theValue<BigIntValue>(boost::intrusive_ptr<BigIntValue> x);
template <> const BigInt theValue<const BigIntValue>(boost::intrusive_ptr<const BigIntValue> x);

(...)

couldn't it be 
template <> const BigInt& theValue<const BigIntValue>(boost::intrusive_ptr<const BigIntValue> x);

Related issues

Related to CoCoA-5 - Slug #687: Builtin fn makes unnecessary copy of argClosed2015-04-19

Related to CoCoA-5 - Slug #1597: GetRow/GetRows are extraordinarily slowClosed2021-05-27

Related to CoCoA-5 - Slug #1228: SLUG: filling an arrayIn Progress2018-09-30

Related to CoCoA-5 - Feature #1112: New function: IsEmptyClosed2017-10-27

Related to CoCoA-5 - Bug #946: Function "ideal" evaluates the argument twiceResolved2016-10-17

History

#1 Updated by Anna Maria Bigatti over 12 years ago

  • Subject changed from theValue make copy to theValue makes copy

#2 Updated by Anna Maria Bigatti over 12 years ago

  • Category set to Parser/Interpreter

#3 Updated by Giovanni Lagorio about 12 years ago

  • % Done changed from 0 to 50

The solution is to implement a move constructor in BigInt (and every "huge" data structure ;-) )

#4 Updated by Anna Maria Bigatti almost 12 years ago

  • Status changed from New to In Progress

open until we make experiments with C++11

#5 Updated by Anna Maria Bigatti over 11 years ago

Giovanni Lagorio wrote:

The solution is to implement a move constructor in BigInt (and every "huge" data structure ;-) )

Giovanni: do you have a easy reference/example about implementing and using a move constructor?
Do I need to do it only for the BigInt class (say) or also for the cocoa-5 wrapper class (I guess not, since that is a pointer....)

#6 Updated by Anna Maria Bigatti about 10 years ago

  • Target version set to CoCoA-5.1.0 Easter14

#7 Updated by John Abbott about 10 years ago

  • Target version changed from CoCoA-5.1.0 Easter14 to CoCoA-5.?.?

#8 Updated by Anna Maria Bigatti over 3 years ago

  • Target version changed from CoCoA-5.?.? to CoCoA-5.4.2

#9 Updated by John Abbott almost 3 years ago

  • Related to Slug #1597: GetRow/GetRows are extraordinarily slow added

#10 Updated by Anna Maria Bigatti almost 3 years ago

  • Assignee set to Anna Maria Bigatti
  • Target version changed from CoCoA-5.4.2 to CoCoA-5.4.0
  • % Done changed from 50 to 60

The (template) function theValue makes a copy of

typename CoCoALibType<T>::type

so I changed it so that it returns
const typename CoCoALibType<T>::type&

and modified accordingly all its implementations... relying on CVS to recover the original code ;-)

Well, it works, all tests pass. Too good to be true?

#11 Updated by Anna Maria Bigatti almost 3 years ago

Now investigate why the code is split in Interpreter.H and BuiltInOneLiners.H

#12 Updated by John Abbott almost 3 years ago

  • Assignee deleted (Anna Maria Bigatti)
  • Target version changed from CoCoA-5.4.0 to CoCoA-5.4.2
  • % Done changed from 60 to 50
Current plan of action:
  • Anna will check in her changes (hinted at above)
  • Anna will make an interim release of the source (so that "alpha testers" can help) -- maybe wait about a month so that problems are likely to surface
  • Find out why the template fns are defined in BuiltinOneLiners.H but also declared in Interpreter.H; hopefully rectify
  • probably rename theValue to something indicating that it returns a reference.
  • investigate whether it makes sense to have almost duplicate template fns RefTo (if so, document the difference)

#13 Updated by John Abbott almost 3 years ago

  • Related to Slug #1228: SLUG: filling an array added

#14 Updated by John Abbott almost 3 years ago

#15 Updated by John Abbott almost 3 years ago

  • Assignee set to Anna Maria Bigatti
  • Target version changed from CoCoA-5.4.2 to CoCoA-5.4.0
  • % Done changed from 50 to 60

Correcting earlier mistake (when I forgot to reload the page)

#16 Updated by Anna Maria Bigatti almost 3 years ago

John Abbott wrote:

Current plan of action:
  • Find out why the template fns are defined in BuiltinOneLiners.H but also declared in Interpreter.H; hopefully rectify

I think this was because I implemented RefTo in BuiltinOneLiners.H, and they are very similar.
Now I moved the definition of theValue back to Interpreter.H and it works fine.

  • Anna will check in her changes (hinted at above)

done

  • Anna will make an interim release of the source (so that "alpha testers" can help) -- maybe wait about a month so that problems are likely to surface

done with "final" 5.3.3c

#17 Updated by Anna Maria Bigatti about 2 years ago

  • Target version changed from CoCoA-5.4.0 to CoCoA-5.4.2

#18 Updated by John Abbott about 1 year ago

  • Related to Bug #946: Function "ideal" evaluates the argument twice added

Also available in: Atom PDF