Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Testing of various C++20 features #1207

Closed
wants to merge 49 commits into from
Closed

Testing of various C++20 features #1207

wants to merge 49 commits into from

Conversation

robertlipe
Copy link
Collaborator

A little bit of everything just to see how well C++20 is supported. This is just
a few commits for each major feature. If there's anything useful in this PR, I expect
to have to pick it apart and cherry pick as I can't imagine this whole PR going in as
one.

See related discussion on #1204.

  • Templatize FahrenheitToCelsius and inverse.
  • Toe dip for C++-20: use host endianness.
  • unicsv.cc
  • Babys's first concept.
  • Use enum class and C++20 'using' directive in xcsv.
  • use using enum in units to reduce chatter
  • DANGER: wreckless use of std::format for testing

GPSBabelDeveloper and others added 13 commits January 30, 2021 15:58
Now they DTRT without excessive promotion for ints, doubles, floats, etc...
Barely an improvement and still falls back to Qt if we don't have <bit>,
but this is just a proving ground.
The errors actually ARE easier to read. Sure, it's a lot of words in
this training-wheels case.

gpsbabel/garmin_txt.cc:974:22: error: no matching function for call to 'FahrenheitToCelsius'
      *temperature = FahrenheitToCelsius("booger");
                     ^~~~~~~~~~~~~~~~~~~
gpsbabel/defs.h:92:3: note: candidate template ignored: constraints not satisfied [with T = const char *]
T FahrenheitToCelsius(T a) { return (a - 32.0) / 1.8;}
  ^
gpsbabel/defs.h:91:10: note: because 'const char *' does not satisfy 'integral'
requires std::integral<T> || std::floating_point<T>
         ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.0.sdk/usr/include/c++/v1/__concepts/arithmetic.h:30:20: note: because 'is_integral_v<const char *>' evaluated to false
concept integral = is_integral_v<_Tp>;
                   ^
gpsbabel/defs.h:91:30: note: and 'const char *' does not satisfy 'floating_point'
requires std::integral<T> || std::floating_point<T>
                             ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.0.sdk/usr/include/c++/v1/__concepts/arithmetic.h:39:26: note: because 'is_floating_point_v<const char *>' evaluated to false
concept floating_point = is_floating_point_v<_Tp>;
Better type safety from enum class. Less annoying repetition via using.
@robertlipe robertlipe mentioned this pull request Nov 2, 2023
fmt.h Outdated
} ;

// It's present everywhere else, right?
#elif __has_include(<format.h>)
Copy link
Collaborator

@tsteven4 tsteven4 Nov 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The standard header is <format>, not <format.h>.
https://en.cppreference.com/w/cpp/header/format

@tsteven4
Copy link
Collaborator

tsteven4 commented Nov 3, 2023

You had a bad merge on xcsv.cc. I think this is what you meant:
xcsv.zip


// Fallback to the system version.
#elif __has_include(<format>)
# include <format.h>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, the system version is<format>

@tsteven4
Copy link
Collaborator

tsteven4 commented Nov 3, 2023

With the xcsv.cc I attached above, and actually including <format> after testing for it, on visual studio I get down to 7 errors, all in the MS supplied format include.

Severity	Code	Description	Project	File	Line	Suppression State
Error	C2338	static_assert failed: 'Cannot format an argument. To make type T formattable, provide a formatter<T> specialization. See N4928 [format.arg.store]/2 and [formatter.requirements].'	C:\Users\tstev\Source\Repos\upstream\out\build\x64-Debug\upstream	C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.37.32822\include\format	3648	
Error	C2665	'std::_Format_arg_traits<_Context>::_Phony_basic_format_arg_constructor': no overloaded function could convert all the argument types	C:\Users\tstev\Source\Repos\upstream\out\build\x64-Debug\upstream	C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.37.32822\include\format	1873	
Error	C2993	'unknown-type': is not a valid type for non-type template parameter '_Test'	C:\Users\tstev\Source\Repos\upstream\out\build\x64-Debug\upstream	C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.37.32822\include\format	3430	
Error	C2641	cannot deduce template arguments for 'std::formatter'	C:\Users\tstev\Source\Repos\upstream\out\build\x64-Debug\upstream	C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.37.32822\include\format	3432	
Error	C2783	'std::formatter<_Ty,_CharT> std::formatter(void)': could not deduce template argument for '_Ty'	C:\Users\tstev\Source\Repos\upstream\out\build\x64-Debug\upstream	C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.37.32822\include\format	3432	
Error	C2780	'std::formatter<_Ty,_CharT> std::formatter(std::formatter<_Ty,_CharT>)': expects 1 arguments - 0 provided	C:\Users\tstev\Source\Repos\upstream\out\build\x64-Debug\upstream	C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.37.32822\include\format	3432	
Error	C2039	'parse': is not a member of 'std::formatter'	C:\Users\tstev\Source\Repos\upstream\out\build\x64-Debug\upstream	C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.37.32822\include\format	3433	

@robertlipe robertlipe mentioned this pull request Aug 9, 2024
* de-duplicate format and filter vector display code.

* consolidate release of argument memory in vecs, filter_vecs.
* refactor unicsv date time handling.

* restrict utc option range to match Qt offsetfromutc

* move xcsv from C-style legacy time to Qt.

* fill in lower order date/time fields when parsing.

* don't return date/time if we don't have one!

* enhance xcsv date/time testing, fix bug.

* csv format date time adjustements.

use QDateTime::fromString to parse iso date times.
return invalid QTime from addhms if parsing fails entirely.

* datetime display fixes.

For the xcsv writer:
avoid priting date/time fields when there isn't a valid date/time.
when printing with am/pm use times from a 12 hour clock.
add support for printing dotnet time.
use QDateTime::toString to print ISO time.
For the xcsv and unicsv readers:
use QDateTime::fromString instead of xml_parse_time when reading
ISO datetimes.  This avoids xml_parse_times intentional odd behavior
of treating non-timezoned times as UTC.

add a test of the xcsv writer time related fields.  This only runs
in America/Denver time zone.  As setting the time zone is system
dependent the test only runs if tzselect is available.  If so it
assumes America/Denver is available.

correct documentation to give a sensible format for xcsv field GMT_TIME.

* add reference files for new test.

* silence xcsv reader conversion warnings on empty strings.

* warn on parse errors reading excel time.

* xcsv date/time fixes.

fix addhms to account for 12/24 hour clock.
don't print invalid datetimes with iso_time, iso_time_ms.

add testcases to exercise all the xcsv reader date/time flavors.

* clarify HMSL, HMSG wrt 12/24 hour clocks.

* add missing reference file.

* enhance xcsv for HMS[L|G] before or after [LOCAL|GMT]_TIME

* restore YYYYYMMDD to use UTC.

This has been broken for some time.  Mail from 2012 indicates the intent
was UTC (https://sourceforge.net/p/gpsbabel/mailman/message/29544538/)

* unicsv review catches

don't ripple into trouble with msec rounding.
pass outputs that may or may not be written as references.

* retire xcsv fields HMSG_TIME, HMSL_TIME.

This is potentially a user visible change.  It could require
users to rewrite any style files they have created that use
these fields.

These are replaced by repeated use of GMT_TIME and LOCAL_TIME.
This eliminates our pain over 12/24 hour clock issues. strptime/
strftime (as well as QTime) have distinct conversion specifiers
for hours with 12/24 hour clocks.  Our support for HMSG_TIME,
HMSL_TIME used integer conversion specifiers for hours, minutes
and seconds.  This made it difficult to decide if a 12 or 24 hour
clock should be used, and made it impossible to have reduced
precision values with 12 hour clock using an AM/PM designation.
We also always printed AM/PM designations.

* fix whitespace in serialization reference file
* move Qt floor to 5.15.

* fix focal, coverage, windows 32 bit builds.

* try harder to fix coverage build

* fix codeql build

* kill fedora 32 CI build which uses Qt 5.14.
* use container for garmin_fs ilinks.

and leave memory management up to the copy on write container.

* make gmsd const for read only usage.

this prevents the possiblity of detachment of the ilinks list.

* incorporate some variables and functions into garmin_fs_t

* unify garmin category conversions.

* fiddle with test time zone sensitivity.

* add utc option for garmin_txt reader.

* correct sign of adjustment

* update garmin_txt includes.
* update some builds to Qt 6.5.1

* conditional code for QVersionNumber evolution
tsteven4 and others added 25 commits August 11, 2024 21:58
* fix humminbird icon matching,

eliminating one use of xasprintf.
This was broken in 6029c82 over a decade ago.

* eliminate unnecessary xasprinf usage.

* eliminate another xasprintf usage.

* eliminate xasprintf usage.

* retire xasprintf, xvasprintf.
As discussed in
#1090
these units have long since been deprecated by their maker. We have no
known remaining users. Numerous attempts (including  phone calls and
attempt to contact via the windsurfing community) to reach the last
known users have all failed.

Buh Bye.
* reduce complexity of mkshort from O(n^2) to O(n).

* add converting constructor for mkshort hash key
* use QHash for gdb waypoint searches.

This reduces the complexity from O(n^2) to O(n).

For the reader two parallel hashes are maintained with different
keys but identical values.  This is necessary because sometimes
we want to match the name and position, and other times we just want
to match the name.

* workaround missing qHashMulti in Qt5.

and supply missing default for our qHash functions.

* fix MSVC C2666 errors due to an erroneous assumption.

In Qt6 the return type of qHash, and the type of the seed,
happens to be the same as QHash::size_type.
However, this isn't true in Qt5.
* improve correctness and speed of duplicate filter.

The duplicate filter could errouneously delete points that were
not duplicates if the crc's happened to match.

waypt_del(Waypoint*) is inefficent as it requires a search of the
list to find the matching waypoint.  Support waypt_del with iterators.

* retire util_crc.cc

* improve duplicate to linear complexity

* polish new list creation.

* Remove final remnants of 'exported'

* Revert "Remove final remnants of 'exported'"

This reverts commit 6996e4d.

---------

Co-authored-by: Robert Lipe <[email protected]>
1. With the length option the last point deleted took the total error
over the specified limit.

2. When computing the total error if another point is deleted it was
possible to refer to an xte record that needed to be updated due to
the deletion of one of its neighbors.
* fix bugs with simplify filter

1. With the length option the last point deleted took the total error
over the specified limit.

2. When computing the total error if another point is deleted it was
possible to refer to an xte record that needed to be updated due to
the deletion of one of its neighbors.

* wip on reduced complexity simplify filter.

* mimic ineheitence of new_trkseg flag in simplify filter.

* cleanup simplify header.

* work around apparent Qt5 QMap bug.

* fix shadowing issues.

* astyle

* correct spelling.

* kill Waypoint route_priority.
* Introduce efficient member functions for deleting waypoints

New functions with complexity O(n) for deleting pre-marked
waypoints are:
  WaypointList::del_wpts
  RouteList::del_wpts
  del_wpts
  route_del_wpts
  track_del_wpts

Use those functions instead of the inefficient versions
WaypointList::waypt_del
RouteList::del_wpt
waypt_del
route_del_wpt
track_del_wpt.
When these functions are using while looping over a waypoint list
the overall complexity is O(n^2).  This is because these functions
themselves amortize to O(n).

* review nits.

* tweak deletes

* use dedicated wpt deletion flag.
)

and add a test case that demonstrates the issue.
* use efficient methods to delete trkpts in track filter.

* clean up some code issues flagged by windows tools.
* tweak greatcircle conversions.

* modify arc-project test to use microdegrees instead of nanodegrees.

* kill unused metric prefix conversion macros.
* add codacy clang-tidy to workflows

* attempt to run clang-tidy on ci

* update jammy image.

get rid of ppa
add clang-tidy, jq.

* fixes.

* whittle down tidy checks.

* kick out another tidy warning
* Add support for Google Takeout Location History

This adds support for parsing the location history provided by Google Takeout.
This makes it possible to convert a month's, year's, or your entire location
history at once instead of having to export one day's history at a time on
Google Maps.

There is documentation in the `xmldoc` folder with some examples.

It works great, but there are one, possibly two, other additions I'd like to
make in following diffs:

1. Add the ability to select date ranges. For example, I went on a road trip
   that started in the last week of February, but right now I can only select
	 tracks at the resolution of a whole month.

2. Add support for the higher-resolution "roadSegment" blocks in newer
   "activitySegment" blocks. This will have to be optional as you have to
	 query the Google Maps API to get lat/long, which costs money. However,
	 if you're willing to pay for it, you can get great detail out of it.
	 They look like this:

```
       "roadSegment": [{
          "placeId": "ChIJBdXmuuWaK4gRoiClERhZcy8",
          "duration": "76s"
        }, {
          "placeId": "ChIJ4ekNlOWaK4gRxu-moMRFo60",
          "duration": "58s"
        }, {
          "placeId": "ChIJ10qEheWaK4gRQ1T24wEuuN4",
          "duration": "58s"
        }, {
          "placeId": "ChIJe7NWfu-aK4gRFHBB6P0PMe4",
          "duration": "58s"
[...]
```

* rename variable

* this TODO is done

* now builds on focal, fix documentation

* patch -p2 < ~/testo.log

* use `&&` instead of `and` for old compilers

* don't leak objects when we skip events at exact lat/lon 0/0

* use a reference (even though the compiler would have optimized that away)

* rename to googletakeout

* fix docs

* Address code review requests

* add `googletakeout.h` to HEADERS
* convert most `#define`s to static members
* don't pollute global namespace
* rename logging functions
* convert some methods to static functions for clang-tidy
* `title_case` loop by reference instead of by index (clang-tidy)
* better use of debugging levels
* drop dead simplifiedRawPath code for now
* drop empty tracks
* default constructor for GoogleTakeoutInputStream
* add some tests that check the content of the data

Also, I found some unofficial documentation about the location history
format and added that (thanks @CarlosBergillos !)

* apply @tsteven4 's patch. thank you!!

* add license

* fix logging

* As I assigned the correct license to my code, I am skeptical that this makes a difference, but give Robert co-copyright.

* make the test more boring

* rm xml_slice

* revert constructor change from @tsteven4 's patch

* apply @tsteven4 's latest constructor

* apply @tsteven4 's latest patch -- `cmake --build . --target check` passes

---------

Co-authored-by: tsteven4 <[email protected]>
The declaration appeared in 6a9a058, 7/25/2011, without a definition
or usage.
* fix mkshort unique.

1. we were off by one when deciding if we could just concatentate
the generated suffix to the name, failing to use the last available
character.
2. when attempting to copy the generated suffix to a presumably
shortened name, and the generated suffix was longer than the name,
the target of the copy was outside the name buffer.  In the common
case that the rank of size_t was greater than the rank of int, the
target was well beyond the end of name, not before as one would expect
with signed arithmetic.

With the new algorithm when the target length is insufficient to
fit both the name and the suffix we will only truncate the name as
required to fit the truncated name and the complete suffix in the
target length(as opposed to the original length of the name).
We fatal if this is not possible.

Add a test case to exercise the make unique code.

* fix reference mode.

* add notes for future enhancements

* fix testcase cut and paste booboo

* refactor mkshort input from char* to QByteArray.
* convert mkshort from char* to QByteArray.

and store conflict count directly in hash.

* fix potential detach

and rename vars

* tweak whitespace removal.

* don't split codepoints that comprise a grapheme.

* enhance mkshort test

* fix enhanced test

* test mkshort replacements

* remformat mkshort with astyle

* add mkshort test to cmake
This process was fairly disasterous and I epect to be mopping bloood off
the walls for a while.
@robertlipe
Copy link
Collaborator Author

Actually, after I finally got through the merge noise in all this and could whwat was outstatnding, there's nothing to see here.

c++20, it is. Case closed.

@robertlipe robertlipe closed this Aug 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants