Skip to content

Check if the compiler supports std::filesystem#779

Merged
kris-rowe merged 2 commits intodevelopmentfrom
check_if_std_filesystem_is_supported
Jun 5, 2025
Merged

Check if the compiler supports std::filesystem#779
kris-rowe merged 2 commits intodevelopmentfrom
check_if_std_filesystem_is_supported

Conversation

@thilinarmtb
Copy link
Collaborator

@thilinarmtb thilinarmtb commented Feb 28, 2025

Description

We need std::filesystem support in the CXX compiler in order to compile OCCA.
Seems like some gcc versions even with -std=c++17 set doesn't support std::filesystem.

@thilinarmtb thilinarmtb changed the title Check if std filesystem is supported Check if the compiler supports std filesystem Feb 28, 2025
@thilinarmtb thilinarmtb changed the title Check if the compiler supports std filesystem Check if the compiler supports std::filesystem Feb 28, 2025
@thilinarmtb thilinarmtb force-pushed the check_if_std_filesystem_is_supported branch 2 times, most recently from 79b1b6a to 1db303a Compare March 4, 2025 01:46
@thilinarmtb thilinarmtb force-pushed the check_if_std_filesystem_is_supported branch 2 times, most recently from 5d802c5 to d1f2830 Compare March 25, 2025 18:46
@kris-rowe kris-rowe marked this pull request as draft May 29, 2025 21:02
@thilinarmtb thilinarmtb force-pushed the check_if_std_filesystem_is_supported branch 2 times, most recently from a668fd6 to 81a91ed Compare June 4, 2025 13:40
@thilinarmtb
Copy link
Collaborator Author

This PR fails on GitHub during CI since we have -Werror which forces all warnings to be
treated as errors (except for the exceptions we later specify) and gcc >= 13 finds a false
positive in the occa code:

/home/runner/work/occa/occa/src/c/device.cpp: In function ‘occaJson occaDeviceGetKernelProperties(occaDevice)’:
/home/runner/work/occa/occa/src/c/device.cpp:51:21: error: possibly dangling reference to a temporary [-Werror=dangling-reference]
   51 |   const occa::json &props = occa::c::device(device).kernelProperties();

One option is to just remove -Werror (we can still see all the warnings). Other option is to
convert the references to pointers (just copying without the references or pointers lead to
test failures and possibly could have performance impacts as well) with something like the
following:

diff --git a/include/occa/core/device.hpp b/include/occa/core/device.hpp
index b0461f23..4e1e8e5f 100644
--- a/include/occa/core/device.hpp
+++ b/include/occa/core/device.hpp
@@ -281,6 +281,7 @@ namespace occa {
      * @endDoc
      */
     const occa::json& properties() const;
+    const occa::json* propertiesPtr() const;

     /**
      * @startDoc{arch}
@@ -298,12 +299,15 @@ namespace occa {
     const std::string& arch() const;

     const occa::json& kernelProperties() const;
+    const occa::json* kernelPropertiesPtr() const;
     occa::json kernelProperties(const occa::json &additionalProps) const;

     const occa::json& memoryProperties() const;
+    const occa::json* memoryPropertiesPtr() const;
     occa::json memoryProperties(const occa::json &additionalProps) const;

     const occa::json& streamProperties() const;
+    const occa::json* streamPropertiesPtr() const;
     occa::json streamProperties(const occa::json &additionalProps) const;

     /**
diff --git a/src/core/device.cpp b/src/core/device.cpp
index 45fbdd42..f8dbb902 100644
--- a/src/core/device.cpp
+++ b/src/core/device.cpp
@@ -176,6 +176,11 @@ namespace occa {
     return modeDevice->properties;
   }

+  const occa::json* device::propertiesPtr() const {
+    assertInitialized();
+    return &(modeDevice->properties);
+  }
+
   const std::string& device::arch() const {
     static const std::string noArch = "No Arch";
     return (modeDevice
@@ -188,6 +193,11 @@ namespace occa {
     return (const occa::json&) modeDevice->properties["kernel"];
   }

+  const occa::json* device::kernelPropertiesPtr() const {
+    assertInitialized();
+    return (const occa::json*) &(modeDevice->properties["kernel"]);
+  }
+
   occa::json device::kernelProperties(const occa::json &additionalProps) const {
     return (
       kernelProperties()
@@ -200,6 +210,11 @@ namespace occa {
     return (const occa::json&) modeDevice->properties["memory"];
   }

+  const occa::json* device::memoryPropertiesPtr() const {
+    assertInitialized();
+    return (const occa::json*) &(modeDevice->properties["memory"]);
+  }
+
   occa::json device::memoryProperties(const occa::json &additionalProps) const {
     return (
       memoryProperties()
@@ -212,6 +227,11 @@ namespace occa {
     return (const occa::json&) modeDevice->properties["stream"];
   }

+  const occa::json* device::streamPropertiesPtr() const {
+    assertInitialized();
+    return (const occa::json*) &(modeDevice->properties["stream"]);
+  }
+
   occa::json device::streamProperties(const occa::json &additionalProps) const {
     return (
       streamProperties()
diff --git a/src/c/device.cpp b/src/c/device.cpp
index 475f3467..c424e1a8 100644
--- a/src/c/device.cpp
+++ b/src/c/device.cpp
@@ -39,8 +39,8 @@ const char* occaDeviceMode(occaDevice device) {
 }

 occaJson occaDeviceGetProperties(occaDevice device) {
-  const occa::json &props = occa::c::device(device).properties();
-  return occa::c::newOccaType(props, false);
+  const occa::json *props = occa::c::device(device).propertiesPtr();
+  return occa::c::newOccaType(*props, false);
 }

 const char* occaDeviceArch(occaDevice device) {
@@ -48,18 +48,18 @@ const char* occaDeviceArch(occaDevice device) {
 }

 occaJson occaDeviceGetKernelProperties(occaDevice device) {
-  const occa::json &props = occa::c::device(device).kernelProperties();
-  return occa::c::newOccaType(props, false);
+  const occa::json *props = occa::c::device(device).kernelPropertiesPtr();
+  return occa::c::newOccaType(*props, false);
 }

 occaJson occaDeviceGetMemoryProperties(occaDevice device) {
-  const occa::json &props = occa::c::device(device).memoryProperties();
-  return occa::c::newOccaType(props, false);
+  const occa::json *props = occa::c::device(device).memoryPropertiesPtr();
+  return occa::c::newOccaType(*props, false);
 }

 occaJson occaDeviceGetStreamProperties(occaDevice device) {
-  const occa::json &props = occa::c::device(device).streamProperties();
-  return occa::c::newOccaType(props, false);
+  const occa::json *props = occa::c::device(device).streamPropertiesPtr();
+  return occa::c::newOccaType(*props, false);
 }

 occaUDim_t occaDeviceMemorySize(occaDevice device) {

@thilinarmtb thilinarmtb self-assigned this Jun 4, 2025
@thilinarmtb thilinarmtb marked this pull request as ready for review June 4, 2025 16:41
@kris-rowe
Copy link
Member

It seems like pointers are the way to go, especially since the underlying C struct is storing the reference as a pointer anyway.
We should address this in a separate PR as a way of documenting things.

@thilinarmtb thilinarmtb force-pushed the check_if_std_filesystem_is_supported branch from 81a91ed to c25f2c6 Compare June 5, 2025 03:41
@thilinarmtb
Copy link
Collaborator Author

It seems like pointers are the way to go, especially since the underlying C struct is storing the reference as a pointer anyway. We should address this in a separate PR as a way of documenting things.

Fixed in #783. Seems like the usage of pointers wasn't necessary as we could simply avoid using the temporary variable.

@thilinarmtb thilinarmtb force-pushed the check_if_std_filesystem_is_supported branch from c25f2c6 to 4e4027e Compare June 5, 2025 20:39
@kris-rowe kris-rowe merged commit 66a6b20 into development Jun 5, 2025
4 checks passed
@thilinarmtb thilinarmtb deleted the check_if_std_filesystem_is_supported branch June 6, 2025 00:13
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.

2 participants