Re: [webkit-dev] Use of [[maybe_unused]]
I think that would still use 1 extra byte per object, which isn’t ideal but we
may be ok with that.
In general [[maybe_unused]] tells the compiler to stop telling us about
potential speedups. Usually that speedup is just a value in a register or on
the stack, which has a relatively small cost, but sometimes it can be a large
cost if it’s using lots of memory. We may choose that’s ok.
> On Jan 14, 2020, at 11:55 AM, Suzuki, Basuke wrote:
>
>>> `sessionID` is used in RELEASE_LOG_IF_ALLOWED() and we have empty
>>> implementation of RELEASE_LOG() so that it's ended up with unused
>>> parameter warning of sessionID. We can add UNUSED_PARAM(sessionID) in
>>> this case, but [[maybe_unused]] is more correct choice to describe the code
>>> because sessionID is actually used.
>>
>> In this case you could use RELEASE_LOG_DISABLED but I agree
>> [[maybe_unused]] would be better. I’m ok with it as long as all the ports
>> are
>> OK with it, including support in their oldest supported compiler.
>
> Got it. I'll try sending to EWS to see it works for every ports.
>
>> I think it would be better to put #if PLATFORM(COCOA) around member
>> variables like this because I don’t think [[maybe_unused]] will remove the
>> additional byte in the structure. Otherwise we’re just wasting memory.
>
> Let us think about the cost to maintain platform dependent implementation of
> this class.
> Because the member variable is assigned in constructor, we have to make
> platform dependent
> Constructor which is usually hard to maintain and want to away from that if
> possible.
>
> How about this?
>
> template struct Unused { explicit Empty(T) { } };
>
> #if PLATFORM(COCOA)
> bool m_isHTTPSCheckEnabled;
> #else
> [[maybe_unused]] Unused m_isHTTPSCheckEnabled;
> #endif
>
>
> -
> Basuke Suzuki
> SONY PlayStation
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Use of [[maybe_unused]]
> > `sessionID` is used in RELEASE_LOG_IF_ALLOWED() and we have empty
> > implementation of RELEASE_LOG() so that it's ended up with unused
> > parameter warning of sessionID. We can add UNUSED_PARAM(sessionID) in
> > this case, but [[maybe_unused]] is more correct choice to describe the code
> > because sessionID is actually used.
>
> In this case you could use RELEASE_LOG_DISABLED but I agree
> [[maybe_unused]] would be better. I’m ok with it as long as all the ports are
> OK with it, including support in their oldest supported compiler.
Got it. I'll try sending to EWS to see it works for every ports.
> I think it would be better to put #if PLATFORM(COCOA) around member
> variables like this because I don’t think [[maybe_unused]] will remove the
> additional byte in the structure. Otherwise we’re just wasting memory.
Let us think about the cost to maintain platform dependent implementation of
this class.
Because the member variable is assigned in constructor, we have to make
platform dependent
Constructor which is usually hard to maintain and want to away from that if
possible.
How about this?
template struct Unused { explicit Empty(T) { } };
#if PLATFORM(COCOA)
bool m_isHTTPSCheckEnabled;
#else
[[maybe_unused]] Unused m_isHTTPSCheckEnabled;
#endif
-
Basuke Suzuki
SONY PlayStation
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Use of [[maybe_unused]]
> On Jan 13, 2020, at 4:08 PM, Suzuki, Basuke wrote: > > `sessionID` is used in RELEASE_LOG_IF_ALLOWED() and we have empty > implementation of RELEASE_LOG() so that it's ended up with unused parameter > warning of sessionID. We can add UNUSED_PARAM(sessionID) in this case, but > [[maybe_unused]] is more correct choice to describe the code because > sessionID is actually used. In this case you could use RELEASE_LOG_DISABLED but I agree [[maybe_unused]] would be better. I’m ok with it as long as all the ports are OK with it, including support in their oldest supported compiler. > This member variable is only used in COCOA port. > (https://github.com/WebKit/webkit/blob/master/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp#L203) > > We can add UNUSED_PARAM(isHTTPSUpgradeEnabled) in our platform code, but > adding [[maybe_unused]] in the header file is straight forward. I think it would be better to put #if PLATFORM(COCOA) around member variables like this because I don’t think [[maybe_unused]] will remove the additional byte in the structure. Otherwise we’re just wasting memory. ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
[webkit-dev] Use of [[maybe_unused]]
Hi WebKit-dev,
I'm asking about whether we can use C++ 17's new attribute [[maybe_unused]].
These are situations I cannot solve well with UNUSED_PARAM().
Case 1: The function parameter is only used in RELEASE_LOG macro which is empty
in some platform
https://github.com/WebKit/webkit/blob/master/Source/WebKit/NetworkProcess/NetworkHTTPSUpgradeChecker.cpp#L105
m_workQueue->dispatch([this, host = host.isolatedCopy(), sessionID,
callback = WTFMove(callback)] () mutable {
...
RELEASE_LOG_IF_ALLOWED(sessionID, "query - Ran successfully. Result =
%s", (foundHost ? "true" : "false"));
...
});
`sessionID` is used in RELEASE_LOG_IF_ALLOWED() and we have empty
implementation of RELEASE_LOG() so that it's ended up with unused parameter
warning of sessionID. We can add UNUSED_PARAM(sessionID) in this case, but
[[maybe_unused]] is more correct choice to describe the code because sessionID
is actually used.
I also tried to replace empty RELEASE_LOG macro with something to use every
parameter marked used somehow, but the trial just failed.
Case 2: The member variable is just used by specific port
https://github.com/WebKit/webkit/blob/master/Source/WebKit/NetworkProcess/NetworkLoadChecker.h#L155
bool m_isHTTPSUpgradeEnabled { false };
This member variable is only used in COCOA port.
(https://github.com/WebKit/webkit/blob/master/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp#L203)
We can add UNUSED_PARAM(isHTTPSUpgradeEnabled) in our platform code, but adding
[[maybe_unused]] in the header file is straight forward.
Compiler support of this attribute seems okay for any WebKit build. I've done
quick check on Godbolt. https://godbolt.org/z/w47XXn
-
Basuke Suzuki
SONY PlayStation
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev
