Bug 85858 - -Weffc++ should not require copy ctor for const pointers
Summary: -Weffc++ should not require copy ctor for const pointers
Status: RESOLVED INVALID
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 8.1.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
Depends on:
Blocks: Weffc++
  Show dependency treegraph
 
Reported: 2018-05-21 15:28 UTC by Mike Sharov
Modified: 2018-05-22 13:25 UTC (History)
0 users

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2018-05-21 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Sharov 2018-05-21 15:28:03 UTC
-Weffc++ warns about missing operator= and copy ctor in a class containing a const pointer. The intent of the warning is to detect manually allocated memory owned by the class, and to ensure copying operation was explicitly considered. When the pointer is const, it can not point to owned memory and so should not result in a warning.
Comment 1 Jonathan Wakely 2018-05-21 16:59:31 UTC
(In reply to Mike Sharov from comment #0)
> When the pointer is const, it can not point to owned memory

Why not?
Comment 2 Mike Sharov 2018-05-21 19:29:12 UTC
(In reply to Jonathan Wakely from comment #1)
> (In reply to Mike Sharov from comment #0)
> > When the pointer is const, it can not point to owned memory
> Why not?

Because a const pointer can not be freed. By "owned memory" I mean memory that was explicitly allocated by the object, which I assume was the situation that Effective C++ rule was referring to, or memory the ownership of which was passed to the object. In both cases the object has to keep a non-const pointer in order to be able to free it or to pass on the ability to free it to some other object. I can't think of any case for an owned const pointer; can you?
Comment 3 Jonathan Wakely 2018-05-21 19:40:14 UTC
(In reply to Mike Sharov from comment #2)
> (In reply to Jonathan Wakely from comment #1)
> > (In reply to Mike Sharov from comment #0)
> > > When the pointer is const, it can not point to owned memory
> > Why not?
> 
> Because a const pointer can not be freed.

That's what I thought you meant, and it's wrong.

> By "owned memory" I mean memory
> that was explicitly allocated by the object, which I assume was the
> situation that Effective C++ rule was referring to, or memory the ownership
> of which was passed to the object. In both cases the object has to keep a
> non-const pointer in order to be able to free it or to pass on the ability
> to free it to some other object.

Nothing stops you deallocating a const pointer.

struct X {
  const int* const p;
  X() : p(new int[10]) { }
  ~X() { delete[] p; }
};

This type should have a user-defined copy constructor (it doesn't need a user-defined assignment operator though, because it's implicitly defined as deleted).
Comment 4 Mike Sharov 2018-05-21 19:55:43 UTC
(In reply to Jonathan Wakely from comment #3)
> Nothing stops you deallocating a const pointer.

According to http://en.cppreference.com/w/cpp/memory/new/operator_delete
The delete operator takes a void* and attempting to delete a const pointer would require a const_cast. This is logical, since freeing a memory block is a modification operation that changes the block's contents by marking it invalid.

To my surprise, I found that g++ actually does currently accept delete of a const pointer. I believe that should be a bug.
Comment 5 Jonathan Wakely 2018-05-21 20:04:39 UTC
Nope, see the C++ standard:

  [ Note: A pointer to a const type can be the operand of a delete-expression;
  it is not necessary to cast away the constness (8.5.1.11) of the pointer
  expression before it is used as the operand of the delete-expression. — end
  note ]

The deallocation function 'operator delete(void*)' is not the same as a delete-expression such as delete[] p.
Comment 6 Mike Sharov 2018-05-21 21:01:12 UTC
(In reply to Jonathan Wakely from comment #5)
> Nope, see the C++ standard:
> 
>   [ Note: A pointer to a const type can be the operand of a
> delete-expression;

Ok, I guess; you have to follow the standard, after all. But I would like to see the rationale for this, because it sure looks like a violation of const correctness. I certainly feel violated.
Comment 7 Jonathan Wakely 2018-05-21 22:50:38 UTC
It's completely logical.

  using const_int = const int;
  const int* p = new const_int();

Do you expect to never be able to delete this object, instead being forced to leak it?

What about:

void f() { const int i = 0; }

Do you think this stack variable can't be destroyed, because it's const?

The const qualifier affects modification of the object, not its destruction. Your mental model of C++ is simply not how the language works.
Comment 8 Mike Sharov 2018-05-21 23:40:45 UTC
(In reply to Jonathan Wakely from comment #7)
> Your mental model of C++ is simply not how the language works.

My mental model here is actually of const correctness, not C++ specifically. When I pass around a const object I expect it to stay unmodified. Consider a function that takes a const T* argument. The signature suggests that the passed-in object will only be read and will not be modified. If that function deletes the pointer, "bad things" will likely happen. Suddenly the object contains garbage and you can't figure out how it happened. All called functions took it as const, so they shouldn't have messed with it. You assume memory corruption and onto valgrind we go.

> The const qualifier affects modification of the object, not its destruction.

This is precisely the part I have a problem with. It seems downright loony to state that destruction is not modification when the object is most definitely modified by it. It's like saying that if I break into your house and take stuff, I am a criminal, but if I burn it down, it's all perfectly fine.

> void f() { const int i = 0; }
> 
> Do you think this stack variable can't be destroyed, because it's const?

What it boils down to is this: const restricts access, and so prevents ownership. If you can't destroy a thing, you don't really own it. The standard appears to have taken the position that ownership beats const correctness. I instead argue in favor of const correctness, and its guarantee of invariance.

The stack variable in the example illustrates the difference between access and ownership. f() has read-only access to i, and therefore does not own it. Who owns i? The stack does. The stack passes i to f() with limited access, and then, when f() terminates, the stack destroys i. This way ownership is clearly delineated. If f() were to say delete i (assuming i were a pointer), it should be prevented from doing so.

>   using const_int = const int;
>   const int* p = new const_int();
> 
> Do you expect to never be able to delete this object,
> instead being forced to leak it?

Consequently, const objects created with new are owned by nobody, and simply do not make sense. Somebody has to own the allocated object, so creating a const object should be an invalid operation.

I suppose it doesn't really matter what my opinion is in this matter. Neither I nor you write the standard, so I'll just leave this as a closing footnote in a bug correctly resolved INVALID.
Comment 9 Jonathan Wakely 2018-05-22 08:15:33 UTC
That's not how C++ works.
Comment 10 Jonathan Wakely 2018-05-22 08:24:27 UTC
(In reply to Mike Sharov from comment #8)
> My mental model here is actually of const correctness, not C++ specifically.
> When I pass around a const object I expect it to stay unmodified. Consider a
> function that takes a const T* argument. The signature suggests that the
> passed-in object will only be read and will not be modified.

No, no, no. It says **that function** can't modify it **through that pointer**. If the pointee was not actually declared const then it can still be changed.

> If that
> function deletes the pointer, "bad things" will likely happen. Suddenly the
> object contains garbage and you can't figure out how it happened.

Your assumption that const T* means the pointer remains valid is just completely unjustifiable:

   int* p = new int();
   const int* pc = p;
   delete p;
   // pc is valid here, nobody can delete it due to "const correctness" (wrong)

It's simply not how C++ works. An object's lifetime is distinct from it's constness, and a pointer-to-const doesn't imply anything about whether the pointed-to object is immutable.
Comment 11 Jonathan Wakely 2018-05-22 08:28:42 UTC
(In reply to Jonathan Wakely from comment #10)
> (In reply to Mike Sharov from comment #8)
> > My mental model here is actually of const correctness, not C++ specifically.
> > When I pass around a const object I expect it to stay unmodified. Consider a
> > function that takes a const T* argument. The signature suggests that the
> > passed-in object will only be read and will not be modified.
> 
> No, no, no. It says **that function** can't modify it **through that
> pointer**. If the pointee was not actually declared const then it can still
> be changed.

I misread your comment slightly, yes, it suggests the passed-in object will only be read and will not be modified **by that function**. But it can still be modified by other access paths, and if it's a pointer toan object on the heap it can certainly be deleted and therefore invalidated. You seem to be saying that a pointer-to-const implies an immortal object that will never be destroyed. Why should that be true for pointers to the heap but not pointers to the stack? It's not true, and it wouldn't make sense for it to be true.
Comment 12 Mike Sharov 2018-05-22 13:25:37 UTC
(In reply to Jonathan Wakely from comment #10)
> It's simply not how C++ works.

Quite right. I already agreed with you here; we are arguing about whether it /should/ work this way :)

> An object's lifetime is distinct from it's constness, and a pointer-to-const 
> doesn't imply anything about whether the pointed-to object is immutable.

Exactly! I can restate my gripe in these terms: C++ has no way of explicitly marking the owner of the object or its lifetime. When f() creates object const A a and passes it as const A* to g(), both f() and g() see the same const A object, but f() is the owner and should be allowed to delete it, while g() has only been granted read-only access and should not. If delete required a non-const pointer, then f() would either keep a non-const pointer to indicate that it owns a, or have to explicitly const_cast it to delete.

> You seem to be saying that a pointer-to-const implies
> an immortal object that will never be destroyed.

Not at all. Object lifetime is a separate subject, but const correctness should help enforce it by restricting who gets to set it. Ideally, the object will have exactly one owner (insert rant on the evils of shared_ptr), and that owner will determine the lifetime of the object. If const prevented delete, the compiler could help you catch violations of the one-owner rule that may compromise defined object lifetime and cause undefined behavior in functions that hold pointers to that object.

A function can only assume that the pointer it was given remains valid if the object lifetime is explicitly known, and there is no explicit C++ way of making it known. We can only define the lifetime in documentation. For example:

> Why should that be true for pointers to the heap
> but not pointers to the stack?

Because the stack frees all owned objects when the scope is exited and the heap does not. The stack will call destructors to cleanup the objects, the heap will not. Consequently the stack can be said to be the owner of local objects, but the heap owns nothing because it destroys nothing.