Add NOTREACHED_IN_MIGRATION() [chromium/mini_chromium : main]
0 views
Skip to first unread message
Peter Boström (Gerrit)
May 16, 2024, 1:41:16 PMMay 16
to Mark Mentovai, crashp...@chromium.org
Attention needed from Mark Mentovai
Peter Boström added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Related details
Attention is currently required from:
- Mark Mentovai
Submit Requirements:
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mark Mentovai (Gerrit)
May 16, 2024, 1:52:07 PMMay 16
to Peter Boström, crashp...@chromium.org
Attention needed from Peter Boström
Mark Mentovai added 3 comments![]( "Open in Gerrit")
File base/notreached.h
Line 13, Patchset 2 (Latest):
// stream arguments to it. For a more complete implementation we should
// LOG(FATAL) but that doesn't work until mini_chromium's LOG(FATAL) is properly
// understood as [[noreturn]].Mark Mentovai . unresolved
What’s preventing that?
Line 11, Patchset 2 (Latest):
// crashpad and chromium has migrated off of the non-noreturn version. This isMark Mentovai . unresolved
Capitalize these proper names. And again on the next line, and on line 19.
Line 10, Patchset 2 (Latest):
// TODO(crbug.com/40580068): Redefine NOTREACHED() to be the [[noreturn]] onceMark Mentovai . unresolved
This word seems out of place.
Related details
Attention is currently required from:
- Peter Boström
Submit Requirements:
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Peter Boström (Gerrit)
May 16, 2024, 2:04:03 PMMay 16
to Mark Mentovai, crashp...@chromium.org
Attention needed from Mark Mentovai
Peter Boström added 4 comments![]( "Open in Gerrit")
Patchset-level comments
Peter Boström . resolved
PTAL!
File base/notreached.h
Line 13, Patchset 2:
// stream arguments to it. For a more complete implementation we should
// LOG(FATAL) but that doesn't work until mini_chromium's LOG(FATAL) is properly
// understood as [[noreturn]].Mark Mentovai . resolved
Peter BoströmWhat’s preventing that?
TODO added to base/logging.h instead and back referenced here. LMK if that makes sense.
Line 11, Patchset 2:
// crashpad and chromium has migrated off of the non-noreturn version. This isMark Mentovai . resolved
Capitalize these proper names. And again on the next line, and on line 19.
Peter Boström
Done
Line 10, Patchset 2:
// TODO(crbug.com/40580068): Redefine NOTREACHED() to be the [[noreturn]] onceMark Mentovai . resolved
This word seems out of place.
Attention is currently required from:
- Mark Mentovai
Submit Requirements:
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mark Mentovai (Gerrit)
May 16, 2024, 2:45:41 PMMay 16
to Peter Boström, crashp...@chromium.org
Attention needed from Peter Boström
Mark Mentovai voted and added 3 comments![]( "Open in Gerrit")
Votes added by Mark Mentovai
| Code-Review | +1 |
3 comments
Patchset-level comments
Mark Mentovai . resolved
Thanks, the comment makes more sense in logging.h.
File base/notreached.h
Line 12, Patchset 4 (Latest):
// easiest done by defining it as std::abort() as crashpad currently doesn'tMark Mentovai . unresolved
Crashpad
Line 11, Patchset 4 (Latest):
// Crashpad and Chromium has migrated off of the non-noreturn version. This isMark Mentovai . unresolved
have
Related details
Attention is currently required from:
- Peter Boström
Submit Requirements:
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Peter Boström (Gerrit)
May 16, 2024, 3:10:23 PMMay 16
to Mark Mentovai, crashp...@chromium.org
Peter Boström added 2 comments![]( "Open in Gerrit")
File base/notreached.h
Line 12, Patchset 4:
// easiest done by defining it as std::abort() as crashpad currently doesn'tMark Mentovai . resolved
Peter BoströmCrashpad
lol, oops, done.
Line 11, Patchset 4:
// Crashpad and Chromium has migrated off of the non-noreturn version. This isMark Mentovai . resolved
Peter Boströmhave
Done
Related details
Attention set is empty
Submit Requirements:
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Peter Boström (Gerrit)
May 16, 2024, 3:10:29 PMMay 16
to Mark Mentovai, crashp...@chromium.org
Peter Boström submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
4 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: base/notreached.h
Insertions: 2, Deletions: 2.
@@ -8,8 +8,8 @@
#include "base/check.h"
// TODO(crbug.com/40580068): Redefine NOTREACHED() to be [[noreturn]] once
-// Crashpad and Chromium has migrated off of the non-noreturn version. This is
-// easiest done by defining it as std::abort() as crashpad currently doesn't
+// Crashpad and Chromium have migrated off of the non-noreturn version. This is
+// easiest done by defining it as std::abort() as Crashpad currently doesn't
// stream arguments to it. For a more complete implementation we should use
// LOG(FATAL) but that is currently not annotated as [[noreturn]] because
// ~LogMessage is not. See TODO in base/logging.h
```
Change information
Commit message:
Add NOTREACHED_IN_MIGRATION()
This is to be used by crashpad temporarily NOTREACHED() migrates to be
[[noreturn]].
Bug: chromium:40580068
Change-Id: Ib3490671364190e9917218897f5627a0d79b8053
Reviewed-by: Mark Mentovai <ma...@chromium.org>
Files:
- M base/logging.h
- M base/notreached.h
- M base/threading/thread_local_storage.cc
Change size: S
Delta: 3 files changed, 14 insertions(+), 1 deletion(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Mark Mentovai
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages