[Base] replace some instances of SourceLocation by std::source_location by tdavidcl · Pull Request #1858 · Shamrock-code/Shamrock

tdavidcl · 2026-05-29T22:05:12Z

No description provided.

github-actions · 2026-05-29T22:06:17Z

Thanks @tdavidcl for opening this PR!

You can do multiple things directly here:
1 - Comment pre-commit.ci run to run pre-commit checks.
2 - Comment pre-commit.ci autofix to apply fixes.
3 - Add label autofix.ci to fix authorship & pre-commit for every commit made.
4 - Add label light-ci to only trigger a reduced & faster version of the CI (need the full one before merge).
5 - Add label trigger-ci to create an empty commit to trigger the CI.

Once the workflow completes a message will appear displaying informations related to the run.

Also the PR gets automatically reviewed by gemini, you can:
1 - Comment /gemini review to trigger a review
2 - Comment /gemini summary for a summary
3 - Tag it using @gemini-code-assist either in the PR or in review comments on files

gemini-code-assist

Code Review

This pull request migrates the custom SourceLocation class to C++20's standard std::source_location across kernel calls, stack trace utilities, and test assertions. The review feedback correctly points out that std::source_location is a small, trivially copyable type and should be passed by value instead of by reference or rvalue reference to prevent unnecessary indirection and improve efficiency.

gemini-code-assist · 2026-05-29T22:06:36Z

            index_t n,
            Functor &&kernel_gen,
-            SourceLocation &&callsite = SourceLocation{}) {
+            std::source_location &&callsite = std::source_location::current()) {


In C++20, std::source_location is a small, trivially copyable object (typically consisting of a couple of pointers and integers). Passing it by rvalue reference (std::source_location&&) is an anti-pattern that prevents passing lvalue std::source_location objects and adds unnecessary indirection. It should be passed by value instead.

std::source_location callsite = std::source_location::current()) {

gemini-code-assist · 2026-05-29T22:06:36Z

            index_t n,
            Functor &&func,
-            SourceLocation &&callsite = SourceLocation{}) {
+            std::source_location &&callsite = std::source_location::current()) {


In C++20, std::source_location is a small, trivially copyable object. Passing it by rvalue reference (std::source_location&&) is an anti-pattern that prevents passing lvalue std::source_location objects and adds unnecessary indirection. It should be passed by value instead.

std::source_location callsite = std::source_location::current()) {

gemini-code-assist · 2026-05-29T22:06:36Z

        u32 n,
        Functor &&func,
-        SourceLocation &&callsite = SourceLocation{}) {
+        std::source_location &&callsite = std::source_location::current()) {


In C++20, std::source_location is a small, trivially copyable object. Passing it by rvalue reference (std::source_location&&) is an anti-pattern that prevents passing lvalue std::source_location objects and adds unnecessary indirection. It should be passed by value instead.

std::source_location callsite = std::source_location::current()) {

gemini-code-assist · 2026-05-29T22:06:36Z

        u64 n,
        Functor &&func,
-        SourceLocation &&callsite = SourceLocation{}) {
+        std::source_location &&callsite = std::source_location::current()) {


In C++20, std::source_location is a small, trivially copyable object. Passing it by rvalue reference (std::source_location&&) is an anti-pattern that prevents passing lvalue std::source_location objects and adds unnecessary indirection. It should be passed by value instead.

std::source_location callsite = std::source_location::current()) {

gemini-code-assist · 2026-05-29T22:06:36Z

        u32 n,
        Functor &&kernel_gen,
-        SourceLocation &&callsite = SourceLocation{}) {
+        std::source_location &&callsite = std::source_location::current()) {


In C++20, std::source_location is a small, trivially copyable object. Passing it by rvalue reference (std::source_location&&) is an anti-pattern that prevents passing lvalue std::source_location objects and adds unnecessary indirection. It should be passed by value instead.

std::source_location callsite = std::source_location::current()) {

gemini-code-assist · 2026-05-29T22:06:36Z

        u64 n,
        Functor &&kernel_gen,
-        SourceLocation &&callsite = SourceLocation{}) {
+        std::source_location &&callsite = std::source_location::current()) {


In C++20, std::source_location is a small, trivially copyable object. Passing it by rvalue reference (std::source_location&&) is an anti-pattern that prevents passing lvalue std::source_location objects and adds unnecessary indirection. It should be passed by value instead.

std::source_location callsite = std::source_location::current()) {

gemini-code-assist · 2026-05-29T22:06:36Z

+        inline CallStackEntry(const std::source_location &loc) {
+            // Push the source location to the call stack
+            call_stack.emplace(SourceLocation{loc});
+        }


In C++20, std::source_location is a small, trivially copyable object. Passing it by const std::source_location& is less efficient than passing by value because it introduces pointer indirection. It should be passed by value instead.

Suggested change

inline CallStackEntry(const std::source_location &loc) {

// Push the source location to the call stack

call_stack.emplace(SourceLocation{loc});

}

inline CallStackEntry(std::source_location loc) {

// Push the source location to the call stack

call_stack.emplace(SourceLocation{loc});

}

gemini-code-assist · 2026-05-29T22:06:36Z

+        inline BasicStackEntry(
+            const std::source_location &callsite,
+            bool do_timer              = true,
+            std::source_location &&loc = std::source_location::current())
+            : BasicStackEntry(SourceLocation{callsite}, do_timer, SourceLocation{loc}) {}


In C++20, std::source_location is a small, trivially copyable object. Passing it by reference (const std::source_location& or std::source_location&&) is less efficient and can prevent passing lvalue objects. It should be passed by value instead.

Suggested change

inline BasicStackEntry(

const std::source_location &callsite,

bool do_timer = true,

std::source_location &&loc = std::source_location::current())

: BasicStackEntry(SourceLocation{callsite}, do_timer, SourceLocation{loc}) {}

inline BasicStackEntry(

std::source_location callsite,

bool do_timer = true,

std::source_location loc = std::source_location::current())

: BasicStackEntry(SourceLocation{callsite}, do_timer, SourceLocation{loc}) {}

github-actions · 2026-05-29T23:11:03Z

Workflow report

workflow report corresponding to commit 5f4016a
Commiter email is timothee.davidcleris@proton.me

Light CI is enabled. This will only run the basic tests and not the full tests.
Merging a PR require the job "on PR / all" to pass which is disabled in this case.

Pre-commit check report

Pre-commit check: ✅

trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check for merge conflicts................................................Passed
check that executables have shebangs.....................................Passed
check that scripts with shebangs are executable..........................Passed
check for added large files..............................................Passed
check for case conflicts.................................................Passed
check for broken symlinks................................................Passed
check yaml...............................................................Passed
detect private key.......................................................Passed
No-tabs checker..........................................................Passed
Tabs remover.............................................................Passed
cmake-format.............................................................Passed
Validate GitHub Workflows................................................Passed
clang-format.............................................................Passed
ruff check...............................................................Passed
ruff format..............................................................Passed
Check doxygen headers....................................................Passed
Check license headers....................................................Passed
Check #pragma once.......................................................Passed
Check SYCL #include......................................................Passed
No ssh in git submodules remote..........................................Passed
No UTF-8 in files (except for authors)...................................Passed

Test pipeline can run.

Doxygen diff with `main`

Removed warnings : 6
New warnings : 8
Warnings count : 7856 → 7858 (0.0%)

Detailed changes :

- src/shambackends/include/shambackends/kernel_call.hpp:353: warning: The following parameter of sham::kernel_call(sham::DeviceQueue &q, RefIn in, RefOut in_out, u32 n, Functor &&func, SourceLocation &&callsite=SourceLocation{}) is not documented:
+ src/shambackends/include/shambackends/kernel_call.hpp:353: warning: The following parameter of sham::kernel_call(sham::DeviceQueue &q, RefIn in, RefOut in_out, u32 n, Functor &&func, std::source_location &&callsite=std::source_location::current()) is not documented:
- src/shambackends/include/shambackends/kernel_call.hpp:546: warning: Member kernel_call_hndl(sham::DeviceQueue &q, RefIn in, RefOut in_out, u32 n, Functor &&kernel_gen, SourceLocation &&callsite=SourceLocation{}) (function) of namespace sham is not documented.
+ src/shambackends/include/shambackends/kernel_call.hpp:546: warning: Member kernel_call_hndl(sham::DeviceQueue &q, RefIn in, RefOut in_out, u32 n, Functor &&kernel_gen, std::source_location &&callsite=std::source_location::current()) (function) of namespace sham is not documented.
- src/shambase/include/shambase/stacktrace.hpp:106: warning: Compound shambase::details::BasicStackEntry is not documented.
+ src/shambase/include/shambase/stacktrace.hpp:111: warning: Compound shambase::details::BasicStackEntry is not documented.
- src/shambase/include/shambase/stacktrace.hpp:137: warning: The following parameter of shambase::details::BasicStackEntry::BasicStackEntry(SourceLocation &callsite, bool do_timer=true, SourceLocation &&loc=SourceLocation{}) is not documented:
+ src/shambase/include/shambase/stacktrace.hpp:142: warning: The following parameter of shambase::details::BasicStackEntry::BasicStackEntry(const SourceLocation &callsite, bool do_timer=true, SourceLocation &&loc=SourceLocation{}) is not documented:
+ src/shambase/include/shambase/stacktrace.hpp:160: warning: Member BasicStackEntry(const std::source_location &callsite, bool do_timer=true, std::source_location &&loc=std::source_location::current()) (function) of struct shambase::details::BasicStackEntry is not documented.
- src/shambase/include/shambase/stacktrace.hpp:170: warning: Compound shambase::details::NamedBasicStackEntry is not documented.
+ src/shambase/include/shambase/stacktrace.hpp:183: warning: Compound shambase::details::NamedBasicStackEntry is not documented.
- src/shambase/include/shambase/stacktrace.hpp:88: warning: Member CallStackEntry(SourceLocation &loc) (function) of struct shambase::details::CallStackEntry is not documented.
+ src/shambase/include/shambase/stacktrace.hpp:88: warning: Member CallStackEntry(const SourceLocation &loc) (function) of struct shambase::details::CallStackEntry is not documented.
+ src/shambase/include/shambase/stacktrace.hpp:93: warning: Member CallStackEntry(const std::source_location &loc) (function) of struct shambase::details::CallStackEntry is not documented.

[Base] replace some instances of SourceLocation by std::source_location

eecaf30

tdavidcl added draft light-ci labels May 29, 2026

gemini-code-assist Bot reviewed May 29, 2026

View reviewed changes

more

5f4016a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Base] replace some instances of SourceLocation by std::source_location#1858

[Base] replace some instances of SourceLocation by std::source_location#1858
tdavidcl wants to merge 2 commits into
Shamrock-code:mainfrom
tdavidcl:start_source_loc_removal

tdavidcl commented May 29, 2026

Uh oh!

github-actions Bot commented May 29, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot May 29, 2026

Uh oh!

gemini-code-assist Bot May 29, 2026

Uh oh!

gemini-code-assist Bot May 29, 2026

Uh oh!

gemini-code-assist Bot May 29, 2026

Uh oh!

gemini-code-assist Bot May 29, 2026

Uh oh!

gemini-code-assist Bot May 29, 2026

Uh oh!

gemini-code-assist Bot May 29, 2026

Uh oh!

gemini-code-assist Bot May 29, 2026

Uh oh!

github-actions Bot commented May 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

tdavidcl commented May 29, 2026

Uh oh!

github-actions Bot commented May 29, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 29, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 29, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 29, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 29, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 29, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 29, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 29, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 29, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 29, 2026

Workflow report

Pre-commit check report

Doxygen diff with main

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Doxygen diff with `main`