3

I'm trying to make a simple map to look up some data, but the results are coming out very strange:

#include "stdafx.h"
#include "atlstr.h"
#include <map>

enum InputTypes { Manual, Automatic, Assisted, Imported, Offline };

struct Params
{
    int inputType;
    const char* moduleName;
    DWORD flag;
};

int _tmain()
{
    std::map<CString, Params> options {
        { "Add",       { Manual,    "RecordLib",  0 } },
        { "Open",      { Assisted,  "ViewLib",    1 } },
        { "Close",     { Imported,  "EditLib",    2 } },
        { "Inventory", { Automatic, "ControlLib", 3 } },
        { "Report",    { Offline,   "ReportLib",  4 } }
    };

    for (std::map<CString, Params>::iterator iter = options.begin(); iter != options.end(); ++iter)
    {
        printf("Entry: %s ==> { %d, %s, %d }\n", (const char*)(iter->first),
            iter->second.inputType, iter->second.moduleName, iter->second.flag);
    }


    return 0;
}

Output:

Entry: îþîþîþîþîþîþîþîþîþîþîþîþ[â0; t ==> { 0, RecordLib, 0 }
Entry: Close ==> { 3, EditLib, 2 }
Entry: Inventory ==> { 1, ControlLib, 3 }
Entry: îþîþîþîþîþîþîþîþîþîþîþîþCâ0# t ==> { 2, ViewLib, 1 }
Entry: Report ==> { 4, ReportLib, 4 }

As you can see, a couple of the CString values turned to garbage. But I don't see any reason why I couldn't create a map this way.

Is this a bug in the Microsoft Visual Studio 2013 compiler?
Is there something peculiar about my code that I'm missing?

############ Edit ##############

For those who think this is a problem with CString, I re-wrote it with std::string, and got worse output:

#include "stdafx.h"
#include "atlstr.h"
#include <map>

enum InputTypes { Manual, Automatic, Assisted, Imported, Offline };

struct Params
{
    int inputType;
    std::string moduleName;
    DWORD flag;
};

int _tmain(int argc, _TCHAR* argv[])
{
    std::map<std::string, Params> options{
        { "Add",       { Manual, "RecordLib", 0 } },
        { "Open",      { Assisted, "ViewLib", 1 } },
        { "Close",     { Imported, "EditLib", 2 } },
        { "Inventory", { Automatic, "ControlLib", 3 } },
        { "Report",    { Offline, "ReportLib", 4 } }
    };

    for (std::map<std::string, Params>::iterator iter = options.begin(); iter != options.end(); ++iter)
    {
        printf("Entry: %s ==> { %d, %s, %d }\n", iter->first.c_str(),
            iter->second.inputType, iter->second.moduleName.c_str(), iter->second.flag);
    }
    return 0;
}

Output

Entry:  ==> { 0, , 0 }
Entry: Report ==> { 4, , 4 }

Note that this does work on IDEOne

pnuts
  • 58,317
  • 11
  • 87
  • 139
abelenky
  • 63,815
  • 23
  • 109
  • 159
  • You have to expicitly cast `CString` objects to `LPCTSTR`. Better to get away from `CString` all together though. – Chad Nov 05 '15 at 16:21
  • 1
    @Chad, Together with all ellipsis past-century nonsense :D – SergeyA Nov 05 '15 at 16:22
  • I've updated my code to have an explicit cast. However, the same problem persists. – abelenky Nov 05 '15 at 16:24
  • You are still using iter->second.moduleName. Who knows what it does to parameters there? By the way, does the issue persist when you use std::string in place of this MSFT special? I believe, there are more ppl familiar with std than MS, so it would be worth the effort to eliminate as much of MS specific as possible. – SergeyA Nov 05 '15 at 16:27
  • `moduleName` is `LPCTSTR`, which is just a typedef for `char*`. There should be nothing "supicious" about that?? – abelenky Nov 05 '15 at 16:28
  • On MSDN all assignments to `CString` from string literal is done through `_T()` macro, could that be the issue? – Slava Nov 05 '15 at 16:28
  • What is LPCTSTR? It might be char or wide char depending on your setting as far as Google tells me. – SergeyA Nov 05 '15 at 16:29
  • Removed `LPCTSTR` in favor of `const char*` to make it really clear. – abelenky Nov 05 '15 at 16:31
  • It is possible that the error comes from the use of wide character sets. If you want to verify that, you can disable wide character sets in the project settings: `Configuration Properties -> General -> Character Set : Not Set` – A.S.H Nov 05 '15 at 16:33
  • Have tried it with MultiByte Character set and "Not Set". (Unicode does not apply for this particular project). The problem still exists both ways. – abelenky Nov 05 '15 at 16:34
  • I just rephrased the same example with pure std::. (On gcc). As expected, works OK. I suggest you do what I already suggested - get rid of CString in favor of std::string and see if the problem persists. – SergeyA Nov 05 '15 at 16:37
  • Thank you, however, that is a work-around. The Question is, "Should this work? Is this a compiler bug?". Working around the problem is not an answer. – abelenky Nov 05 '15 at 16:39
  • It is very unlikely to be a compiler bug. I can't answer whether it should work or not because I am not familiar with CString. it is either a bug in CString implementation, a confusion related to wide-char / single-char conversion or a known issue with CStrings. – SergeyA Nov 05 '15 at 16:43
  • By the way, how do you expect the std::map the calculate the hash of a CString? I has experienced with MFC's CMap and even that one had difficulties when the key is a CString! – A.S.H Nov 05 '15 at 16:48
  • A.S.H: `map` does not use hashes. `unordered_map` uses hashes. `map` uses equality and strict ordering. – abelenky Nov 05 '15 at 16:49
  • ok :). However, I think that the problem is with CString itself, (something probably not standard in its copy constructor for example), rather than a compiler bug. – A.S.H Nov 05 '15 at 16:53
  • I have copied you example into MSVC. It is even more than incorret printing - it is a memory violation in ATL CString upon destruction of map. But all works with std::string. Conclusion - buggy ATL implementation. If I am to take a wild guess, I'd say, it's a bug in move constructor. – SergeyA Nov 05 '15 at 17:07
  • I posted a variation of the code using std::string, which still does not work. I do not know why you'd be getting success. – abelenky Nov 05 '15 at 17:08
  • Try to disable move ctor for `Params` see if that changes anything. – Slava Nov 05 '15 at 17:09
  • Do I have to post screenshot so that you believe me? I had the same issues as you with CString, no more with std::string. – SergeyA Nov 05 '15 at 17:10
  • 1
    Scrap that, only tried in 2015. – Mike Vine Nov 05 '15 at 17:10
  • My guess is that the std::string version does work for you. CString is defined to be a CStringW when built with unicode, so your printf using %s is incorrect which is why you see the issue with CString – Mike Vine Nov 05 '15 at 17:15
  • @MikeVine, unlikely. See my comments - it produces a memory access viloation when used. I did a cursory glance on CString implementation, and I am pretty sure, it's reference counting doesn't play along with rvalue references. – SergeyA Nov 05 '15 at 17:28
  • 1
    Your edited code works for me in vs2013 (barring removal of `stdafx` and `atlstr`, and replacing `_tmain(...)` with `main()`). Both 32 and 64 bit... – Rostislav Nov 05 '15 at 17:37
  • Actually your non-edit version of code works in its original form. – Rostislav Nov 05 '15 at 17:39

1 Answers1

3

I have copied you example into MSVC. It is even more than incorret printing - it is a memory violation in ATL CString upon destruction of map. But all works with std::string. Conclusion - buggy ATL implementation. If I am to take a wild guess, I'd say, it's a bug in move constructor.

SergeyA
  • 61,605
  • 5
  • 78
  • 137