Skip to content

better debug name for interned query arguments#837

Merged
davidbarsky merged 2 commits intosalsa-rs:masterfrom
MichaReiser:better-debug-name-for-interned-query-arguments
Apr 30, 2025
Merged

better debug name for interned query arguments#837
davidbarsky merged 2 commits intosalsa-rs:masterfrom
MichaReiser:better-debug-name-for-interned-query-arguments

Conversation

@MichaReiser
Copy link
Contributor

@MichaReiser MichaReiser commented Apr 30, 2025

The debug name Configuration isn't very helpful when debugging salsa. Use a name constructed from the query name.

I'm open to suggestions on how to avoid collisions with actual query names. e.g. we could prefix the name with two underscores or similar.

@netlify
Copy link

netlify bot commented Apr 30, 2025

Deploy Preview for salsa-rs canceled.

Name Link
🔨 Latest commit 9213e84
🔍 Latest deploy log https://app.netlify.com/sites/salsa-rs/deploys/681259992fe37a0008c17b86

@codspeed-hq
Copy link

codspeed-hq bot commented Apr 30, 2025

CodSpeed Performance Report

Merging #837 will not alter performance

Comparing MichaReiser:better-debug-name-for-interned-query-arguments (9213e84) with master (42f1583)

Summary

✅ 12 untouched benchmarks

@MichaReiser MichaReiser force-pushed the better-debug-name-for-interned-query-arguments branch from 6dbc86a to 77599c7 Compare April 30, 2025 15:59
@MichaReiser MichaReiser marked this pull request as ready for review April 30, 2025 16:00
line: line!(),
};
const DEBUG_NAME: &'static str = "Configuration";
const DEBUG_NAME: &'static str = concat!(stringify!($fn_name), "_arguments");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe something like?

Suggested change
const DEBUG_NAME: &'static str = concat!(stringify!($fn_name), "_arguments");
const DEBUG_NAME: &'static str = concat!(stringify!($fn_name), "_interned_args");

(In general, I'm a +1 on this change.)

@davidbarsky davidbarsky enabled auto-merge April 30, 2025 17:13
@davidbarsky davidbarsky added this pull request to the merge queue Apr 30, 2025
Merged via the queue into salsa-rs:master with commit ad3dff6 Apr 30, 2025
11 checks passed
@github-actions github-actions bot mentioned this pull request Apr 30, 2025
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