aaron.ballman added inline comments.
================
Comment at: clang/docs/ReleaseNotes.rst:179
by prioritizing ``-Wunreachable-code-fallthrough``.
+- Clang now correctly diagnoses statement attributes
``[[clang::always_inine]]` and
+ ``[[clang::noinline]]`` when used on a statement with dependent call
expressions.
----------------
================
Comment at: clang/lib/Sema/SemaStmtAttr.cpp:225-231
+ // If the call expressions lists are equal in size, we can skip
+ // previously emitted diagnostics. However, if the statement has a pack
+ // expansion, we have no way of telling which CallExpr is the instantiated
+ // version of the other. In this case, we will end up re-diagnosing in the
+ // instantiation.
+ // ie: [[clang::always_inline]] non_dependent(), (other_call<Pack>()...)
+ // will diagnose non_dependent again.
----------------
Minor cleanup; NFC
================
Comment at: clang/lib/Sema/SemaStmtAttr.cpp:241
+
+ for (unsigned I = 0; I < CEF.getCallExprs().size(); ++I) {
+ // If the original call expression already had a callee, we already
----------------
Range-based for loop using `llvm::enumerate()` so you can keep the index?
================
Comment at: clang/lib/Sema/TreeTransform.h:411-412
+#define ATTR(X)
\
+ const X##Attr *TransformStmt##X##Attr(const Stmt *OrigS, const Stmt *InstS,
\
+ const X##Attr *A) {
\
+ return getDerived().Transform##X##Attr(A);
\
----------------
So we don't get unused parameter warnings.
================
Comment at: clang/lib/Sema/TreeTransform.h:7617
for (const auto *I : S->getAttrs()) {
- const Attr *R = getDerived().TransformAttr(I);
+ const Attr *R = getDerived().TransformStmtAttr(S, SubStmt.get(), I);
AttrsChanged |= (I != R);
----------------
Hmmm, this seems a bit inconsistent -- why do we pass the `AttributedStmt` as
the first argument and not `S->getSubStmt()`? It may be totally reasonable, but
should we make the first parameter of `TransformStmtAttr()` be `const
AttributedStmt *` instead of `const Stmt *` so it's clear to callers that what
they're getting for the original is already wrapped as opposed to unwrapped
like the second argument?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146323/new/
https://reviews.llvm.org/D146323
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
