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