Skip to content

Add SAL annotations to safecast.h helper methods#620

Merged
jonwis merged 2 commits intomicrosoft:masterfrom
dmachaj:user/dmachaj/safecast-annotations
Feb 24, 2026
Merged

Add SAL annotations to safecast.h helper methods#620
jonwis merged 2 commits intomicrosoft:masterfrom
dmachaj:user/dmachaj/safecast-annotations

Conversation

@dmachaj
Copy link
Collaborator

@dmachaj dmachaj commented Feb 24, 2026

Why is this change being made?

After adding some usage of these helpers in the OS we received a static analysis bug that was a false positive. The underlying problem is that these methods lack annotations on the result so the static analysis tooling had to assume the worst.

Briefly summarize what changed

Add SAL annotations so that static analysis understands that upon success the input and output values are unchanged.

How was this change tested?

As part of the OS. This is a port to GitHub after the fact.

@dmachaj dmachaj requested review from dunhor and jonwis February 24, 2026 18:02
// This conversion is unsafe, therefore the two parameter version of safe_cast_nothrow must be used
template <typename NewT, typename OldT, wistd::enable_if_t<details::is_supported_unsafe_cast_v<NewT, OldT>, int> = 0>
NewT safe_cast_nothrow(const OldT /*var*/)
_Out_range_(==, _Param_(1)) NewT safe_cast_nothrow(const OldT /*var*/)
Copy link
Member

Choose a reason for hiding this comment

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

Just out of curiosity, if this is absent, does the analyzer error still trigger? This is a static assert that should always fire (and if it doesn't, the function doesn't have a return statement and should warn). Just curious if the analyzer is smart enough to determine the correct overload.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This variant wasn't part of the false positive. I included the annotation for completeness. It probably isn't necessary but it also doesn't hurt to include.

@jonwis jonwis merged commit 3ecba60 into microsoft:master Feb 24, 2026
15 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