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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to