[clang] [clang] Generate note on declaration for nodiscard-related attributes (PR #112289)
Yihe Li via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 15 11:08:31 PDT 2024
================
@@ -302,27 +307,43 @@ void Sema::DiagnoseUnusedExprResult(const Stmt *S, unsigned DiagID) {
if (const Decl *FD = CE->getCalleeDecl()) {
if (ShouldSuppress)
return;
- if (FD->hasAttr<PureAttr>()) {
- Diag(Loc, diag::warn_unused_call) << R1 << R2 << "pure";
- return;
- }
- if (FD->hasAttr<ConstAttr>()) {
- Diag(Loc, diag::warn_unused_call) << R1 << R2 << "const";
+
+ const InheritableAttr *A = nullptr;
+ if (const auto *PA = FD->getAttr<PureAttr>())
+ A = PA;
+ else if (const auto *CA = FD->getAttr<ConstAttr>())
+ A = CA;
+
+ if (A) {
+ StringRef type = (isa<PureAttr>(A) ? "pure" : "const");
+ Diag(Loc, diag::warn_unused_call) << R1 << R2 << type;
+ if (const auto *ND = dyn_cast<NamedDecl>(OffendingDecl)) {
+ if (!ND->getIdentifier()->getBuiltinID())
----------------
Mick235711 wrote:
As I said above, the real motivation for avoiding marking here is twofold:
1. Builtins like `isdigit` are just marked pure internally, the note "explicitly marked pure/const here" is just wrong, there is no such attribute on that line.
```cpp
int isdigit(int c) __attribute__((overloadable)); // The note actually fires on this line, which doesn't have a pure attribute
int isdigit(int c) __attribute__((overloadable)) // expected-note {{'isdigit' has been explicitly marked unavailable here}}
__attribute__((enable_if(c <= -1 || c > 255, "'c' must have the value of an unsigned char or EOF")))
__attribute__((unavailable("'c' must have the value of an unsigned char or EOF")));
void test3(int c) {
isdigit(c); // expected-warning{{ignoring return value of function declared with pure attribute}}
isdigit(10); // expected-warning{{ignoring return value of function declared with pure attribute}}
#ifndef CODEGEN
isdigit(-10); // expected-error{{'isdigit' is unavailable: 'c' must have the value of an unsigned char or EOF}}
#endif
}
```
Unless `A->getLocation()` is not the right invocation to find the attribute location, I don't think there is a "builtin" declaration line that can be referenced by the note.
2. Language builtins like `__builtin_operator_new` will just fire the note on the first use:
```cpp
void test_typo_in_args() {
__builtin_operator_new(DNE); // expected-error {{undeclared identifier 'DNE'}}
__builtin_operator_new(DNE, DNE2); // expected-error {{undeclared identifier 'DNE'}} expected-error {{'DNE2'}}
__builtin_operator_delete(DNE); // expected-error {{'DNE'}}
__builtin_operator_delete(DNE, DNE2); // expected-error {{'DNE'}} expected-error {{'DNE2'}}
}
```
In this test case (`SemaCXX/builtin-operator-new-delete.cpp`), the note is generated on the second line, i.e. first use of builtin, which is even more wrong as it is not even a declaration.
Excluding builtins from note generation fixes both errors, I think...
https://github.com/llvm/llvm-project/pull/112289
More information about the cfe-commits
mailing list