Skip to content

clang-19 build fix: workaround narrowing-const-reference error#39249

Closed
paparodeo wants to merge 1 commit intosagemath:developfrom
paparodeo:narrowing
Closed

clang-19 build fix: workaround narrowing-const-reference error#39249
paparodeo wants to merge 1 commit intosagemath:developfrom
paparodeo:narrowing

Conversation

@paparodeo
Copy link
Contributor

@paparodeo paparodeo commented Jan 3, 2025

while building sage 10.5 with clang-19 with nixpkgs on macOS the build fails with the following error:

In file included from build/cythonized/sage/graphs/base/boost_graph.cpp:1278:
build/cythonized/sage/graphs/base/boost_interface.cpp:119:34: error: non-constant-expression cannot be narrowed from type 'value_type' (aka 'unsigned long') to 'int' in initializer list [-Wc++11-narrowing-const-reference]
  119 | to_return.push_back({index[boost::source(*ei, graph)],
      |                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
build/cythonized/sage/graphs/base/boost_graph.cpp:17503:29: note: in instantiation of member function 'BoostGraph<boost::vecS, boost::vecS, boost::directedS, boost::vecS, boost::property<boost::edge_weight_t, double>>::edge_list' requested here
 17503 |   __pyx_v_edges = __pyx_v_g.edge_list();
                                     ^

https://clang.llvm.org/docs/DiagnosticsReference.html#wc-11-narrowing-const-reference

To silence the error assign to a v_index outside of the initializer list.

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

@paparodeo
clang-19 build: workaround narrowing-const-reference error
216f91c 
```
In file included from build/cythonized/sage/graphs/base/boost_graph.cpp:1278:
build/cythonized/sage/graphs/base/boost_interface.cpp:119:34: error: non-constant-expression cannot be narrowed from type 'value_type' (aka 'unsigned long') to 'int' in initializer list [-Wc++11-narrowing-const-reference]
  119 | to_return.push_back({index[boost::source(*ei, graph)],
      |                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
build/cythonized/sage/graphs/base/boost_graph.cpp:17503:29: note: in instantiation of member function 'BoostGraph<boost::vecS, boost::vecS, boost::directedS, boost::vecS, boost::property<boost::edge_weight_t, double>>::edge_list' requested here
 17503 |   __pyx_v_edges = __pyx_v_g.edge_list();
                                     ^
```

https://clang.llvm.org/docs/DiagnosticsReference.html#wc-11-narrowing-const-reference

To silence the error assign to a v_index outside of the
initializer list.
@github-actions
Copy link

github-actions bot commented Jan 3, 2025

Documentation preview for this PR (built with commit 216f91c; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@paparodeo paparodeo mentioned this pull request Jan 4, 2025
Merged
13 tasks
@paparodeo paparodeo closed this by deleting the head repository Jan 31, 2025
@tobiasdiez tobiasdiez mentioned this pull request Feb 6, 2025
Merged
5 tasks
@saraedum
Copy link
Member

saraedum commented Feb 11, 2025 •
edited
Loading

@paparodeo why did you close this PR? Is this not an actual issue that should be fixed? (Got the same problem on conda-forge when building for macOS.)

@paparodeo
Copy link
Contributor Author

paparodeo commented Feb 11, 2025

@paparodeo why did you close this PR? Is this not an actual issue that should be fixed? (Got the same problem on conda-forge when building for macOS.)

it is still an issue -- the PR closed when I deleted the repository. the PR was open for a month with no response.

@saraedum
Copy link
Member

saraedum commented Feb 14, 2025

I'm actually a bit puzzled that this fixes the warning. It seems that the following line contains another such narrowing. Was that the only warning that showed up for you @paparodeo ?

@saraedum
Copy link
Member

saraedum commented Feb 14, 2025

This looks quite wrong:
boost_interface.cpp

typedef int v_index;
typedef long e_index;

boost_interface.pxd

    ctypedef unsigned long v_index
    ctypedef unsigned long e_index

@paparodeo
Copy link
Contributor Author

paparodeo commented Feb 14, 2025

I'm actually a bit puzzled that this fixes the warning. It seems that the following line contains another such narrowing. Was that the only warning that showed up for you @paparodeo ?

the change in this PR fixed the build -- it is an error not a warning.

saraedum added a commit to saraedum/sage that referenced this pull request Feb 14, 2025
@saraedum
Fix compile error when compiling boost graphs with recent clang
395ca33 
this fixes an underlying type confusion in our wrapper that has lead to
warnings before but now produces an actual error, see sagemath#39249
@saraedum saraedum mentioned this pull request Feb 14, 2025
Merged
vbraun pushed a commit to vbraun/sage that referenced this pull request Apr 5, 2025
4d3664d 
sagemathgh-39526: Fix compile error when compiling boost graphs with recent clang
    
this fixes an underlying type confusion in our wrapper that has lead to
warnings before but now produces an actual error, see
sagemath#39249
    
URL: sagemath#39526
Reported by: Julian Rüth
Reviewer(s): Dima Pasechnik, Julian Rüth, user202729
vbraun pushed a commit to vbraun/sage that referenced this pull request Apr 7, 2025
d548ef9 
sagemathgh-39526: Fix compile error when compiling boost graphs with recent clang
    
this fixes an underlying type confusion in our wrapper that has lead to
warnings before but now produces an actual error, see
sagemath#39249
    
URL: sagemath#39526
Reported by: Julian Rüth
Reviewer(s): Dima Pasechnik, Julian Rüth, user202729
vbraun pushed a commit to vbraun/sage that referenced this pull request Apr 10, 2025
ecf6332 
sagemathgh-39526: Fix compile error when compiling boost graphs with recent clang
    
this fixes an underlying type confusion in our wrapper that has lead to
warnings before but now produces an actual error, see
sagemath#39249
    
URL: sagemath#39526
Reported by: Julian Rüth
Reviewer(s): Dima Pasechnik, Julian Rüth, user202729
vbraun pushed a commit to vbraun/sage that referenced this pull request Apr 13, 2025
fa15909 
sagemathgh-39526: Fix compile error when compiling boost graphs with recent clang
    
this fixes an underlying type confusion in our wrapper that has lead to
warnings before but now produces an actual error, see
sagemath#39249
    
URL: sagemath#39526
Reported by: Julian Rüth
Reviewer(s): Dima Pasechnik, Isuru Fernando, Julian Rüth, user202729
vbraun pushed a commit to vbraun/sage that referenced this pull request Apr 18, 2025
8226bea 
sagemathgh-39526: Fix compile error when compiling boost graphs with recent clang
    
this fixes an underlying type confusion in our wrapper that has lead to
warnings before but now produces an actual error, see
sagemath#39249
    
URL: sagemath#39526
Reported by: Julian Rüth
Reviewer(s): Dima Pasechnik, Isuru Fernando, Julian Rüth, user202729
vbraun pushed a commit to vbraun/sage that referenced this pull request Oct 11, 2025
080bdea 
sagemathgh-39189: Add support for Python 3.13 in conda
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->


Fixes sagemath#39163

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [ ] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

- sagemath#38749
- sagemath#39249

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#39189
Reported by: Tobias Diez
Reviewer(s): Dima Pasechnik
vbraun pushed a commit to vbraun/sage that referenced this pull request Oct 12, 2025
b829826 
sagemathgh-39189: Add support for Python 3.13 in conda
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->


Fixes sagemath#39163

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [ ] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

- sagemath#38749
- sagemath#39249

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#39189
Reported by: Tobias Diez
Reviewer(s): Dima Pasechnik
vbraun pushed a commit to vbraun/sage that referenced this pull request Oct 15, 2025
12c7aa5 
sagemathgh-39189: Add support for Python 3.13 in conda
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->


Fixes sagemath#39163

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [ ] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

- sagemath#38749
- sagemath#39249

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#39189
Reported by: Tobias Diez
Reviewer(s): Dima Pasechnik
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

s: needs review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@paparodeo @saraedum