aaron.ballman added a comment.
In https://reviews.llvm.org/D43248#1035477, @jdenny wrote:
> In https://reviews.llvm.org/D43248#1035466, @aaron.ballman wrote:
> > It seems like there are some other changes than just the serialize and
> > deserialize that I'm not opposed to, but am wondering why they're needed.
> > It seems some functions are now `getFoo()` calls
> These were originally named getFoo, and my previous patch changed them to
> foo. I believe I did that to make ParamIdxArgument accessors more like
> VariadicParamIdxArgument accessors (which inherits accessors from
> VariadicArgument), but I probably shouldn't have done that. In any case,
> this new revision implements ParamIdxArgument using SimpleArgument, and that
> names accessors like getFoo.
Ahhh, thank you for the explanation, that makes sense.
>> and it seems like some declarations moved around. Are those intended as part
>> of this patch?
> Are you referring to the changes in SemaDeclAttr.cpp? Those changes are
> needed because the ParamIdx constructor now asserts that Idx is one-origin,
> but that requires validating that it's actually one-origin beforehand.
> Sorry, I should've mentioned the new asserts.
Ah, okay, thank you!
Comment at: cfe/trunk/test/Frontend/ast-attr.cpp:1-2
+// %S/../Sema/attr-print.cpp exercises many different attributes, so we reuse
+// it here to check -emit-ast for attributes.
> aaron.ballman wrote:
> > Can you move this below the RUN line?
> Sure. I'm still trying to learn the LLVM coding standards. Is this
> specified there?
Nope! I'm just used to looking at the very first line of the test to know what
it's running, and that seems consistent with other tests.
cfe-commits mailing list