View Issue Details

IDProjectCategoryView StatusLast Update
0035844FPCRTLpublic2020-10-20 15:27
Reporterrd0x Assigned To 
PrioritynormalSeverityminorReproducibilityN/A
Status newResolutionopen 
Product Version3.3.1 
Summary0035844: Support for TTimeSpan record
DescriptionAdd missing TimeSpan unit from Delphi
Additional Informationhttp://docwiki.embarcadero.com/Libraries/Rio/en/System.TimeSpan.TTimeSpan
TagsNo tags attached.
Fixed in Revision
FPCOldBugId
FPCTarget
Attached Files

Relationships

related to 0036705 new Add TStopwatch record 

Activities

Thaddy de Koning

2019-07-15 09:14

reporter   ~0117263

Last edited: 2019-07-15 09:18

View 3 revisions

The Delphi implementation is flawed.
  TTimeSpan = record
  private
    FTicks: Int64;

Whereas the rest is strict...as it should be for the above too.

From the declaration alone, you can already see it is half a job. I wonder how this could make production.
That doesn't mean we should not implement it (not too difficult), but the Delphi code is everything except clean.

Thaddy de Koning

2019-07-23 14:30

reporter   ~0117352

Last edited: 2019-07-23 14:35

View 2 revisions

Good job and my tests worked! works on armhf-linux too (3.2.0 and 3.3.1-r42460)

There may be some point - for the time being - to solve the compatibility issue 6 with using abs() in Delphi modes.

rd0x

2019-07-23 14:55

reporter   ~0117353

@Serge
wow - nice work!

@Thaddy
maybe add your tests so that they could be added to testsuite :)

NoName

2020-05-08 19:35

reporter   ~0122673

Any chance to get that added? With the tests from Thaddy?

Michael Van Canneyt

2020-08-06 10:52

administrator   ~0124610

I'm sorry, but this is clearly an edited copy of the Delphi unit.
The ToString method is a verbatim copy. The GetScaledInterval as well.

So I'm forced to remove these files from the bugtracker.

Please don't publish copied code from Delphi.

NoName

2020-08-06 21:06

reporter   ~0124628

Abs100NanoSecs: Int64; // Positive only
  AbsDaysInSpan: Integer; // Positive only
  AbsPartOfDay: Int64; // Positive only
  AbsPartOfSec: Integer; // Positive only
Why not using unsigned types? -> Cardinal & QWord or Uint32/UInt64

NoName

2020-08-06 21:19

reporter   ~0124629

Btw TTimeSpan.GetScaledInterval is public anyway: https://en.delphipraxis.net/topic/303-delphi-103-ttimespangetscaledinterval-android/

Michael Van Canneyt

2020-08-06 22:32

administrator   ~0124631

@NoName: Whether it is public or not is irrelevant.
It's copyrighted code.
We had problems in the past with Embarcadero, we're extremely careful of what we accept, we no longer want the hassle of having to retract code.

@Serge, sorry, but we can't accept code from you for this: you clearly saw the implementation, no amount of editing will help.

We need a clean-room implementation.

Serge Anvarov

2020-08-07 13:36

reporter   ~0124641

To Michael: Of course, I have seen the implementation, I think this can be said about everyone who wrote RTL. But the specific implementation is different. For example, the string parser is completely different and fixes errors that exist in Delphi (how would I know about them without seeing the implementation). This also applies to range checking and exception generation. And I think the source code analyzer is unlikely to decide that these sources are similar.
To NoName: I tried to avoid side effects when converting types. Variables take values from functions that return signed integers. Some of the variables involved in expressions with the sign, why risk the outcome?

Bart Broersma

2020-08-07 14:47

reporter   ~0124642

> how would I know about them without seeing the implementation
Black box testing?

Abou Al Montacir

2020-08-07 15:24

manager   ~0124643

@Michael: As far as I can say, this kind of code can not be included in Debian, so please don't get this patch.

Jonas Maebe

2020-08-07 17:45

manager   ~0124646

> And I think the source code analyzer is unlikely to decide that these sources are similar.
Copyright law does not care about source code analysers. If you first look at their code, you are tainted. It's possible that if it came to a court case, the judge/jury would still decide that your implementation and train of thought are different enough for it to be considered a completely unrelated work, but if even 1% is considered to belong to Embarcadero, FPC is screwed. In practice, it would probably never come to a court case, but just to a lot of bad PR for the FPC project if some blogger would pick it up and start posting about how we don't care about copyright.

Serge Anvarov

2020-08-07 18:41

reporter   ~0124647

I think that if you take someone now, show only the interface and ask them to implement it, then 80% of the code will repeat what it is (it's just the most obvious solution), and implement the rest yourself. And most likely the overall solution will be even more similar to the Delphi code.

Jonas Maebe

2020-08-07 18:57

manager   ~0124648

Last edited: 2020-08-08 16:08

View 4 revisions

That's the difference between patent law and copyright law:
* in copyright law, independent creation is all that's needed to be in the clear. It doesn't matter how similar the end results are.
* in patent law, all that matters is how you do it (the algorithm in case of code). It doesn't matter whether you first studied the other implementation or not (except that if you did, you may be liable for higher damages).

In this case, copyright law applies (while Embarcadero probably holds software patents, but it's unlikely any of them cover the TTimespan record implementation).

Michael Van Canneyt

2020-08-08 14:03

administrator   ~0124667

@Serge,

We discussed this in the core team. All participants are agreed we unfortunately cannot accept your code.
 
But you can help by creating a testsuite which someone can then use to create an independent implementation, including corrections, exceptions and whatnot.
You are now exceptionally well placed to do so.

Bart Broersma

2020-08-08 15:42

reporter   ~0124670

Can we copy the whole record definition (normally only the non-private interface is copied)?
If not, nobody who looks at the refrence DocWiki page can implement it anymore...

Jonas Maebe

2020-08-08 15:49

manager   ~0124671

Copyright only applies to creative works. API definitions are generally not considered to be creative works, although Oracle is trying its best to create case-law to overturn that in its lawsuit against Google about Java/Android.

So yes, copying the record definition (without any related comments) should be okay.

Bart Broersma

2020-08-08 17:50

reporter   ~0124673

OK then, we can start "hacking" our way.
And yes, we would need a test suite (preferrably stand alone for the moment) for black box testing.

Janex

2020-10-20 14:22

reporter   ~0126422

Hi.
And where can I download TimeSpan.pp added by Serge Anvarov ?

WBR
Janex

Michael Van Canneyt

2020-10-20 15:27

administrator   ~0126425

You can't, I had to delete it as it violates copyright.

Issue History

Date Modified Username Field Change
2019-07-13 11:18 rd0x New Issue
2019-07-15 09:14 Thaddy de Koning Note Added: 0117263
2019-07-15 09:17 Thaddy de Koning Note Edited: 0117263 View Revisions
2019-07-15 09:18 Thaddy de Koning Note Edited: 0117263 View Revisions
2019-07-23 11:01 Serge Anvarov File Added: TimeSpan.pp
2019-07-23 11:03 Serge Anvarov File Added: TimeSpan-2.pp
2019-07-23 12:58 Marco van de Voort File Deleted: TimeSpan.pp
2019-07-23 14:30 Thaddy de Koning Note Added: 0117352
2019-07-23 14:35 Thaddy de Koning Note Edited: 0117352 View Revisions
2019-07-23 14:55 rd0x Note Added: 0117353
2020-02-18 07:40 Sven Barth Relationship added related to 0036705
2020-05-08 19:35 NoName Note Added: 0122673
2020-08-06 10:52 Michael Van Canneyt Note Added: 0124610
2020-08-06 10:52 Michael Van Canneyt File Deleted: TimeSpan-2.pp
2020-08-06 21:06 NoName Note Added: 0124628
2020-08-06 21:19 NoName Note Added: 0124629
2020-08-06 22:32 Michael Van Canneyt Note Added: 0124631
2020-08-07 13:36 Serge Anvarov Note Added: 0124641
2020-08-07 14:47 Bart Broersma Note Added: 0124642
2020-08-07 15:24 Abou Al Montacir Note Added: 0124643
2020-08-07 17:45 Jonas Maebe Note Added: 0124646
2020-08-07 18:41 Serge Anvarov Note Added: 0124647
2020-08-07 18:57 Jonas Maebe Note Added: 0124648
2020-08-07 18:57 Jonas Maebe Note Edited: 0124648 View Revisions
2020-08-08 14:03 Michael Van Canneyt Note Added: 0124667
2020-08-08 15:42 Bart Broersma Note Added: 0124670
2020-08-08 15:49 Jonas Maebe Note Added: 0124671
2020-08-08 16:08 Jonas Maebe Note Edited: 0124648 View Revisions
2020-08-08 16:08 Jonas Maebe Note Edited: 0124648 View Revisions
2020-08-08 17:50 Bart Broersma Note Added: 0124673
2020-10-20 14:22 Janex Note Added: 0126422
2020-10-20 15:27 Michael Van Canneyt Note Added: 0126425