Quuxplusone added inline comments.
================ Comment at: include/clang/Sema/Sema.h:5337 ExprResult ActOnFinishFullExpr(Expr *Expr, SourceLocation CC, - bool DiscardedValue = false, + bool WarnOnDiscardedValue = false, bool IsConstexpr = false); ---------------- aaron.ballman wrote: > rsmith wrote: > > Why "WarnOn"? Shouldn't this flag simply indicate whether the expression is > > a discarded-value expression? > It probably can; but then it feels like the logic is backwards from the > suggested changes as I understood them. If it's a discarded value expression, > then the value being unused should *not* be diagnosed because the expression > only exists for its side effects (not its value computations), correct? Peanut gallery says: There are at least three things that need to be computed somewhere: (1) Is this expression's value discarded? (2) Is this expression the result of a `[[nodiscard]]` function? (3) Is the diagnostic enabled? It is unclear to me who's responsible for computing which of these things. I.e., it is unclear to me whether `WarnOnDiscardedValue=true` means "Hey `ActOnFinishFullExpr`, please give a warning //because// this value is being discarded" (conjunction of 1,2, and maybe 3) or "Hey `ActOnFinishFullExpr`, please give a warning //if// this value is being discarded" (conjunction of 2 and maybe 3). I also think it is needlessly confusing that `ActOnFinishFullExpr` gives `WarnOnDiscardedValue` a defaulted value of `false` but `ActOnExprStmt` gives `WarnOnDiscardedValue` a defaulted value of `true`. Defaulted values (especially of boolean type) are horrible, but context-dependent defaulted values are even worse. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55955/new/ https://reviews.llvm.org/D55955 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits