Bug 195205

Summary: Fix debug builds with GCC
Product: WebKit Reporter: Dominik Inführ <dominik.infuehr>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, csaavedra, ews-watchlist, fred.wang, keith_miller, mark.lam, msaboff, saam, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Dominik Inführ 2019-03-01 03:02:51 PST
Fix debug builds with GCC
Comment 1 Dominik Inführ 2019-03-01 03:04:14 PST
Created attachment 363319 [details]
Patch
Comment 2 WebKit Commit Bot 2019-03-01 03:43:38 PST
Comment on attachment 363319 [details]
Patch

Clearing flags on attachment: 363319

Committed r242262: <https://trac.webkit.org/changeset/242262>
Comment 3 WebKit Commit Bot 2019-03-01 03:43:39 PST
All reviewed patches have been landed.  Closing bug.
Comment 4 Radar WebKit Bug Importer 2019-03-01 03:44:16 PST
<rdar://problem/48506653>
Comment 5 Claudio Saavedra 2019-03-01 03:46:48 PST
Why not add a ASSERT_NOT_REACHED_UNDER_CONSTEXPR that would expand to ASSERT_UNDER_CONSTEXPR(false) instead? Removing the constexpr you're dropping its benefits for all builds, just for the sake of the few that expand the macro (debug builds).
Comment 6 Dominik Inführ 2019-03-01 04:04:12 PST
I would have preferred that solution as well: But using ASSERT_UNDER_CONSTEXPR_CONTEXT(false) failed to build for me as well. The macro invokes WTFReportAssertionFailure which isn't constexpr. Since Saam recommended to remove the constexpr keyword in https://bugs.webkit.org/show_bug.cgi?id=194912 I used this solution to get the build working again.
Comment 7 Claudio Saavedra 2019-03-01 05:02:35 PST
+fredw

> The macro invokes WTFReportAssertionFailure which isn't constexpr.

Isn't that then a bug in ASSERT_UNDER_CONSTEXPR_CONTEXT()?
Comment 8 Frédéric Wang (:fredw) 2019-03-01 05:19:49 PST
(In reply to Claudio Saavedra from comment #7)
> +fredw
> 
> > The macro invokes WTFReportAssertionFailure which isn't constexpr.
> 
> Isn't that then a bug in ASSERT_UNDER_CONSTEXPR_CONTEXT()?

I think this is expected, as the name indicates it only works under a constexpr context. cc'in Yusuke since he is the one who initially introduced this.
Comment 9 Claudio Saavedra 2019-03-01 05:38:09 PST
(In reply to Frédéric Wang (:fredw) from comment #8)
> (In reply to Claudio Saavedra from comment #7)
> > +fredw
> > 
> > > The macro invokes WTFReportAssertionFailure which isn't constexpr.
> > 
> > Isn't that then a bug in ASSERT_UNDER_CONSTEXPR_CONTEXT()?
> 
> I think this is expected, as the name indicates it only works under a
> constexpr context. cc'in Yusuke since he is the one who initially introduced
> this.

But it's ASSERT_UNDER_CONSTEXPR_CONTEXT() that ends up calling WTFReportAssertionFailure(), which is not constexpr. I understood that the idea of ASSERT_UNDER_CONSTEXPR_CONTEXT() was to allow for assertions when the calling methods are constexpr, but it won't work.

Unless I'm understanding it completely wrong, of course :)