search

SpartanJ / efsw

efsw is a C++ cross-platform file system watcher and notifier.
Other
645 stars 98 forks source link

Nested template syntax breaks LEGACY builds #167

Closed solarispika closed 10 months ago

solarispika commented 10 months ago

The use of double angle brackets (>>) for nested templates in commit 1deac78f8c5cff6921db2ac9ac7ab8b34db6371f prevents the project from building with C++ standards earlier than C++11 when the LEGACY CMake option is used.

Additionally, commit 2c29a328d78548cf8443c96652caaa966840fada introduced clang-format with a default C++11 standard, which also formats double angle brackets into the C++11 style.

Together, these changes break existing builds that rely on LEGACY to maintain pre-C++11 compatibility. The LEGACY option is currently misleading, as specifying it no longer allows building with pre-C++11 compilers or --std=c++03.

We should decide whether to continue supporting pre-C++11 standards by:

Or only support C++11 and later standards moving forward.

The latter would simplify the codebase, but permanently break LEGACY builds that need pre-C++11 compatibility. We should weigh the tradeoffs and decide on the best path forward.

Please share your thoughts on how we should handle the LEGACY build breakage introduced by the nested template syntax change. I'm happy to update the issue description once we align on an approach.

SpartanJ commented 10 months ago

Hi! If there's a demand let's simply fix those issues since they seem to require minor fixes. I kind of assumed that no one would be using such an old GCC version so at some point I stopped being careful about it. But I understand that you need that compatibility, right? What's the minimum version you need to be supported?

solarispika commented 10 months ago

To be honest, I don't have a strong need to use old compilers myself. I've just been fixing some bugs and will submit a PR later. During this work I noticed the LEGACY option when trying to use C++11 features.

If maintaining the LEGACY option is no longer a priority for the project, I'm happy to help remove it and do some cleanup in a pull request. Let me know if taking out that obsolete option would be helpful.

I don't have a stake in keeping legacy compatibility - just thought I'd raise the issue since the option wasn't working as expected. If the team decides supporting pre-C++11 is more trouble than it's worth, I can assist with removing LEGACY and modernizing the codebase.

SpartanJ commented 10 months ago

Thanks for offering your collaboration. I'm trying to think for a good reason to maintain a pre C++11 version and I honestly don't find any, I remember doing that because there was a single user using an old NAS Linux installation that required support for an old GCC version and old Linux version but it was really long ago. If you want to remove the legacy option please feel free to do it. Thank you very much!