Mordante added a comment.
Herald added a subscriber: danielkiss.

In D86559#2242713 <https://reviews.llvm.org/D86559#2242713>, @aaron.ballman 
wrote:

> In D86559#2242586 <https://reviews.llvm.org/D86559#2242586>, @Mordante wrote:
>
>>> That is exactly the behavior I am coming to believe we should follow. You 
>>> can either write it on a compound statement that is guarded by a flow 
>>> control decision (`if`/`else`/`while`) or you can write it on a label, 
>>> otherwise the attribute is parsed and ignored (with a diagnostic). This 
>>> feels like the right mixture of useful with understandable semantics so 
>>> far, but perhaps we'll find other examples that change our minds.
>>>
>>> The fallthrough behavior question has one more case we may want to think 
>>> about:
>>>
>>>   switch (x) {
>>>   case 0:
>>>   case 1:
>>>   [[likely]] case 2: break;
>>>   [[unlikely]] default:
>>>   }
>>>
>>> Does this mark cases `0` and `1` as being likely or not? I could see users 
>>> wanting to use shorthand rather than repeat themselves on all the cases. 
>>> However, I'm also not certain whether there would be any performance impact 
>>> if we marked only `case 2` as likely and left cases `0` and `1` with 
>>> default likelihood. My gut feeling is that this should only mark `case 2`, 
>>> but others may have different views.
>>
>> Not according to the standard: "A path of execution includes a label if and 
>> only if it contains a jump to that label."
>
> A switch statement contains a jump to all of its labels, though. So all of 
> those labels are included in the path of execution, which is not likely 
> what's intended by the standard.

In the example above if `x == 0` there will be a jump to `case 0` which then 
falls through to `case 1` and `case 2` so `case 0` doesn't jump to `case 2` and 
thus doesn't "execute" the label.

>> if(a) {
>>
>>   [[likely]] return; // Ignored, not on the first statement
>>
>> }
>
> Agreed.
>
>> if(a)                      // Attribute not allowed on a declaration,
>>
>>   [[likely]] int i = 5;  // but I can't think about a good use-case
>>                          // for this code.
>
> This is a fun example because I can think of a somewhat reasonable use-case 
> but we can't support it without a compound statement. :-D The declaration 
> could be an RAII object,
>
>   if (a)
>     [[likely]] SomeRAIIObj Obj(*a);
>
> However, you're right that we cannot write the attribute there because a 
> declaration-statement cannot be attributed 
> (http://eel.is/c++draft/stmt.stmt#stmt.pre-1), so the attribute instead 
> appertains to the declaration and not the statement. So the user would have 
> to write:
>
>   if (a) [[likely]] {
>     SomeRAIIObj Obj(*a);
>   }
>
> That said, I think this is enough of a corner case that requiring the 
> compound statement is not really a burden. If we find users run into that 
> often (they'd get an attribute ignored error if they tried), we could add a 
> fix-it to help them out, but that doesn't seem necessary initially.

I had thought about RAII before and I think there it's also not a real issue. 
Your example does the same as:

  if (a)
     [[likely]] SomeRAIIObj{*a};
   

Here's  no declaration and the attribute is allowed. If the RAII object is used 
in a declaration I expect it usually will be inside a compound statement to 
create a scope where the object is alive. In that case the attribute is placed 
on the compound statement. I don't expect people to really write code like 
this, but it may happen when using macros.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86559/new/

https://reviews.llvm.org/D86559

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to