[clang] nolock/noalloc attributes (PR #84983)
Doug Wyatt via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 18 08:02:10 PDT 2024
================
@@ -3922,6 +3922,42 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, NamedDecl *&OldD, Scope *S,
return true;
}
+ const auto OldFX = Old->getFunctionEffects();
+ const auto NewFX = New->getFunctionEffects();
+ if (OldFX != NewFX) {
+ const auto Diffs = FunctionEffectSet::differences(OldFX, NewFX);
+ for (const auto &Item : Diffs) {
+ const FunctionEffect *Effect = Item.first;
+ const bool Adding = Item.second;
+ if (Effect->diagnoseRedeclaration(Adding, *Old, OldFX, *New, NewFX)) {
+ Diag(New->getLocation(),
+ diag::warn_mismatched_func_effect_redeclaration)
+ << Effect->name();
+ Diag(Old->getLocation(), diag::note_previous_declaration);
+ }
+ }
+
+ const auto MergedFX = OldFX | NewFX;
+
+ // Having diagnosed any problems, prevent further errors by applying the
+ // merged set of effects to both declarations.
+ auto applyMergedFX = [&](FunctionDecl *FD) {
+ const auto *FPT = FD->getType()->getAs<FunctionProtoType>();
+ FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
+ EPI.FunctionEffects = MergedFX;
+ QualType ModQT = Context.getFunctionType(FD->getReturnType(),
+ FPT->getParamTypes(), EPI);
+
+ FD->setType(ModQT);
+ };
+
+ applyMergedFX(Old);
+ applyMergedFX(New);
+
+ OldQType = Old->getType();
----------------
dougsonos wrote:
> it’s better to avoid modifying the `Old` declaration
OK, I agree that that will make more sense.
> look at all declarations of a function (starting at the most recent one, because that one will most likely have the attribute on it)
There are quite a number of places that ask this question but I'll work through this. This is an interesting example:
```
void foo() {
auto* x = new int;
}
void foo() [[clang::nolock]];
void bar() [[clang::nolock]] {
foo();
}
```
I think that `foo()` ought to get verified because by the time verification happens, the redeclaration has been seen.
I'm having difficulty testing this though, because now that the `Old` declaration is untouched, `MergeFunctionDecl` is unhappy that `New` has a different canonical type. Would it make sense to add another check near the end like this one?
```
// Check if the function types are compatible when pointer size address
// spaces are ignored.
if (Context.hasSameFunctionTypeIgnoringPtrSizes(OldQType, NewQType))
return false;
```
https://github.com/llvm/llvm-project/pull/84983
More information about the cfe-commits
mailing list