[PATCH] D24507: Add attribute for return values that shouldn't be cast to bool

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D24507#546241, @urusant wrote:

> Thank you for the feedback.
>
> > The patch is missing Sema tests for the attribute (that it only applies to declarations you expect, accepts no args, etc).
>
>
> There is one test case for that in test/ReturnNonBoolTestCompileTime.cpp. I've added another one for attribute accepting no args, so now the last two test cases in this file are those you were asking about. Can you think of any other cases of invalid attribute usage?


We try to keep our tests segregated by functionality. e.g., tests relating to the way the attribute is handled (what it appertains to, args, etc) should live in Sema, tests relating to the static analyzer behavior should live in test/Analysis, etc.

Tests that are still missing are: applying to a non-function type, applying to a member function, applying to an Obj-C method. For member functions, what should happen if the function is virtual? What if the overriders do not specify the attribute? What if an override specifies the attribute but the base does not?

> > Have you considered making this a type attribute on the return type of the function rather than a declaration attribute on the function declaration?

> 

> 

> No, I hadn't. On a quick look though, I couldn't find a way to simplify my solution using this idea, because as far as I understand, the type attribute isn't inherited, so, for example, if I have something like `int r = X509_verify_cert(...)` and the function `X509_verify_cert` has a return type with attribute, `r` won't have the attribute. If that is correct, we still need to backtrace the value to the function declaration. Is there something I am missing?


I was thinking it would be diagnosed if you attempted to assign from your attributed type to a type that is not compatible. However, that may still be problematic because it raises other questions (can you SFINAE on it? Overload? etc).


Repository:
  rL LLVM

https://reviews.llvm.org/D24507