[clang] NFC: Remove forced type merging in multiversion function checks.

[tahonermann]

Summary

Checking of multiversion function declarations performed by various functions
in clang/lib/Sema/SemaDecl.cpp previously forced the valus of a passed in
'MergeTypeWithPrevious' reference argument in several scenarios. This was
unnecessary and possibly incorrect in the one case that the value
was forced to 'true' (though seemingly unobservably so).

[aaron.ballman]

You should fix the clang-format issues (I leave @erichkeane to sign off on this one, but it looks reasonable to me).

[erichkeane]

Hmm... my understanding is "MergeTypeWithPrevious" is important for cases where we decide that this is a redeclaration, right? We need to set it to 'false' when we "know" that this is a different declaration, right? So at least the '=false' was correct?

[tahonermann]

Hmm... my understanding is "MergeTypeWithPrevious" is important for cases where we decide that this is a redeclaration, right? We need to set it to 'false' when we "know" that this is a different declaration, right? So at least the '=false' was correct?

As best as I can tell, MergeTypeWithPrevious is only true when 1) compiling as C, 2) the previous declaration (if any) is not shadowed, and 3) there is no visible previous declaration and there is a conflicting non-visible declaration. When already set to true, setting it to false would likely defeat some later diagnostics.

I spent a while trying to intuit a test that demonstrated different behavior before and after this change and wasn't able to identify one.

[erichkeane]

Ok then, I seem to then just not understand its purpose, but you seem to understand it well enough. I'm ok with this patch now (after the format fixes!).

[tahonermann]

Updates to address clang-format complaints.

[tahonermann]

Fix bad merge conflict resolution that I failed to even test compiling.

[erichkeane]

Still OK to me.

clang/lib/Sema/SemaDecl.cpp

if (D->getAsFunction()->getMultiVersionKind() != MultiVersionKind::None)
       return true;
   }
 
   return false;
 }
 
 static bool CheckTargetCausesMultiVersioning(
     Sema &S, FunctionDecl *OldFD, FunctionDecl *NewFD, const TargetAttr *NewTA,
-    bool &Redeclaration, NamedDecl *&OldDecl, bool &MergeTypeWithPrevious,
-    LookupResult &Previous) {
+    bool &Redeclaration, NamedDecl *&OldDecl, LookupResult &Previous) {
   const auto *OldTA = OldFD->getAttr<TargetAttr>();
   ParsedTargetAttr NewParsed = NewTA->parse();
   // Sort order doesn't matter, it just needs to be consistent.
   llvm::sort(NewParsed.Features);
 
   // If the old decl is NOT MultiVersioned yet, and we don't cause that
   // to change, this is a simple redeclaration.
   if (!NewTA->isDefaultVersion() &&
if (PreviousDeclsHaveMultiVersionAttribute(FD) &&
       NewFD->setInvalidDecl();
       return true;
     }
   }
 
   OldFD->setIsMultiVersion();
   NewFD->setIsMultiVersion();
   Redeclaration = false;
-  MergeTypeWithPrevious = false;
   OldDecl = nullptr;
   Previous.clear();
   return false;
 }
 
 static bool MultiVersionTypesCompatible(MultiVersionKind Old,
                                         MultiVersionKind New) {
   if (Old == New || Old == MultiVersionKind::None ||
       New == MultiVersionKind::None)
     return true;
 
   return (Old == MultiVersionKind::CPUDispatch &&
           New == MultiVersionKind::CPUSpecific) ||
          (Old == MultiVersionKind::CPUSpecific &&
           New == MultiVersionKind::CPUDispatch);
 }
 
 /// Check the validity of a new function declaration being added to an existing
 /// multiversioned declaration collection.
 static bool CheckMultiVersionAdditionalDecl(
     Sema &S, FunctionDecl *OldFD, FunctionDecl *NewFD,
     MultiVersionKind NewMVType, const TargetAttr *NewTA,
     const CPUDispatchAttr *NewCPUDisp, const CPUSpecificAttr *NewCPUSpec,
     const TargetClonesAttr *NewClones, bool &Redeclaration, NamedDecl *&OldDecl,
-    bool &MergeTypeWithPrevious, LookupResult &Previous) {
+    LookupResult &Previous) {
 
   MultiVersionKind OldMVType = OldFD->getMultiVersionKind();
   // Disallow mixing of multiversioning types.
   if (!MultiVersionTypesCompatible(OldMVType, NewMVType)) {
     S.Diag(NewFD->getLocation(), diag::err_multiversion_types_mixed);
     S.Diag(OldFD->getLocation(), diag::note_previous_declaration);
     NewFD->setInvalidDecl();
     return true;
case MultiVersionKind::Target: {
         return true;
       }
       break;
     }
     case MultiVersionKind::TargetClones: {
       const auto *CurClones = CurFD->getAttr<TargetClonesAttr>();
       Redeclaration = true;
       OldDecl = CurFD;
-      MergeTypeWithPrevious = true;
       NewFD->setIsMultiVersion();
 
       if (CurClones && NewClones &&
           (CurClones->featuresStrs_size() != NewClones->featuresStrs_size() ||
            !std::equal(CurClones->featuresStrs_begin(),
                        CurClones->featuresStrs_end(),
                        NewClones->featuresStrs_begin()))) {
         S.Diag(NewFD->getLocation(), diag::err_target_clone_doesnt_match);
if (!OldFD->isMultiVersion()) {
     NewFD->setIsMultiVersion();
     Redeclaration = true;
     OldDecl = OldFD;
     return false;
   }
 
   NewFD->setIsMultiVersion();
   Redeclaration = false;
-  MergeTypeWithPrevious = false;
   OldDecl = nullptr;
   Previous.clear();
   return false;
 }
 
 /// Check the validity of a mulitversion function declaration.
 /// Also sets the multiversion'ness' of the function itself.
 ///
 /// This sets NewFD->isInvalidDecl() to true if there was an error.
 ///
 /// Returns true if there was an error, false otherwise.
 static bool CheckMultiVersionFunction(Sema &S, FunctionDecl *NewFD,
                                       bool &Redeclaration, NamedDecl *&OldDecl,
-                                      bool &MergeTypeWithPrevious,
                                       LookupResult &Previous) {
   const auto *NewTA = NewFD->getAttr<TargetAttr>();
   const auto *NewCPUDisp = NewFD->getAttr<CPUDispatchAttr>();
   const auto *NewCPUSpec = NewFD->getAttr<CPUSpecificAttr>();
   const auto *NewClones = NewFD->getAttr<TargetClonesAttr>();
   MultiVersionKind MVType = NewFD->getMultiVersionKind();
 
   // Main isn't allowed to become a multiversion function, however it IS
if (OldFD->isMultiVersion() && MVType == MultiVersionKind::None &&
     NewFD->setInvalidDecl();
     return true;
   }
 
   if (!OldFD->isMultiVersion()) {
     switch (MVType) {
     case MultiVersionKind::Target:
       return CheckTargetCausesMultiVersioning(S, OldFD, NewFD, NewTA,
-                                              Redeclaration, OldDecl,
-                                              MergeTypeWithPrevious, Previous);
+                                              Redeclaration, OldDecl, Previous);
     case MultiVersionKind::TargetClones:
       if (OldFD->isUsed(false)) {
         NewFD->setInvalidDecl();
         return S.Diag(NewFD->getLocation(), diag::err_multiversion_after_used);
       }
       OldFD->setIsMultiVersion();
       break;
     case MultiVersionKind::CPUDispatch:
     case MultiVersionKind::CPUSpecific:
     case MultiVersionKind::None:
       break;
     }
   }
 
   // At this point, we have a multiversion function decl (in OldFD) AND an
   // appropriate attribute in the current function decl.  Resolve that these are
   // still compatible with previous declarations.
-  return CheckMultiVersionAdditionalDecl(
-      S, OldFD, NewFD, MVType, NewTA, NewCPUDisp, NewCPUSpec, NewClones,
-      Redeclaration, OldDecl, MergeTypeWithPrevious, Previous);
+  return CheckMultiVersionAdditionalDecl(S, OldFD, NewFD, MVKind, NewTA,
+                                         NewCPUDisp, NewCPUSpec, NewClones,
+                                         Redeclaration, OldDecl, Previous);
 }
 
 /// Perform semantic checking of a new function declaration.
 ///
 /// Performs semantic analysis of the new function declaration
 /// NewFD. This routine performs all semantic checking that does not
 /// require the actual declarator involved in the declaration, and is
 /// used both for the declaration of functions as they are parsed
if (!Previous.empty()) {
           MayNeedOverloadableChecks = true;
           Redeclaration = false;
           OldDecl = nullptr;
         }
       }
     }
   }
 
-  if (CheckMultiVersionFunction(*this, NewFD, Redeclaration, OldDecl,
-                                MergeTypeWithPrevious, Previous))
+  if (CheckMultiVersionFunction(*this, NewFD, Redeclaration, OldDecl, Previous))
     return Redeclaration;
 
   // PPC MMA non-pointer types are not allowed as function return types.
   if (Context.getTargetInfo().getTriple().isPPC64() &&
       CheckPPCMMAType(NewFD->getReturnType(), NewFD->getLocation())) {
     NewFD->setInvalidDecl();
   }