aaron.ballman added inline comments.
================
Comment at: clang/include/clang/Basic/AttrDocs.td:1693
+The attributes are used to aid the compiler to determine which branch is
+likely or unlikely to be taken. This is done by marking the first statement in
+the branch with one of the two attributes.
----------------
How about: `This is done by marking the branch substatement with one of the two
attributes.`
================
Comment at: clang/include/clang/Basic/AttrDocs.td:1697
+It isn't allowed to annotate a single statement with both ``likely`` and
+``unlikely``. It's allowed to create a contradictions by marking
+the first statement in the ``true`` and ``false`` branch with the same
----------------
This sentence and the next one seem to say opposite things. I think this should
read:
`Annotating the 'true' and 'false' branch of an 'if' statement with the same
likelihood attribute will result in a diagnostic and the attributes are ignored
on both branches.`
================
Comment at: clang/include/clang/Basic/AttrDocs.td:1705
+
+In Clang the attributes will be ignored if they're not placed on the first
+statement in a branch. The C++ Standard recommends to honor them on every
----------------
How about: `In Clang, the attributes will be ignored if they're not placed on
the substatement of a selection statement.`
================
Comment at: clang/include/clang/Basic/AttrDocs.td:1726
+ foo(b);
+ // Whether or not the next statement is in the path of execution depends
+ // on the declaration of foo():
----------------
The formatting on this comment looks off (maybe different indentation, or tabs
vs spaces).
================
Comment at: clang/include/clang/Basic/AttrDocs.td:1736
+
+At the moment the attribute only has effect when used in an ``if`` statement.
+
----------------
`if` or `else` statement.
================
Comment at: clang/lib/AST/JSONNodeDumper.cpp:1452
attributeOnlyIfTrue("isConstexpr", IS->isConstexpr());
+ dumpLikelihood(JOS, "thenLikelihood", IS->getThenLikelihood());
}
----------------
I don't think this change should be necessary because the attribute should be
written out for the AST node already. It's worth verifying though.
================
Comment at: clang/lib/AST/TextNodeDumper.cpp:927
OS << " has_else";
+ dumpLikelihood(OS, Node->getThenLikelihood());
}
----------------
Similar here.
================
Comment at: clang/lib/Sema/SemaStmt.cpp:618
+
+ // The function returs the likelihood of Then so invert the likelihood of
+ // Else.
----------------
returs -> returns
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85091/new/
https://reviews.llvm.org/D85091
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
