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 <riccib...@gmail.com > <mailto:riccib...@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@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> > <mailto:cfe-commits@lists.llvm.org <mailto:cfe-commits@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@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> > <mailto:cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> > > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > > _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
