rsmith added a comment.
Another "fun" testcase:
struct S {
void operator++(int n) _diagnose_if(n, "wat", "warning");
};
void f(S s) {
s++; // no warning
s.operator++(1); // warning
}
================
Comment at: include/clang/Sema/Sema.h:2638
- /// Check the diagnose_if attributes on the given function. Returns the
- /// first succesful fatal attribute, or null if calling Function(Args) isn't
- /// an error.
+ /// Emit diagnostics for the diagnose_if attributes on Function, ignoring any
+ /// non-ArgDependent DiagnoseIfAttrs.
----------------
Can you add a comment somewhere about here explaining why these functions are
split? Something like "Argument-dependent diagnose_if attributes are checked
when the function is used as a direct callee of a function call." here, and
"Argument-independent diagnose_if attributes are checked on every use of the
function." below.
================
Comment at: include/clang/Sema/Sema.h:9925
SourceLocation Loc, SourceRange Range,
- VariadicCallType CallType);
+ VariadicCallType CallType, const Expr *ThisArg);
----------------
We have a loose convention that function parameter argument order matches
source order, which would suggest that `ThisArg` should precede `Args` here.
================
Comment at: lib/Sema/SemaExprCXX.cpp:6717
+
+ checkDiagnoseIfAttrsOnCall(Method, CE);
return CE;
----------------
Can you call `CheckFunctionCall` instead here, and remove
`checkDiagnoseIfAttrsOnCall`? It looks like the only reason we don't already
call that is because none of its checks could ever fire for a call to a
conversion function before, and that's no longer true.
================
Comment at: lib/Sema/SemaOverload.cpp:12035
+ checkDiagnoseIfAttrsOnCall(FnDecl, TheCall);
return MaybeBindToTemporary(TheCall);
----------------
Likewise call `CheckFunctionCall` here.
================
Comment at: lib/Sema/SemaOverload.cpp:12488
+ checkDiagnoseIfAttrsOnCall(Method, TheCall);
return MaybeBindToTemporary(TheCall);
----------------
... and here.
================
Comment at: lib/Sema/SemaOverload.cpp:13228
+ checkDiagnoseIfAttrsOnCall(Method, TheCall);
return MaybeBindToTemporary(TheCall);
----------------
... and here.
================
Comment at: test/SemaCXX/diagnose_if.cpp:614
+ // FIXME: This should emit diagnostics. It seems that our constexpr
+ // evaluator isn't able to evaluate `adl::Foo(1)` to a constexpr, though.
+ // I'm assuming this is because we assign it to a temporary.
----------------
`constexpr` is an adjective; "to a constant" might make more sense.
================
Comment at: test/SemaCXX/diagnose_if.cpp:615
+ // evaluator isn't able to evaluate `adl::Foo(1)` to a constexpr, though.
+ // I'm assuming this is because we assign it to a temporary.
+ for (void *p : adl::Foo(1)) {}
----------------
The range-based for is desugared to
```
auto &&__range = adl::Foo(1);
auto __begin = begin(__range);
auto __end = end(__range);
// ...
```
so the argument in the call to `begin` is not considered constant.
https://reviews.llvm.org/D28889
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
