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

David Blaikie via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 4 08:59:01 PST 2018


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> 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>> 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>
> >     http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20181204/ff34a6f9/attachment.html>


More information about the cfe-commits mailing list