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