Skip to content

Tweak C4172 warning reference #5445

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Tweak C4172 warning reference #5445

wants to merge 7 commits into from

Conversation

Rageking8
Copy link
Contributor

  • Add blockquotes for warning message
  • Add "Remarks" and "Example" headings
  • Tweak example
    • Use /c flag instead of /LD
    • Initialize global variable f with a float literal instead
    • Rename bar to func
    • Add another possible resolution with const auto&
    • Format example

@prmerger-automator PRMerger Automator
Copy link
Contributor

@Rageking8 : Thanks for your contribution! The author(s) and reviewer(s) have been notified to review your proposed change.

@prmerger-automator prmerger-automator bot requested a review from TylerMSFT June 20, 2025 10:29
@learn-build-service-prod Learn Build Service (PROD)
Copy link
Contributor

Learn Build status updates of commit 10d9a37:

✅ Validation status: passed

File Status Preview URL Details
docs/error-messages/compiler-warnings/compiler-warning-level-1-c4172.md ✅Succeeded

For more details, please refer to the build report.

@prmerger-automator PRMerger Automator
Copy link
Contributor

PRMerger Results

Issue Description
File Change Percent This PR contains file(s) with more than 30% file change.

@v-regandowner
Copy link
Contributor

@TylerMSFT - Can you review the proposed changes?

IMPORTANT: When the changes are ready for publication, adding a #sign-off comment is the best way to signal that the PR is ready for the review team to merge.

#label:"aq-pr-triaged"
@MicrosoftDocs/public-repo-pr-review-team

@prmerger-automator prmerger-automator bot added the aq-pr-triaged Tracking label for the PR review team label Jun 20, 2025
@TylerMSFT
Update compiler-warning-level-1-c4172.md
2acb092 
We demo an unusual case but not perhaps a more common case. Added it.
TylerMSFT
TylerMSFT approved these changes Jun 24, 2025
View reviewed changes
Copy link
Collaborator

@TylerMSFT TylerMSFT left a comment

Choose a reason for hiding this comment

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

I like what you did with this.
I thought the existing example was a little opaque, so explained it. And added the more common case example.

@learn-build-service-prod Learn Build Service (PROD)
Copy link
Contributor

Learn Build status updates of commit 2acb092:

✅ Validation status: passed

File Status Preview URL Details
docs/error-messages/compiler-warnings/compiler-warning-level-1-c4172.md ✅Succeeded

For more details, please refer to the build report.

@prmerger-automator PRMerger Automator
Copy link
Contributor

PRMerger Results

Issue Description
File Change Percent This PR contains file(s) with more than 30% file change.

@TylerMSFT
Copy link
Collaborator

#sign-off

Rageking8
Rageking8 commented Jun 25, 2025
View reviewed changes
docs/error-messages/compiler-warnings/compiler-warning-level-1-c4172.md
@@ -26,7 +26,7 @@ The following example generates C4172:
const int* func1()
{
int i = 42;
return &i; // C4172
return &i; // C4172
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should standardize use of spaces over tabs and the 3 spaces before // CXXXX.

@learn-build-service-prod Learn Build Service (PROD)
Copy link
Contributor

Learn Build status updates of commit 4380ed1:

✅ Validation status: passed

File Status Preview URL Details
docs/error-messages/compiler-warnings/compiler-warning-level-1-c4172.md ✅Succeeded

For more details, please refer to the build report.

@prmerger-automator PRMerger Automator
Copy link
Contributor

PRMerger Results

Issue Description
File Change Percent This PR contains file(s) with more than 30% file change.

Rageking8
Rageking8 commented Jun 25, 2025
View reviewed changes
docs/error-messages/compiler-warnings/compiler-warning-level-1-c4172.md
f1_keywords: ["C4172"]
helpviewer_keywords: ["C4172"]
---
# Compiler Warning (level 1) C4172

> returning address of local variable or temporary
> returning address of local variable or temporary : *optional_context*
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just realized that the warning message didn't account for the optional context.

@learn-build-service-prod Learn Build Service (PROD)
Copy link
Contributor

Learn Build status updates of commit 78e1890:

✅ Validation status: passed

File Status Preview URL Details
docs/error-messages/compiler-warnings/compiler-warning-level-1-c4172.md ✅Succeeded
docs/error-messages/compiler-warnings/compiler-warnings-c4000-through-c4199.md ✅Succeeded

For more details, please refer to the build report.

@prmerger-automator PRMerger Automator
Copy link
Contributor

PRMerger Results

Issue Description
File Change Percent This PR contains file(s) with more than 30% file change.

@v-dirichards
Copy link
Contributor

@TylerMSFT Would you add a comment to indicate what action to take on this PR?

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TylerMSFT TylerMSFT TylerMSFT approved these changes

Assignees

@TylerMSFT TylerMSFT

Labels
aq-pr-triaged Tracking label for the PR review team Change sent to author do-not-merge errors-warnings/subsvc needs-human-review visual-cpp/svc
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Rageking8 @v-regandowner @TylerMSFT @v-dirichards