[PATCH] D43248: [Attr] Fix parameter indexing for attributes
Joel E. Denny via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 2 09:18:05 PST 2018
jdenny added a comment.
Aaron, thanks for the review. I've applied your suggestions and am ready to commit.
I've noticed a variety of styles in commit logs, I've read the coding standards, and it seems there's a lot of freedom here. Below are the two commit logs I'm planning to use. Please let me know if you have any objections. Thanks.
[Attr] Fix parameter indexing for several attributes
The patch fixes a number of bugs related to parameter indexing in
attributes:
* Parameter indices in some attributes (argument_with_type_tag,
pointer_with_type_tag, nonnull, ownership_takes, ownership_holds,
and ownership_returns) are specified in source as one-origin
including any C++ implicit this parameter, were stored as
zero-origin excluding any this parameter, and were erroneously
printing (-ast-print) and confusingly dumping (-ast-dump) as the
stored values.
* For alloc_size, the C++ implicit this parameter was not subtracted
correctly in Sema, leading to assert failures or to silent failures
of __builtin_object_size to compute a value.
* For argument_with_type_tag, pointer_with_type_tag, and
ownership_returns, the C++ implicit this parameter was not added
back to parameter indices in some diagnostics.
This patch fixes the above bugs and aims to prevent similar bugs in
the future by introducing careful mechanisms for handling parameter
indices in attributes. ParamIdx stores a parameter index and is
designed to hide the stored encoding while providing accessors that
require each use (such as printing) to make explicit the encoding that
is needed. Attribute declarations declare parameter index arguments
as [Variadic]ParamIdxArgument, which are exposed as ParamIdx[*]. This
patch rewrites all attribute arguments that are processed by
checkFunctionOrMethodParameterIndex in SemaDeclAttr.cpp to be declared
as [Variadic]ParamIdxArgument. The only exception is xray_log_args's
argument, which is encoded as a count not an index.
Differential Revision: https://reviews.llvm.org/D43248
For the test/Sema/attr-ownership.c change:
[Attr] Use -fsyntax-only in test
Suggested at: https://reviews.llvm.org/D43248
https://reviews.llvm.org/D43248
More information about the cfe-commits
mailing list