[PATCH] D148381: [Clang] Implement the 'counted_by' attribute
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 2 11:45:54 PDT 2023
aaron.ballman added a comment.
In D148381#4652056 <https://reviews.llvm.org/D148381#4652056>, @void wrote:
> FINALLY! found out how to do suggestions for typos.
<3 Thank you for your perseverance!
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:8420-8428
+ if (Result.getResultKind() == LookupResult::Found) {
+ SourceRange SR = CBA->getCountedByFieldLoc();
+ Diag(SR.getBegin(),
+ diag::err_flexible_array_counted_by_attr_field_not_found_in_struct)
+ << CBA->getCountedByField() << SR;
+
+ SR = Result.getAsSingle<NamedDecl>()->getSourceRange();
----------------
The logic here still seems incorrect. I was expecting the code to look more like this:
```
bool Sema::CheckCountedByAttr(Scope *S, const FieldDecl *FD) {
const RecordDecl *RD = FD->getParent();
const auto *CBA = FD->getAttr<CountedByAttr>();
const IdentifierInfo *FieldName = CBA->getCountedByField();
DeclarationNameInfo NameInfo(FieldName,
CBA->getCountedByFieldLoc().getBegin());
LookupResult Result(*this, NameInfo, Sema::LookupMemberName);
LookupName(Result, S);
if (Result.empty()) {
CXXScopeSpec SS;
DeclFilterCCC<FieldDecl> Filter(const_cast<IdentifierInfo *>(FieldName));
if (DiagnoseEmptyLookup(S, SS, Result, Filter, nullptr, std::nullopt,
const_cast<DeclContext *>(FD->getDeclContext())))
return true;
}
const FieldDecl *Field = Result.getAsSingle<FieldDecl>();
LangOptions::StrictFlexArraysLevelKind StrictFlexArraysLevel =
Context.getLangOpts().getStrictFlexArraysLevel();
...
```
I tested this locally on code like:
```
struct not_a_fam {
int foo;
int fam[] __attribute__((counted_by(fob)));
};
```
and get a diagnostic like:
```
C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:3:39: error: use of undeclared identifier 'fob'; did you
mean 'foo'?
3 | int fam[] __attribute__((counted_by(fob)));
| ^~~
| foo
C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:2:7: note: 'foo' declared here
2 | int foo;
| ^
1 error generated.
```
Note, I had to add a constructor to `DeclFilterCCC` to expose the base class constructor, and modify `DiagnoseEmptyLookup()` like this:
```
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 2ed31a90c5dc..3c4ade391a5e 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -2458,7 +2458,8 @@ bool Sema::DiagnoseDependentMemberLookup(const LookupResult &R) {
bool Sema::DiagnoseEmptyLookup(Scope *S, CXXScopeSpec &SS, LookupResult &R,
CorrectionCandidateCallback &CCC,
TemplateArgumentListInfo *ExplicitTemplateArgs,
- ArrayRef<Expr *> Args, TypoExpr **Out) {
+ ArrayRef<Expr *> Args, DeclContext *LookupCtx,
+ TypoExpr **Out) {
DeclarationName Name = R.getLookupName();
unsigned diagnostic = diag::err_undeclared_var_use;
@@ -2474,7 +2475,9 @@ bool Sema::DiagnoseEmptyLookup(Scope *S, CXXScopeSpec &SS, LookupResult &R,
// unqualified lookup. This is useful when (for example) the
// original lookup would not have found something because it was a
// dependent name.
- DeclContext *DC = SS.isEmpty() ? CurContext : nullptr;
+ DeclContext *DC =
+ LookupCtx ? LookupCtx : (SS.isEmpty() ? CurContext : nullptr);
+ DeclContext *OrigLookupCtx = DC;
while (DC) {
if (isa<CXXRecordDecl>(DC)) {
LookupQualifiedName(R, DC);
@@ -2517,12 +2520,12 @@ bool Sema::DiagnoseEmptyLookup(Scope *S, CXXScopeSpec &SS, LookupResult &R,
emitEmptyLookupTypoDiagnostic(TC, *this, SS, Name, TypoLoc, Args,
diagnostic, diagnostic_suggest);
},
- nullptr, CTK_ErrorRecovery);
+ nullptr, CTK_ErrorRecovery, OrigLookupCtx);
if (*Out)
return true;
- } else if (S &&
- (Corrected = CorrectTypo(R.getLookupNameInfo(), R.getLookupKind(),
- S, &SS, CCC, CTK_ErrorRecovery))) {
+ } else if (S && (Corrected = CorrectTypo(R.getLookupNameInfo(),
+ R.getLookupKind(), S, &SS, CCC,
+ CTK_ErrorRecovery, OrigLookupCtx))) {
std::string CorrectedStr(Corrected.getAsString(getLangOpts()));
bool DroppedSpecifier =
Corrected.WillReplaceSpecifier() && Name.getAsString() == CorrectedStr;
@@ -2812,7 +2815,7 @@ Sema::ActOnIdExpression(Scope *S, CXXScopeSpec &SS,
// a template name, but we happen to have always already looked up the name
// before we get here if it must be a template name.
if (DiagnoseEmptyLookup(S, SS, R, CCC ? *CCC : DefaultValidator, nullptr,
- std::nullopt, &TE)) {
+ std::nullopt, nullptr, &TE)) {
if (TE && KeywordReplacement) {
auto &State = getTypoExprState(TE);
auto BestTC = State.Consumer->getNextCorrection();
```
Can you give something like this a shot and see how well it works for you?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148381/new/
https://reviews.llvm.org/D148381
More information about the cfe-commits
mailing list