Bitshift operations and bitmask not detecting duplicate characters

Question

I've been trying to write a program that checks for duplicate letters in a lowercase input using a bitmask. However, the program returns true regardless of the presence of a duplicate letter. The code is as follows:

#include <iostream>
#include <cctype>
#include <cstring>

using namespace std;

bool all_unique_letters(const string &s) {
    int bitset;

    for(char const &c : s)
    {
        unsigned char set = 1 << (c - 'a');

        if(set & bitset)
            return false;

        bitset |= set;
    }

    return true;
}

int main(int argc, char * const argv[]) {
    // TODO: reads and parses command line arguments.
    // Calls other functions to produce correct output.
    if(all_unique_letters(argv[1]))
        cout << "All letters are unique!" << endl;
    else
        cout << "Duplicate letters found." << endl;
}

Well, what did `gdb` show you, if you claim that you used it? Apart from the undefined behavior due to usage of an uninitialized variable, the `for` loop has nothing in it that will cause a segfault. What parameter did you pass to your program? Pop quiz: what is the value of `argc`? — Sam Varshavchik, Feb 15 '20 at 15:47
@SamVarshavchik Yep, thought I was properly passing an argument to gdb, but I was not. It always returns true, though, which I don't understand. It seems like the bitwise operations should be working. — TheFiveHundredYears, Feb 15 '20 at 15:54
As was mentioned by @SamVarshavchik, try to print `argc` and `argv[1]` that you pass to `all_unique_letters` function. — Tarek Dakhran, Feb 15 '20 at 15:55
Please reread the first part of the 2nd sentence in my first comment, which explains why your bitwise operations appear not to work. — Sam Varshavchik, Feb 15 '20 at 15:59
@SamVarshavchik I initialized bitset to zero, but the program still only returns true. — TheFiveHundredYears, Feb 15 '20 at 16:02
@walnut Post edited. Input: "unique". And I do have a method that checks for lowercase letters, but I omitted it from the post for simplicity. — TheFiveHundredYears, Feb 15 '20 at 16:04
Ok, so one more problem remains. At this point, since you know how to use a debugger, why don't you use it to figure out why it returns true. It should be very obvious. This is something that anyone who knows how to use a debugger should be able to figure out. This is what the debugger is for: to let you find and fix bugs all by yourself, instead of waiting for someone on stackoverflow.com to figure them out for you. — Sam Varshavchik, Feb 15 '20 at 16:08
@SamVarshavchik Sam, I walked through each line of the code. Every time it reaches the if statement, it fails to execute the body. Now, there must be something that I don't understand about bitwise operations in C++, if each time the two sets are "and"ed together, they return 0. — TheFiveHundredYears, Feb 15 '20 at 16:12
@TheFiveHundredYears The debugger can show you, at any time, the values of all your variables. In gdb try e.g. `p bitset` in each iteration and see whether `bitset` has the value that you expect it to have. As mentioned in the very first comment, you will see that it doesn't have the value you expect, probably already in the first iteration, since you use `bitset` uninitialized. — walnut, Feb 15 '20 at 16:21
Did you examine the values of ***all*** variables? The debugger is not only there to run one line at a time. The debugger also allows you to look at all the variables. Use your debugger to see what `set` and `bitset` changes to, every time they are initialized or used, and see if it makes sense to you. Once you see a value you do not expect, the problem should be very obvious. — Sam Varshavchik, Feb 15 '20 at 16:22

score 0 · Accepted Answer · answered Feb 15 '20 at 16:49

I see two problems. First your bitset variable is uninitialized. Second, the type for your set variable is unsigned char an 8-bit type. For a lower-case alphabet you need at least 26 bits to test against your bitset. After fixing both issues your code seems to work.

bool all_unique_letters(const string &s) {
    int bitset = 0; // fixed

    for(char const &c : s)
    {
        int set = 1 << (c - 'a'); // fixed

        if(set & bitset)
            return false;

        bitset |= set;
    }
    return true;
}

Bitshift operations and bitmask not detecting duplicate characters

1 Answers1