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. + ---------------- jdenny wrote: > 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. https://reviews.llvm.org/D43248 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
