Skip to content

says "you must also specify \Og" but that is not true #2300

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

Merged
merged 2 commits into from
Jul 8, 2020

Conversation

angusgraham
Copy link
Contributor

Doc for /Ot /Os says

If you use **/Os** or **/Ot**, then you must also specify [/Og](og-global-optimizations.md) to optimize the code.

...but if you look at the doc for /Og it says

"Deprecated. Provides local and global optimizations, automatic-register allocation, and loop optimization. We recommend you use either /O1 (Minimize Size) or /O2 (Maximize Speed) instead."

...and in the remarks section

"/Og is deprecated. These optimizations are now generally enabled by default. For more information on optimizations, see /O1, /O2 (Minimize Size, Maximize Speed) or /Ox (Enable Most Speed Optimizations)."

...so I've removed the misleading instruction to turn on /Og (which is actually not even possible through the Visual Studio options optimization pane - which makes sense since it's deprecated) and instead added a more generic message about turning on optimizations (copied the "For more information" part from https://github.com/MicrosoftDocs/cpp-docs/edit/master/docs/build/reference/og-global-optimizations.md)

Since it's no longer a direct instruction but just further info I've changed it to a [!NOTE]

Doc for /Ot /Os says

`If you use **/Os** or **/Ot**, then you must also specify [/Og](og-global-optimizations.md) to optimize the code.`

...but if you look at the doc for /Og it says

"Deprecated. Provides local and global optimizations, automatic-register allocation, and loop optimization. We recommend you use either /O1 (Minimize Size) or /O2 (Maximize Speed) instead."

...and in the remarks section

"/Og is deprecated. These optimizations are now generally enabled by default. For more information on optimizations, see /O1, /O2 (Minimize Size, Maximize Speed) or /Ox (Enable Most Speed Optimizations)."

...so I've removed the misleading instruction to turn on /Og (which is actually not even possible through the Visual Studio options optimization pane - which makes sense since it's deprecated) and instead added a more generic message about turning on optimizations (copied the "For more information" part from https://github.com/MicrosoftDocs/cpp-docs/edit/master/docs/build/reference/og-global-optimizations.md)

Since it's no longer a direct instruction but just further info I've changed it to a [!NOTE]
@PRMerger18
Copy link
Contributor

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

@angusgraham
Copy link
Contributor Author

Looking at a grep of cpp-docs for /Og (https://github.com/MicrosoftDocs/cpp-docs/search?q=Og&unscoped_q=Og) it looks like most references to /Og are similarly misleading. I'm not going to fix them all but I suggest someone should.

It's true that /Og is not required with /Os or /Ot anymore, and hasn't been for a long time. I think just removing that statement is enough for now.
@PRMerger13
Copy link
Contributor

@corob-msft : Thanks for your contribution! The author(s) have been notified to review your proposed change.

Copy link
Contributor

@colin-home colin-home left a comment

Choose a reason for hiding this comment

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

@angusgraham Thanks for taking the time to contribute a fix to the docs. I'll look into the other references to /Og for a more comprehensive fix.

@colin-home colin-home merged commit f5830fa into MicrosoftDocs:master Jul 8, 2020
@angusgraham angusgraham deleted the patch-1 branch October 8, 2022 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants