================ @@ -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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits