[llvm] r374102 - Mark several PointerIntPair methods as lvalue-only

[David Blaikie via llvm-commits]

Hmm, actually, thinking about this again - what's the bug usage you're
trying to fix? If these functions don't return any value, what sort of
mistakes could they introduce?

It's not like someone's going to accidentally write "foo(bar().baz())"
(where baz() is the mutating-not-returning function)

Is it in cases like "bar().baz()" where bar() returns by value & the author
thought it returned by non-const-ref?

I don't think the warning or tidy-check should suggest adding a reference
qualifier - just catching the usage that's bogus (ie: diagnosing the
"bar().baz()" usage, not the definition of "baz()" itself) - because I
think there's just too much API surface area to annotate everything.

On Mon, Oct 14, 2019 at 4:31 PM Jordan Rose <jordan_rose at apple.com> wrote:

> I could see it as a clang-tidy pass. "This method modifies members of
> 'this' but does not return 'this' or use it in any way [after
> modification]; do you want to make it lvalue-only?" I wouldn't make it a
> full warning because lvalue qualifiers are still fairly esoteric and people
> may not want to add them to their codebase even if they'd catch bugs.
>
> Jordan
>
>
> On Oct 14, 2019, at 15:43, David Blaikie <dblaikie at gmail.com> wrote:
>
> Seems like a losing race to try to flag every API surface area that might
> have this problem.
>
> Is it worth considering a clang-tidy or full clang warning for cases like
> this? (& could diagnose the usage directly)
>
> On Tue, Oct 8, 2019 at 11:59 AM Jordan Rose via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: jrose
>> Date: Tue Oct  8 12:01:48 2019
>> New Revision: 374102
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=374102&view=rev
>> Log:
>> Mark several PointerIntPair methods as lvalue-only
>>
>> No point in mutating 'this' if it's just going to be thrown away.
>>
>> https://reviews.llvm.org/D63945
>>
>> Modified:
>>     llvm/trunk/include/llvm/ADT/PointerIntPair.h
>>
>> Modified: llvm/trunk/include/llvm/ADT/PointerIntPair.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/PointerIntPair.h?rev=374102&r1=374101&r2=374102&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/ADT/PointerIntPair.h (original)
>> +++ llvm/trunk/include/llvm/ADT/PointerIntPair.h Tue Oct  8 12:01:48 2019
>> @@ -13,6 +13,7 @@
>>  #ifndef LLVM_ADT_POINTERINTPAIR_H
>>  #define LLVM_ADT_POINTERINTPAIR_H
>>
>> +#include "llvm/Support/Compiler.h"
>>  #include "llvm/Support/PointerLikeTypeTraits.h"
>>  #include "llvm/Support/type_traits.h"
>>  #include <cassert>
>> @@ -59,19 +60,19 @@ public:
>>
>>    IntType getInt() const { return (IntType)Info::getInt(Value); }
>>
>> -  void setPointer(PointerTy PtrVal) {
>> +  void setPointer(PointerTy PtrVal) LLVM_LVALUE_FUNCTION {
>>      Value = Info::updatePointer(Value, PtrVal);
>>    }
>>
>> -  void setInt(IntType IntVal) {
>> +  void setInt(IntType IntVal) LLVM_LVALUE_FUNCTION {
>>      Value = Info::updateInt(Value, static_cast<intptr_t>(IntVal));
>>    }
>>
>> -  void initWithPointer(PointerTy PtrVal) {
>> +  void initWithPointer(PointerTy PtrVal) LLVM_LVALUE_FUNCTION {
>>      Value = Info::updatePointer(0, PtrVal);
>>    }
>>
>> -  void setPointerAndInt(PointerTy PtrVal, IntType IntVal) {
>> +  void setPointerAndInt(PointerTy PtrVal, IntType IntVal)
>> LLVM_LVALUE_FUNCTION {
>>      Value = Info::updateInt(Info::updatePointer(0, PtrVal),
>>                              static_cast<intptr_t>(IntVal));
>>    }
>> @@ -89,7 +90,7 @@ public:
>>
>>    void *getOpaqueValue() const { return reinterpret_cast<void *>(Value);
>> }
>>
>> -  void setFromOpaqueValue(void *Val) {
>> +  void setFromOpaqueValue(void *Val) LLVM_LVALUE_FUNCTION {
>>      Value = reinterpret_cast<intptr_t>(Val);
>>    }
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191014/e4e020a7/attachment.html>