[llvm-commits] [llvm] r48289 - in /llvm/trunk: include/llvm/ include/llvm/Support/ lib/AsmParser/ lib/Bitcode/Reader/ lib/Bitcode/Writer/ lib/Target/CBackend/ lib/Target/X86/ lib/Transforms/IPO/ lib/Transforms/Scalar/ lib/VMCore/ tools/llvm-upgrade/ tools/llvm2cpp/

[Duncan Sands]

Hi Chris,


> +  /// getParamAttrs - Return the parameter attributes for this function.
...
> +  /// setParamAttrs - Set the parameter attributes for this Function.

It is "function" in one comment and "Function" in the other...

> +  /// getParamAttrs - Return the PAListPtr for the parameter attributes of this
> +  /// call.

Why not just "Return the parameter attributes of this call."?

> +  /// setParamAttrs - Sets the parameter attributes for this CallInst.

It is "call" in one comment and "CallInst" in the other...

> +  /// getParamAttrs - Return the parameter attribute list for this invoke.

"parameter attribute list" -> "parameter attributes"

> +  /// setParamAttrs - Set the parameter attribute list for this invoke.

Likewise.

> +  /// get - Return a ParamAttr list with the parameters specified by the
> +  /// consequtive random access iterator range.

consequtive -> consecutive

> +  /// operator< - Provide an ordering for parameter attribute lists.
> +  bool operator==(const PAListPtr &RHS) const { return PAList == RHS.PAList; }

In spite of the comment, this is "==" not "<"...

> -    if (PAL) {
> -      ParameterAttributes Attrs = PAL->getParamAttrs(ParamIndex);
> -      if (Attrs != ParamAttr::None)
> -        ParamAttrsVec.push_back(ParamAttrsWithIndex::get(ParamIndex - 1,
> -                                                         Attrs));
> -    }
> +    if (ParameterAttributes Attrs = PAL.getParamAttrs(ParamIndex))
> +      ParamAttrsVec.push_back(ParamAttrsWithIndex::get(ParamIndex - 1, Attrs));

So adding ParamAttr::None is harmless now?

> +  // ParamAttrsList is uniqued, these should not be publicly available

Missing full stop.

Ciao,

Duncan.