521
#include <iostream>
#include <set>

using namespace std;

class StudentT {

public:
    int id;
    string name;
public:
    StudentT(int _id, string _name) : id(_id), name(_name) {
    }
    int getId() {
        return id;
    }
    string getName() {
        return name;
    }
};

inline bool operator< (StudentT s1, StudentT s2) {
    return  s1.getId() < s2.getId();
}

int main() {

    set<StudentT> st;
    StudentT s1(0, "Tom");
    StudentT s2(1, "Tim");
    st.insert(s1);
    st.insert(s2);
    set<StudentT> :: iterator itr;
    for (itr = st.begin(); itr != st.end(); itr++) {
        cout << itr->getId() << " " << itr->getName() << endl;
    }
    return 0;
}

In line:

cout << itr->getId() << " " << itr->getName() << endl;

It give an error that:

../main.cpp:35: error: passing 'const StudentT' as 'this' argument of 'int StudentT::getId()' discards qualifiers

../main.cpp:35: error: passing 'const StudentT' as 'this' argument of 'std::string StudentT::getName()' discards qualifiers

What's wrong with this code? Thank you!

Community
  • 1
  • 1
JASON
  • 7,371
  • 9
  • 27
  • 40
  • 15
    Where is line 35 in your code snippet? – In silico May 12 '11 at 04:54
  • 145
    I wish GCC would improve this error message, e.g. "discards qualifiers" -> "breaks const correctness" – jfritz42 Oct 08 '13 at 22:06
  • 15
    @jfritz42: Would be confusing for the case it discards `volatile` – PlasmaHH Dec 09 '13 at 15:40
  • 5
    @PlasmaHH the error message would be split into "breaks const correctness" and "breaks volatile correctness". Now, not many people will think about something being *volatile correct* – Caleth Aug 31 '17 at 13:50

4 Answers4

596

The objects in the std::set are stored as const StudentT. So when you try to call getId() with the const object the compiler detects a problem, mainly you're calling a non-const member function on const object which is not allowed because non-const member functions make NO PROMISE not to modify the object; so the compiler is going to make a safe assumption that getId() might attempt to modify the object but at the same time, it also notices that the object is const; so any attempt to modify the const object should be an error. Hence compiler generates an error message.

The solution is simple: make the functions const as:

int getId() const {
    return id;
}
string getName() const {
    return name;
}

This is necessary because now you can call getId() and getName() on const objects as:

void f(const StudentT & s)
{
     cout << s.getId();   //now okay, but error with your versions
     cout << s.getName(); //now okay, but error with your versions
}

As a sidenote, you should implement operator< as :

inline bool operator< (const StudentT & s1, const StudentT & s2)
{
    return  s1.getId() < s2.getId();
}

Note parameters are now const reference.

Tomer
  • 531
  • 7
  • 19
Nawaz
  • 353,942
  • 115
  • 666
  • 851
  • 3
    Such a clear explanation. Thanks. But I wonder about your last code snippet. Why use reference in the function parameter ? `const StudentT & s1, const StudentT & s2` ? – Rafael Adel Jun 12 '16 at 09:44
  • 3
    @RafaelAdel: You use reference to avoid unecessary copy, and `const` because the function doesn't need to modify the object, so the `const` enforces this at compile-time. – Nawaz Jun 12 '16 at 10:41
100

Member functions that do not modify the class instance should be declared as const:

int getId() const {
    return id;
}
string getName() const {
    return name;
}

Anytime you see "discards qualifiers", it's talking about const or volatile.

Fred Larson
  • 60,987
  • 18
  • 112
  • 174
  • 2
    @Fred - Do you think it's definitely a requirement to add const modifiers to member functions that do not modify class instance ? Is there some other reason for the error in this case ? I doubt it because in most of the getters I write, I don't add const modifiers to it. – Mahesh May 12 '11 at 05:01
  • @Mahesh: Yes, it's part of [const correctness](http://www.parashift.com/c++-faq-lite/const-correctness.html). I'm not certain where the `const` is coming from here, but I suspect the `set` is returning a const reference from the iterator to prevent the instance from changing and thereby invalidating the set. – Fred Larson May 12 '11 at 05:05
  • @Mahesh: Wouldn't pass my code review. I have a coworker who refers to me as the "const-able". 8v) Change that `foo obj;` to `const foo obj;` once and see what happens. Or pass a `const` reference to a `foo`. – Fred Larson May 12 '11 at 05:06
  • @Fred - That is entirely different. I understand `const` objects can only call const member functions and const member functions can only call another const qualified member function. But in this case, I don't understand why std::set is storing it as `const StudentT` as @Nawaz mentioned. – Mahesh May 12 '11 at 05:10
  • 3
    @Mahesh: It's like I said -- if the elements in a `set` are changed, the order can be upset and then the set isn't valid anymore. In a `map`, only the key is `const`. In a `set`, the whole object is really the key. – Fred Larson May 12 '11 at 05:12
  • 1
    @Mahesh: const are necessary otherwise you cannot call them with const objects. see the function `f()` in my answer. – Nawaz May 12 '11 at 05:12
  • @Nawaz - I understand const references. I was just wondering why std::set is storing it as a const object. And it is clear from @Fred's comment. Actually, I have never used std::set. – Mahesh May 12 '11 at 05:17
9

Actually the C++ standard (i.e. C++ 0x draft) says (tnx to @Xeo & @Ben Voigt for pointing that out to me):

23.2.4 Associative containers
5 For set and multiset the value type is the same as the key type. For map and multimap it is equal to pair. Keys in an associative container are immutable.
6 iterator of an associative container is of the bidirectional iterator category. For associative containers where the value type is the same as the key type, both iterator and const_iterator are constant iterators. It is unspecified whether or not iterator and const_iterator are the same type.

So VC++ 2008 Dinkumware implementation is faulty.


Old answer:

You got that error because in certain implementations of the std lib the set::iterator is the same as set::const_iterator.

For example libstdc++ (shipped with g++) has it (see here for the entire source code):

typedef typename _Rep_type::const_iterator            iterator;
typedef typename _Rep_type::const_iterator            const_iterator;

And in SGI's docs it states:

iterator       Container  Iterator used to iterate through a set.
const_iterator Container  Const iterator used to iterate through a set. (Iterator and const_iterator are the same type.)

On the other hand VC++ 2008 Express compiles your code without complaining that you're calling non const methods on set::iterators.

Eugen Constantin Dinca
  • 8,994
  • 2
  • 34
  • 51
5

Let's me give a more detail example. As to the below struct:

struct Count{
    uint32_t c;

    Count(uint32_t i=0):c(i){}

    uint32_t getCount(){
        return c;
    }

    uint32_t add(const Count& count){
        uint32_t total = c + count.getCount();
        return total;
    }
};

enter image description here

As you see the above, the IDE(CLion), will give tips Non-const function 'getCount' is called on the const object. In the method add count is declared as const object, but the method getCount is not const method, so count.getCount() may change the members in count.

Compile error as below(core message in my compiler):

error: passing 'const xy_stl::Count' as 'this' argument discards qualifiers [-fpermissive]

To solve the above problem, you can:

  1. change the method uint32_t getCount(){...} to uint32_t getCount() const {...}. So count.getCount() won't change the members in count.

or

  1. change uint32_t add(const Count& count){...} to uint32_t add(Count& count){...}. So count don't care about changing members in it.

As to your problem, objects in the std::set are stored as const StudentT, but the method getId and getName are not const, so you give the above error.

You can also see this question Meaning of 'const' last in a function declaration of a class? for more detail.

Zhro
  • 2,546
  • 2
  • 29
  • 39
Jayhello
  • 5,931
  • 3
  • 49
  • 56