Move checks in CladFunction to compile-time #1090

MihailMihov · 2024-09-11T22:46:12Z

No description provided.

github-actions

clang-tidy made some suggestions

github-actions · 2024-09-11T23:00:19Z

include/clad/Differentiator/Differentiator.h

-      }
+        : m_Function(f), m_Functor(functor), m_CUDAkernel(CUDAkernel) {
+#ifndef __CLAD_SO_LOADED
+      static_assert(false, "clad doesn't appear to be loaded; make sure that "


warning: static assertion failed: clad doesn't appear to be loaded; make sure that you pass clad.so to clang. [clang-diagnostic-error]

static_assert(false, "clad doesn't appear to be loaded; make sure that " ^

github-actions · 2024-09-11T23:00:19Z

include/clad/Differentiator/Differentiator.h

+#endif
+
+      size_t length = GetLength(code);
+      char* temp = (char*)malloc(length + 1);


warning: do not manage memory manually; consider a container or a smart pointer [cppcoreguidelines-no-malloc]

char* temp = (char*)malloc(length + 1); ^

github-actions · 2024-09-11T23:00:20Z

include/clad/Differentiator/Differentiator.h

+#endif
+
+      size_t length = GetLength(code);
+      char* temp = (char*)malloc(length + 1);


warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]

char* temp = (char*)malloc(length + 1); ^

github-actions · 2024-09-11T23:00:20Z

include/clad/Differentiator/Differentiator.h

+#endif
+
+      size_t length = GetLength(code);
+      char* temp = (char*)malloc(length + 1);


warning: initializing non-owner 'char *' with a newly created 'gsl::owner<>' [cppcoreguidelines-owning-memory]

char* temp = (char*)malloc(length + 1); ^

github-actions · 2024-09-11T23:00:20Z

include/clad/Differentiator/Differentiator.h

+      size_t length = GetLength(code);
+      char* temp = (char*)malloc(length + 1);
+      m_Code = temp;
+      while ((*temp++ = *code++))


warning: do not use pointer arithmetic [cppcoreguidelines-pro-bounds-pointer-arithmetic]

while ((*temp++ = *code++)) ^

github-actions · 2024-09-11T23:00:20Z

include/clad/Differentiator/Differentiator.h

+      size_t length = GetLength(code);
+      char* temp = (char*)malloc(length + 1);
+      m_Code = temp;
+      while ((*temp++ = *code++))


warning: do not use pointer arithmetic [cppcoreguidelines-pro-bounds-pointer-arithmetic]

while ((*temp++ = *code++)) ^

codecov · 2024-09-17T23:19:01Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.28%. Comparing base (e2b8e35) to head (efa67b3).
Report is 5 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1090      +/-   ##
==========================================
+ Coverage   94.26%   94.28%   +0.02%     
==========================================
  Files          55       55              
  Lines        8445     8441       -4     
==========================================
- Hits         7961     7959       -2     
+ Misses        484      482       -2

Files with missing lines	Coverage Δ
include/clad/Differentiator/Differentiator.h	`86.11% <100.00%> (+6.11%)`	⬆️
tools/ClangPlugin.cpp	`95.62% <100.00%> (+0.04%)`	⬆️

... and 2 files with indirect coverage changes

Files with missing lines	Coverage Δ
include/clad/Differentiator/Differentiator.h	`86.11% <100.00%> (+6.11%)`	⬆️
tools/ClangPlugin.cpp	`95.62% <100.00%> (+0.04%)`	⬆️

... and 2 files with indirect coverage changes

vgvassilev · 2024-09-18T17:19:19Z

test/ForwardMode/ConstevalTest.C

@@ -0,0 +1,58 @@
+// RUN: %cladclang %s -I%S/../../include -std=c++23 -oConstevalTest.out | %filecheck %s
+// RUN: ./ConstevalTest.out | %filecheck_exec %s
+// XFAIL: *


Why the tests are marked with XFAIL? Can we make a test that makes sure this does not regress again into some runtime evaluation?

The changes that would allow constexpr/consteval are not in this pr so I disabled them and we can enable them in that pr, or should I remove them from here?

In that case we should not add the tests as part of this pr.

vgvassilev

LGTM!

vgvassilev · 2024-09-28T18:21:31Z

tools/ClangPlugin.cpp

@@ -90,6 +90,12 @@ namespace clad {
        CGOpts.PassPlugins.push_back(CladSoPath.str());
      }
 #endif // CLANG_VERSION_MAJOR > 8
+
+      // Add define for CLAD_SO_LOADED, so that CladFunction::CladFunction()


Suggested change

// Add define for CLAD_SO_LOADED, so that CladFunction::CladFunction()

// Add define for __CLAD_SO_LOADED, so that CladFunction::CladFunction()

Can you make that change and put it in the right place in the commit history.

These asserts will only assert if clad has malfunctioned, which won't fix anything. Also there isn't a way to keep them while making the methods constexpr.

MihailMihov force-pushed the clad-function-constexpr branch from 5f143db to c15ec08 Compare September 11, 2024 22:51

github-actions bot reviewed Sep 11, 2024

View reviewed changes

vgvassilev force-pushed the clad-function-constexpr branch from 1b944d2 to d7a3d63 Compare September 18, 2024 14:22

MihailMihov force-pushed the clad-function-constexpr branch 2 times, most recently from 8484807 to af905d1 Compare September 18, 2024 15:42

vgvassilev force-pushed the clad-function-constexpr branch from af905d1 to cc3518e Compare September 18, 2024 17:17

vgvassilev reviewed Sep 18, 2024

View reviewed changes

MihailMihov force-pushed the clad-function-constexpr branch from cc3518e to e86a47d Compare September 23, 2024 07:36

MihailMihov marked this pull request as ready for review September 28, 2024 14:37

MihailMihov force-pushed the clad-function-constexpr branch from e86a47d to 710b779 Compare September 28, 2024 14:38

vgvassilev approved these changes Sep 28, 2024

View reviewed changes

MihailMihov added 3 commits September 29, 2024 15:41

Check if clad.so is loaded with macro

5dc8a8b

Remove asserts that are not related to user-errors

a2fc661

These asserts will only assert if clad has malfunctioned, which won't fix anything. Also there isn't a way to keep them while making the methods constexpr.

Run tests with c++14

efa67b3

MihailMihov force-pushed the clad-function-constexpr branch from 710b779 to efa67b3 Compare September 29, 2024 13:41

vgvassilev merged commit 821683f into vgvassilev:master Sep 29, 2024
90 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move checks in CladFunction to compile-time #1090

Move checks in CladFunction to compile-time #1090

MihailMihov commented Sep 11, 2024

github-actions bot left a comment

github-actions bot Sep 11, 2024

github-actions bot Sep 11, 2024

github-actions bot Sep 11, 2024

github-actions bot Sep 11, 2024

github-actions bot Sep 11, 2024

github-actions bot Sep 11, 2024

codecov bot commented Sep 17, 2024 •

edited

Loading

vgvassilev Sep 18, 2024

MihailMihov Sep 18, 2024

vgvassilev Sep 19, 2024

vgvassilev left a comment

vgvassilev Sep 28, 2024

vgvassilev Sep 29, 2024

	// Add define for CLAD_SO_LOADED, so that CladFunction::CladFunction()
	// Add define for __CLAD_SO_LOADED, so that CladFunction::CladFunction()

Move checks in CladFunction to compile-time #1090

Move checks in CladFunction to compile-time #1090

Conversation

MihailMihov commented Sep 11, 2024

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Sep 11, 2024

Choose a reason for hiding this comment

github-actions bot Sep 11, 2024

Choose a reason for hiding this comment

github-actions bot Sep 11, 2024

Choose a reason for hiding this comment

github-actions bot Sep 11, 2024

Choose a reason for hiding this comment

github-actions bot Sep 11, 2024

Choose a reason for hiding this comment

github-actions bot Sep 11, 2024

Choose a reason for hiding this comment

codecov bot commented Sep 17, 2024 • edited Loading

Codecov Report

vgvassilev Sep 18, 2024

Choose a reason for hiding this comment

MihailMihov Sep 18, 2024

Choose a reason for hiding this comment

vgvassilev Sep 19, 2024

Choose a reason for hiding this comment

vgvassilev left a comment

Choose a reason for hiding this comment

vgvassilev Sep 28, 2024

Choose a reason for hiding this comment

vgvassilev Sep 29, 2024

Choose a reason for hiding this comment

codecov bot commented Sep 17, 2024 •

edited

Loading