[clang] nolock/noalloc attributes (PR #84983)

via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 12 16:22:56 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();
----------------
Sirraide wrote:

Assigning to `OldQType` doesn’t seem right here since it gets overwritten five lines down in C++ mode. Generally, I don’t think we really want to modify the `Old` decl here, only the new one. 

Also, this function mostly checks if declarations are compatible, from what I recall from the last time I had to change something here. There’s also `Sema::MergeCompatibleFunctionDecls`, which does most of the actual merging (as well as `Sema::MergeCXXFunctionDecl` for C++-specific things).

It might make sense to allow effects to be specified on the first declaration of a function only, and then either copy them from the old to the new decl when merging declarations or, perhaps better, only ever check the first declaration (and diagnosing/ignoring effects specified on later declarations). Unless there is a specific use case for adding/removing effects in redeclarations (or the definition), I would probably recommend going with that—if anything, it should make dealing w/ all of this here easier.

https://github.com/llvm/llvm-project/pull/84983


More information about the cfe-commits mailing list