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

Suppress clang error #461

Merged
merged 2 commits into from
Sep 7, 2024
Merged

Suppress clang error #461

merged 2 commits into from
Sep 7, 2024

Conversation

IvanaGyro
Copy link
Collaborator

This PR eases the problem of #449.

@IvanaGyro IvanaGyro changed the base branch from master to dev-master September 1, 2024 09:28
@IvanaGyro IvanaGyro force-pushed the suppress-clang-error branch from d5666c3 to 920b446 Compare September 1, 2024 09:29
Copy link

codecov bot commented Sep 1, 2024

Codecov Report

Attention: Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.

Project coverage is 16.60%. Comparing base (dea928d) to head (72453ca).
Report is 14 commits behind head on dev-master.

Files with missing lines Patch % Lines
src/backend/linalg_internal_cpu/Det_internal.cpp 0.00% 4 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           dev-master     #461   +/-   ##
===========================================
  Coverage       16.60%   16.60%           
===========================================
  Files             221      221           
  Lines           48486    48490    +4     
  Branches        20271    20272    +1     
===========================================
+ Hits             8052     8053    +1     
- Misses          36141    36143    +2     
- Partials         4293     4294    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

kaihsin
kaihsin previously requested changes Sep 2, 2024
src/backend/linalg_internal_cpu/Det_internal.cpp Outdated Show resolved Hide resolved
src/backend/linalg_internal_cpu/Det_internal.cpp Outdated Show resolved Hide resolved
tests/gpu/BlockUniTensor_test.h Outdated Show resolved Hide resolved
tests/gpu/linalg_test/Directsum_test.cpp Outdated Show resolved Hide resolved
@kaihsin
Copy link
Member

kaihsin commented Sep 2, 2024

Actually maybe I miss something but what exactly is the issue?

@kaihsin
Copy link
Member

kaihsin commented Sep 2, 2024

[ 83%] Building CXX object CMakeFiles/cytnx.dir/src/backend/linalg_internal_cpu/Ger_internal.cpp.o
/Users/pcchen/github/Cytnx/src/backend/linalg_internal_cpu/Det_internal.cpp:31:36: error: implicit conversion from '_Complex double' to 'double' is not permitted in C++
        od[0] *= (cytnx_complex128)_Rin[i * N + i];

It seems the issue is the implicit cast upon calling *= where its going from complex to double? (specifically od[0] is double and someone try to do this op?) Then I am not sure the fix in this OR goes into the root of problem? @IvanaGyro maybe you can put some more insight on what happends?

@IvanaGyro
Copy link
Collaborator Author

IvanaGyro commented Sep 2, 2024

[ 83%] Building CXX object CMakeFiles/cytnx.dir/src/backend/linalg_internal_cpu/Ger_internal.cpp.o
/Users/pcchen/github/Cytnx/src/backend/linalg_internal_cpu/Det_internal.cpp:31:36: error: implicit conversion from '_Complex double' to 'double' is not permitted in C++
        od[0] *= (cytnx_complex128)_Rin[i * N + i];

It seems the issue is the implicit cast upon calling *= where its going from complex to double? (specifically od[0] is double and someone try to do this op?) Then I am not sure the fix in this OR goes into the root of problem? @IvanaGyro maybe you can put some more insight on what happends?

Sorry, I don't have a Mac device. This original problem of implicit conversion only happens on Mac. Implicit conversion should not happen. Here is the comment I left in the issue. #449 (comment)

@IvanaGyro IvanaGyro force-pushed the suppress-clang-error branch from 920b446 to 7eb7cc8 Compare September 2, 2024 05:22
@kaihsin
Copy link
Member

kaihsin commented Sep 2, 2024 via email

Suppress specific compilation errors reported by Clang for the following
files:

1. **src/linalg/Tensordot.cpp**:
  - Error: non-pod-varargs
  - Details: Passing non-POD types to variadic functions is
      conditionally supported by the compiler, as outlined in the C++17
      standard (section 8.2.2/9). For more information, refer to
      [Passing NON-POD type to Variadic function is undefined behavior?](https://stackoverflow.com/a/10083921).

2. **Other Files**:
  - Error: c++11-narrowing
@pcchen
Copy link
Collaborator

pcchen commented Sep 2, 2024

I would like to suggest to keep this kind of technical note somewhere in the repository.

An issue reported the code below raises error by clang on Mac.

```
doulbe _Complex right = 123.4 + 45435.1I;
std::complex<doulbe> left{43.4, 2532.1};
left *= (std::complex<doulbe>)right; // invalid implicit conversion
```

This may be a bug in Clang on Mac, as no errors are raised by Clang on
CentOS or in the online compiler.
@IvanaGyro IvanaGyro force-pushed the suppress-clang-error branch from 7eb7cc8 to 72453ca Compare September 2, 2024 06:20
@IvanaGyro IvanaGyro requested a review from kaihsin September 2, 2024 06:21
@IvanaGyro
Copy link
Collaborator Author

I would like to suggest to keep this kind of technical note somewhere in the repository.

Mentioned in the comment in the latest update.

@IvanaGyro IvanaGyro dismissed kaihsin’s stale review September 3, 2024 13:03

The commits are updated.

@jeffry1829 jeffry1829 merged commit b67ae85 into dev-master Sep 7, 2024
4 checks passed
@jeffry1829 jeffry1829 deleted the suppress-clang-error branch September 7, 2024 05:12
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.

4 participants