Skip to content

[logging] LOG(FATAL) calls [[noreturn]] functions - #11310

Merged
areusch merged 1 commit into
apache:mainfrom
cconvey:fatal-logging-noreturn
May 13, 2022
Merged

[logging] LOG(FATAL) calls [[noreturn]] functions#11310
areusch merged 1 commit into
apache:mainfrom
cconvey:fatal-logging-noreturn

Conversation

@cconvey

@cconveycconvey commented May 13, 2022

Copy link
Copy Markdown
Contributor

Ensure that LOG(FATAL) always resolves to calling
[[noreturn]] code. This has two benefits:

  • Helps developers more quickly understand the
    intended/required behavior for LOG(FATAL) calls.

  • May eliminate spurious compiler warnings
    based on control-flow analysis. E.g. gcc's / clang's
    -Wno-return warnings.

@cconvey

Copy link
Copy Markdown
ContributorAuthor

CC: @areusch

@cconvey

Copy link
Copy Markdown
ContributorAuthor

Note: one motivation for this PR is to support #11245.

Ensure that `LOG(FATAL)` always resolves to calling
`[[noreturn]]` code. This has two benefits:
- Helps developers more quickly understand the intended/required
behavior for `LOG(FATAL)` calls.
- May eliminate spurious compiler warnings based on control-flow
analysis. E.g. gcc's / clang's `-Wno-return` warnings.
@cconvey
cconveyforce-pushed the fatal-logging-noreturn branch from 777d231 to 6074e6eCompareMay 13, 2022 14:48

@tkonoligetkonolige left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sounds like this will silence the llvm warnings that force us to put empty returns after LOG(FATAL). Thanks!

@areusch
areusch merged commit 80d8270 into apache:mainMay 13, 2022
@cconvey

Copy link
Copy Markdown
ContributorAuthor

Sounds like this will silence the llvm warnings that force us to put empty returns after LOG(FATAL). Thanks!

That was my intent, but I'm not sure it's actually effective, at least with the gcc / clang builds I tried. Hopefully newer/future versions of those compilers will be smarter about this.

@cconvey
cconvey deleted the fatal-logging-noreturn branch May 13, 2022 16:47
mehrdadh pushed a commit to mehrdadh/tvm that referenced this pull request May 16, 2022
Ensure that `LOG(FATAL)` always resolves to calling
`[[noreturn]]` code. This has two benefits:
- Helps developers more quickly understand the intended/required
behavior for `LOG(FATAL)` calls.
- May eliminate spurious compiler warnings based on control-flow
analysis. E.g. gcc's / clang's `-Wno-return` warnings.
shtinsa pushed a commit to Deelvin/tvm that referenced this pull request May 17, 2022
Ensure that `LOG(FATAL)` always resolves to calling
`[[noreturn]]` code. This has two benefits:
- Helps developers more quickly understand the intended/required
behavior for `LOG(FATAL)` calls.
- May eliminate spurious compiler warnings based on control-flow
analysis. E.g. gcc's / clang's `-Wno-return` warnings.
shingjan pushed a commit to shingjan/tvm that referenced this pull request May 17, 2022
Ensure that `LOG(FATAL)` always resolves to calling
`[[noreturn]]` code. This has two benefits:
- Helps developers more quickly understand the intended/required
behavior for `LOG(FATAL)` calls.
- May eliminate spurious compiler warnings based on control-flow
analysis. E.g. gcc's / clang's `-Wno-return` warnings.
Sign up for freeto 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

@cconvey@areusch@tkonolige