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

Set assert variables with PG_USED_FOR_ASSERTS_ONLY #7556

Merged
merged 1 commit into from
Dec 20, 2024

Conversation

antekresic
Copy link
Contributor

Couple of instances where variables are used for assertion only, which can trigger unused variable error on certain builds. Adding the necessary PG_USED_FOR_ASSERTS_ONLY fixes the problem.

Disable-check: force-changelog-file

@antekresic antekresic self-assigned this Dec 20, 2024
@antekresic antekresic requested a review from mkindahl December 20, 2024 08:35
Copy link

@fabriziomello, @akuzm: please review this pull request.

Powered by pull-review

@antekresic antekresic added this to the TimescaleDB 2.18.0 milestone Dec 20, 2024
Couple of instances where variables are used for assertion
only, which can trigger unused variable error on certain
builds. Adding the necessary PG_USED_FOR_ASSERTS_ONLY
fixes the problem.
@mkindahl
Copy link
Contributor

mkindahl commented Dec 20, 2024

One drawback is that if a function is called and populates such a variable, it cannot be optimized away (usually) since the compiler cannot decide if the function has any side-effects.

This effectively means that this attribute just hides the warning, but the problem (of computing unnecessary values, which the warning is really about) is still there.

Copy link

codecov bot commented Dec 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.16%. Comparing base (59f50f2) to head (7f92c3f).
Report is 665 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7556      +/-   ##
==========================================
+ Coverage   80.06%   82.16%   +2.09%     
==========================================
  Files         190      231      +41     
  Lines       37181    43373    +6192     
  Branches     9450    10909    +1459     
==========================================
+ Hits        29770    35637    +5867     
- Misses       2997     3402     +405     
+ Partials     4414     4334      -80     

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

@antekresic antekresic merged commit 015b391 into timescale:main Dec 20, 2024
46 checks passed
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.

3 participants