r348131 - [AST] Fix an uninitialized bug in the bits of FunctionDecl

Bruno Ricci via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 4 09:15:24 PST 2018


Got it. Thanks for the review!

Bruno

On 04/12/2018 16:59, David Blaikie wrote:
> Ah, thanks for the explanation! No worries about pre-commit review or anything - this is what post-commit review is :) Only note for the future is that it might be worth mentioning in the body of the commit message (title/first line was fine) so it's clear why this "extra" work is being done.
> 
> Thanks!
> - Dave
> 
> On Tue, Dec 4, 2018 at 7:54 AM Bruno Ricci <riccibrun at gmail.com <mailto:riccibrun at gmail.com>> wrote:
> 
>     There is two reasons for this change:
> 
>     1.) The first one is that the bit FunctionDeclBits.IsCopyDeductionCandidate
>         is only used by CXXDeductionGuideDecl and so there is no getter/setter
>         for it in FunctionDecl, and it would not make sense to add one.
>         This is a legacy of when these bits where stored in FunctionDecl itself.
> 
>     2.) The second one is that some of these setter do a little bit more than
>         initializing the appropriate bit. For example setInlineSpecified sets both
>         FunctionDeclBits.IsInlineSpecified and FunctionDeclBits.IsInline, but a quick
>         reading of the body of the constructor of FunctionDecl would lead someone to
>         believe that FunctionDeclBits.IsInline is not initialized.
> 
>     However these are not strong reasons, and I can revert it to use the setters
>     instead if preferred. My apologies if it would have been preferable to review
>     it first. It seemed obvious to me and I was familiar with this piece of code
>     since I did the change a few month ago, but I am still trying to fine tune
>     this decision process.
> 
>     Cheers,
> 
>     Bruno
> 
>     On 03/12/2018 22:01, David Blaikie wrote:
>     > Why the change from using setter functions to direct assignment?
>     >
>     > On Mon, Dec 3, 2018 at 5:06 AM Bruno Ricci via cfe-commits <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org> <mailto:cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>>> wrote:
>     >
>     >     Author: brunoricci
>     >     Date: Mon Dec  3 05:04:10 2018
>     >     New Revision: 348131
>     >
>     >     URL: http://llvm.org/viewvc/llvm-project?rev=348131&view=rev
>     >     Log:
>     >     [AST] Fix an uninitialized bug in the bits of FunctionDecl
>     >
>     >     FunctionDeclBits.IsCopyDeductionCandidate was not initialized.
>     >     This caused a warning with valgrind.
>     >
>     >
>     >     Modified:
>     >         cfe/trunk/lib/AST/Decl.cpp
>     >
>     >     Modified: cfe/trunk/lib/AST/Decl.cpp
>     >     URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=348131&r1=348130&r2=348131&view=diff
>     >     ==============================================================================
>     >     --- cfe/trunk/lib/AST/Decl.cpp (original)
>     >     +++ cfe/trunk/lib/AST/Decl.cpp Mon Dec  3 05:04:10 2018
>     >     @@ -2653,27 +2653,29 @@ FunctionDecl::FunctionDecl(Kind DK, ASTC
>     >            DeclContext(DK), redeclarable_base(C), ODRHash(0),
>     >            EndRangeLoc(NameInfo.getEndLoc()), DNLoc(NameInfo.getInfo()) {
>     >        assert(T.isNull() || T->isFunctionType());
>     >     -  setStorageClass(S);
>     >     -  setInlineSpecified(isInlineSpecified);
>     >     -  setExplicitSpecified(false);
>     >     -  setVirtualAsWritten(false);
>     >     -  setPure(false);
>     >     -  setHasInheritedPrototype(false);
>     >     -  setHasWrittenPrototype(true);
>     >     -  setDeletedAsWritten(false);
>     >     -  setTrivial(false);
>     >     -  setTrivialForCall(false);
>     >     -  setDefaulted(false);
>     >     -  setExplicitlyDefaulted(false);
>     >     -  setHasImplicitReturnZero(false);
>     >     -  setLateTemplateParsed(false);
>     >     -  setConstexpr(isConstexprSpecified);
>     >     -  setInstantiationIsPending(false);
>     >     -  setUsesSEHTry(false);
>     >     -  setHasSkippedBody(false);
>     >     -  setWillHaveBody(false);
>     >     -  setIsMultiVersion(false);
>     >     -  setHasODRHash(false);
>     >     +  FunctionDeclBits.SClass = S;
>     >     +  FunctionDeclBits.IsInline = isInlineSpecified;
>     >     +  FunctionDeclBits.IsInlineSpecified = isInlineSpecified;
>     >     +  FunctionDeclBits.IsExplicitSpecified = false;
>     >     +  FunctionDeclBits.IsVirtualAsWritten = false;
>     >     +  FunctionDeclBits.IsPure = false;
>     >     +  FunctionDeclBits.HasInheritedPrototype = false;
>     >     +  FunctionDeclBits.HasWrittenPrototype = true;
>     >     +  FunctionDeclBits.IsDeleted = false;
>     >     +  FunctionDeclBits.IsTrivial = false;
>     >     +  FunctionDeclBits.IsTrivialForCall = false;
>     >     +  FunctionDeclBits.IsDefaulted = false;
>     >     +  FunctionDeclBits.IsExplicitlyDefaulted = false;
>     >     +  FunctionDeclBits.HasImplicitReturnZero = false;
>     >     +  FunctionDeclBits.IsLateTemplateParsed = false;
>     >     +  FunctionDeclBits.IsConstexpr = isConstexprSpecified;
>     >     +  FunctionDeclBits.InstantiationIsPending = false;
>     >     +  FunctionDeclBits.UsesSEHTry = false;
>     >     +  FunctionDeclBits.HasSkippedBody = false;
>     >     +  FunctionDeclBits.WillHaveBody = false;
>     >     +  FunctionDeclBits.IsMultiVersion = false;
>     >     +  FunctionDeclBits.IsCopyDeductionCandidate = false;
>     >     +  FunctionDeclBits.HasODRHash = false;
>     >      }
>     >
>     >      void FunctionDecl::getNameForDiagnostic(
>     >
>     >
>     >     _______________________________________________
>     >     cfe-commits mailing list
>     >     cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org> <mailto:cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>>
>     >     http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>     >
> 


More information about the cfe-commits mailing list