EamonNerbonne / anoprsst

Sorts Span<T> and arrays more quickly than Array.Sort
23 stars 4 forks source link

OrderedAlgorithms.cs won't compile on 2.2 #7

Closed BillSobel closed 5 years ago

BillSobel commented 5 years ago

Two precompiler #if statements check for NETCOREAPP2_1 || NET47, both need NETCOREAPP2_2 added.

EamonNerbonne commented 5 years ago

So, you're probably refering to these statements: https://github.com/EamonNerbonne/anoprsst/blob/25625fc1576d03fc325a8343c4d15ade0a4f2724/src/Anoprsst/OrderedAlgorithms.cs#L141

Those are in the anoprsst project; and that's compiled using the following project file: https://github.com/EamonNerbonne/anoprsst/blob/25625fc1576d03fc325a8343c4d15ade0a4f2724/src/Anoprsst/Anoprsst.csproj#L13

i.e. it's set to cross-compile using .net framework 4.7, .net core 2.1, and .net core 3.0 - not .net core 2.2. The why here being availability of a minor optimization in .net core 3.0 but not 2.2. AFAIK the dotnet build system should be automatically picking the right SDK for you, but presumably it's not?


TL;DR: by design, OrderedAlgorithms.cs should not compile on 2.2; instead it compiles on 2.1 (which can be referenced by .net core 2.2 projects)

Under what circumstances are you seeing a bug (and what bug are you seeing exactly?)

BillSobel commented 5 years ago

Hi. I had reported the bug with the sorters defaulting to new instance which you just closed. When debugging I just imported your code into a 2.2 project, figured it was written earlier and the missing 2.2 label was because you hadn't upgraded to that version yet. Issue resolves when I go back to the new version you posted via NuGet. Thanks!

EamonNerbonne commented 5 years ago

It's a bit annoying that there's no way in a preprocessor to pragmatically detect a range of versions, but oh well.

The idea was to main as broad as possible support, so sticking with the 2.1 LTS (since requiring 2.2 doesn't get you anything anyhow - I makes sense to run 2.2., but that's a different story).

Anyhow, thanks for the feedback!