open-watcom / open-watcom-v2

Open Watcom V2.0 - Source code repository, Wiki, Latest Binary build, Archived builds including all installers for download.
Other
986 stars 161 forks source link

OWSTL vector issue #70

Open revelator opened 10 years ago

revelator commented 10 years ago

Tried my luck again with later wxwidgets and i noticed something that seems to be a bug in OWSTL's vector header. cannot construct object error is thrown in the scintilla code.

Not sure if its because of missing functionality ? but i thought the maintainer might like to know.

pchapin commented 10 years ago

Can you give more specifics about the error? I have a feeling it may be due to missing functionality but that's just a guess right now.

revelator commented 10 years ago

Ill try.

this is the error

H:\Compilers\Euphoria\WATCOM\H\vector(68): Error! E340: col(59) cannot construct object from argument(s) H:\Compilers\Euphoria\WATCOM\H\vector(68): Note! N393: col(59) included from ....\src\stc\scintilla\lexers\LexCPP.cxx(17) H:\Compilers\Euphoria\WATCOM\H\vector(68): Note! N638: col(59) 'PPDefinition' defined in: ....\src\stc\scintilla\lexers\LexCPP.cxx(126) (col 8) H:\Compilers\Euphoria\WATCOM\H\vector(86): Error! E340: col(49) cannot construct object from argument(s) H:\Compilers\Euphoria\WATCOM\H\vector(86): Note! N638: col(49) 'PPDefinition' defined in: ....\src\stc\scintilla\lexers\LexCPP.cxx(126) (col 8) H:\Compilers\Euphoria\WATCOM\H\vector(68): Error! E340: col(59) cannot construct object from argument(s) H:\Compilers\Euphoria\WATCOM\H\vector(68): Note! N638: col(59) 'State' defined in: ....\src\stc\scintilla\lexlib\SparseState.h(19) (col 16) H:\Compilers\Euphoria\WATCOM\H\vector(86): Error! E340: col(49) cannot construct object from argument(s) H:\Compilers\Euphoria\WATCOM\H\vector(86): Note! N638: col(49) 'State' defined in: ....\src\stc\scintilla\lexlib\SparseState.h(19) (col 16) ....\src\stc\scintilla\lexers\LexCPP.cxx(1171): Error! E419: col(81) cannot call non-const function for a constant object ....\src\stc\scintilla\lexers\LexCPP.cxx(1171): Note! N638: col(81) 'map' defined in: H:\Compilers\Euphoria\WATCOM\H\map(58) (col 7) ....\src\stc\scintilla\lexers\LexCPP.cxx(1171): Note! N392: col(81) definition: 'std::_ow::RedBlackTreestd::basic_string<char,std::char_traits<char,std::allocator>,std::lessstd::basic_string<char,std::char_traits<char,std::allocator>>,std::allocatorstd::pair<std::basic_string<char,std::char_traits<char,std::allocator> const,std::basic_string<char,std::char_traits,std::allocator>>>,std::_ow::TreePairWrapperstd::basic_string<char,std::char_traits<char,std::allocator<cha H:\Compilers\Euphoria\WATCOM\H_algnmod.h(316): Error! E531: col(25) left operand must be a pointer, pointer to class member, or arithmetic H:\Compilers\Euphoria\WATCOM\H_algnmod.h(316): Note! N634: col(25) template function instantiation for 'equal' was in: ....\src\stc\scintilla\lexlib\SparseState.h(88) (col 17) H:\Compilers\Euphoria\WATCOM\H_algnmod.h(313): Note! N392: col(10) definition: 'bool std::equal( SparseState<std::basic_string<char,std::char_traits,std::allocator>>::State , SparseStatestd::basic_string<char,std::char_traits<char,std::allocator>>::State , SparseStatestd::basic_string<char,std::char_traits<char,std::allocator>>::State const * )' H:\Compilers\Euphoria\WATCOM\H_algnmod.h(313): Note! N638: col(10) 'State' defined in: ....\src\stc\scintilla\lexlib\SparseState.h(19) (col 16) H:\Compilers\Euphoria\WATCOM\H_algnmod.h(313): Note! N717: col(10) left operand type is 'SparseStatestd::basic_string<char,std::char_traits<char,std::allocator>>::State const' H:\Compilers\Euphoria\WATCOM\H_algnmod.h(313): Note! N718: col(10) right operand type is 'SparseStatestd::basic_string<char,std::char_traits<char,std::allocator>>::State const (lvalue)' ....\src\stc\scintilla\lexlib\SparseState.h(37): Error! E926: col(29) syntax error: 'std::lower_bound' has not been declared as a member ....\src\stc\scintilla\lexlib\SparseState.h(37): Note! N635: col(29) template class member instantiation was in: ....\src\stc\scintilla\lexers\LexCPP.cxx(327) (col 34) ....\src\stc\scintilla\lexlib\SparseState.h(38): Error! E028: col(9) expecting ';' but found '}' ....\src\stc\scintilla\lexlib\SparseState.h(38): Note! N635: col(9) template class member instantiation was in: ....\src\stc\scintilla\lexers\LexCPP.cxx(327) (col 34) Error(E42): Last command making (wat_mswd\wxscintilla_LexCPP.obj) returned a bad status Error(E02): Make execution terminated

Not sure but looks like this part might be the culprit -> Error! E926: col(29) syntax error: 'std::lower_bound' has not been declared as a member

pchapin commented 10 years ago

It is true that std::lower_bound is not yet implemented. However, it looks like the first problem is something else. Right now std::vector requires the elements to be default constructable. A fully conformant implementation can deal with element types that are only copyable.

I believe correcting Open Watcom to handle this will first require lazy instantiation to be implemented. In particular right now there are these two constructors:

explicit vector( const Allocator & = Allocator( ) );
explicit vector( size_type, const Type & = Type( ), const Allocator & = Allocator( ) );

The standard requires instead

explicit vector( const Allocator & = Allocator( ) );
explicit vector( size_type n );
vector( size_type n, const Type &, const Allocator & = Allocator( ) );

In the case when an item is not default constructable the last constructor is used and the middle one is not instantiated (due to lazy instantiation) because it is never used. Open Watcom only defines two constructors but the default argument of Type() requires the element type to be default constructable. Open Watcom can't take advantage of the standard form because it will always instantiate everything, still requiring elements to be default constructable regardless.

revelator commented 10 years ago

Thanks for the explanantion pchapin, only recently started working with C++ but i hope to pick up some insight over time.

So If i got you right implementing lazy instantiation will not fix it ? or is there another way to make this work besides porting the code from wxwidgets to use something that Open Watcom can handle ?.

pchapin commented 10 years ago

I believe implementing lazy instantiation would be part of the fix. The OWSTL library would need to be updated to take advantage of the ability as well. If it is true that the problem arises because wxWidgets is instantiating std::vector with a type without a default constructor (as seems to be the case), I don't see a good workaround.

It is possible to change std::vector to use types without a default constructor but that would require you to also provide an object to initialize vector elements when you create a vector. You'd get this effect:

std::vector<int> my_vector(100);      // Vector of 100 ints. Does not compile!
std::vector<int> my_vector(100, 0);   // Must provide default element value (zero).

This would break more code than it fixes.

revelator commented 10 years ago

I think it uses templates, probably something like template typename where T can be any type. So as void char int unsigned etc. This works with msvc mingw and a number of other compilers so it should be possible to fix, atleast i hope so.

I followed the error in wxwidgets and it leads to this code.

struct PPDefinition { int line; std::string key; std::string value; PPDefinition(int line, const std::string &key, const std::string &value) : line(line), key(key), value(value) { } };

and later the error cannot call non-const function for a const object (which might actually be a bug in wxwidget code).

bool LexerCPP::EvaluateExpression(const std::string &expr, const std::map<std::string, std::string> &preprocessorDefinitions) { // Break into tokens, replacing with definitions std::string word; std::vectorstd::string tokens; const char _cp = expr.c_str(); for (;;) { if (setWord.Contains(_cp)) { word += _cp; } else { std::map<std::string, std::string>::const_iterator it = preprocessorDefinitions.find(word); if (it != preprocessorDefinitions.end()) { tokens.push_back(it->second); } else if (!word.empty() && ((word[0] >= '0' && word[0] <= '9') || (word == "defined"))) { tokens.push_back(word); } word = ""; if (!_cp) { break; } if ((_cp != ' ') && (_cp != '\t')) { std::string op(cp, 1); if (setRelOp.Contains(_cp)) { if (setRelOp.Contains(cp[1])) { op += cp[1]; cp++; } } else if (setLogicalOp.Contains(_cp)) { if (setLogicalOp.Contains(cp[1])) { op += cp[1]; cp++; } } tokens.push_back(op); } } cp++; }

EvaluateTokens(tokens);

// "0" or "" -> false else true
bool isFalse = tokens.empty() ||
    ((tokens.size() == 1) && ((tokens[0] == "") || tokens[0] == "0"));
return !isFalse;

}

where it seems to choke on this std::map<std::string, std::string>::const_iterator it = preprocessorDefinitions.find(word);

pchapin commented 10 years ago

Structure PPDefinition has a constructor with parameters but no default constructor. Thus it is not default constructable. Open Watcom's std::vector template can't be instantiated with such a type right now and that's the problem you are having. It's a failing of Open Watcom; the other compilers you mentioned implement the standard more accurately in this area.

Fixing this will, I believe, require first updating the compiler to support lazy instantiation and then (the easy part) adjusting OWSTL's std::vector template to take advantage of it.

revelator commented 10 years ago

Ok thats good to hear :) Im still not very experienced with C++ besides minor bugfixing so i look forward to the standard library becoming more complete. I might be able to help on missing C parts though. Ill see what comes up.

ak-ambi commented 7 years ago

I just experienced the same issue when compiling CMake project. Any chance that this will be fixed?

jmalak commented 7 years ago

I have it on my todo list but now with low priority because I am working on portability and 64-bit host support. As soon as these tasks will be finished then I start to work on C++ compiler. It will requre big redesign of compiler, because it uses hard-coded 2 level naming support (it should be multilevel) and will require add lazy class instantiating feature.