aaron.ballman added a comment.
I believe this has the incorrect modeling -- in the following code example, the
attribute appertains to the substatement of the if statement, not the if
statement itself:
if (foo) [[likely]] {
blah;
}
This falls out from: http://eel.is/c++draft/stmt.stmt#1. You model this by
applying the attribute to the `if` statement, which will have the wrong
semantics when you hook up the hints to the backend.
================
Comment at: clang/test/SemaCXX/cxx2a-likely-attr.cpp:27
+ switch (n) {
+ [[likely]] return; // expected-error {{likely annotation can't appear
here}}
+
----------------
Why? The attribute appertains to statements, and `return` is a statement. This
should be accepted, I believe.
================
Comment at: clang/test/SemaCXX/cxx2a-likely-attr.cpp:29
+
+ case 0:
+ if (1) [[likely, likely]] // expected-error {{attribute 'likely' cannot
appear multiple times in an attribute specifier}}
----------------
Missing a test case that the attribute should apply to labels like this (should
also have a test for non-switch labels as well).
================
Comment at: clang/test/SemaCXX/cxx2a-likely-attr.cpp:30
+ case 0:
+ if (1) [[likely, likely]] // expected-error {{attribute 'likely' cannot
appear multiple times in an attribute specifier}}
+ return ;
----------------
Also missing the test case covering
http://eel.is/c++draft/dcl.attr.likelihood#1.sentence-3
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59467/new/
https://reviews.llvm.org/D59467
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
