[PATCH] D62784: [NFC] Include short string attributes in the "Function Attrs" comment

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

In D62784#1528498 <https://reviews.llvm.org/D62784#1528498>, @pcc wrote:

> This change seems kind of arbitrary to me. Not all attributes that we might want to test for are shorter than 15 characters. Some are already longer and they can't be changed because of bitcode compatibility.


While I guess we could introduce short names for existing long attributes, I agree with the general premise, this is "kind of arbitrary". Though, we talk about printing comments and what we do now is arguably "kind of arbitrary" as well. If you feel that it is not a good idea to print more arguments in the comment, I will not do it.

> If the goal is to make things involving attributes more easily testable, why not work towards that goal directly? For example, you could add a flag that means "print all attribute names/values in the comment" and then use that in your tests.

I did want to avoid that because we easily have 15+ attributes and new ones are added fairly often. That means testing for all of them is not a good option. Now testing for a subset is hard, especially if you want to ensure some attributes to be there and others to be absent. The way I would write it, potentially not the best way though, would involve multiple interleaved check and check-not lines.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62784/new/

https://reviews.llvm.org/D62784