Skip to content

Commit

Permalink
Replace several NSAssert with FML_CHECK/DCHECK to unblock the build (#…
Browse files Browse the repository at this point in the history
…53048)

In flutter/buildroot#860, disabling `NSAssert` in release build was added, and #53005 tries to roll the change into the engine. However, the change is blocked due to several compilation errors in the engine.

This is due to the fact that Google's `DCHECK`(in flutter engine, `FML_DCHECK`) does not make unused variable warning even in release build, while traditional assertion macro like `assert` or `NSAssert` does. For example, the following code will be expanded like this:
code:
```c++
FML_DCHECK(status == kCVReturnSuccess && pxbuffer != nullptr) <<
   "Failed to create pixel buffer";
```
expanded code:
```c++
true || (status == kCVReturnSuccess && pxbuffer != nullptr)
    ? (void)0 ::
    : fml::LogMessageVoidify() &
          ::fml::LogMessage(::fml::kLogFatal, 0, 0, nullptr).stream()
```

However, equivalent code with `NSAssert` will make unused variable warning, since the expanded code doesn't have any reference to the variable:
code:
```c++
NSAssert(status == kCVReturnSuccess && pxbuffer != NULL, @"Failed to create pixel buffer.");
```

expanded code:
```c++
do {
} while (0)
```
To unblock the build, this CL replaces several `NSAssert` with `FML_DCHECK` in the engine codebase. This CL isn't aimed to replace all `NSAssert` with `FML_DCHECK` since discussions in flutter/buildroot#860 are not finalized whether to replace all `NSAssert` with `FML_DCHECK` or not.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
  • Loading branch information
bc-lee authored May 30, 2024
1 parent e4d3bb0 commit 3c7a5c6
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ - (CVPixelBufferRef)pixelBuffer {
CVPixelBufferRef pxbuffer = NULL;
CVReturn status = CVPixelBufferCreate(kCFAllocatorDefault, _width, _width, _pixelFormatType,
(__bridge CFDictionaryRef)options, &pxbuffer);
NSAssert(status == kCVReturnSuccess && pxbuffer != NULL, @"Failed to create pixel buffer.");
FML_CHECK(status == kCVReturnSuccess && pxbuffer != nullptr) << "Failed to create pixel buffer";
return pxbuffer;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,10 @@ static uint64_t GetLogicalKeyForEvent(NSEvent* event, uint64_t physicalKey) {
uint32_t* keyLabel = DecodeUtf16(keyLabelUtf16, &keyLabelLength);
if (keyLabelLength == 1) {
uint32_t keyLabelChar = *keyLabel;
NSCAssert(!IsControlCharacter(keyLabelChar) && !IsUnprintableKey(keyLabelChar),
@"Unexpected control or unprintable keylabel 0x%x", keyLabelChar);
NSCAssert(keyLabelChar <= 0x10FFFF, @"Out of range keylabel 0x%x", keyLabelChar);
FML_DCHECK(!IsControlCharacter(keyLabelChar) && !IsUnprintableKey(keyLabelChar))
<< "Unexpected control or unprintable keylabel 0x" << std::hex << keyLabelChar;
FML_DCHECK(keyLabelChar <= 0x10FFFF)
<< "Out of range keylabel 0x" << std::hex << keyLabelChar;
character = keyLabelChar;
}
delete[] keyLabel;
Expand Down

0 comments on commit 3c7a5c6

Please sign in to comment.