Remove template class GLUIAPI std::vector<GLUI_String, std::allocator<GLUI_String> >;

[m-7761]

' #ifdef
_MSC_VER
// Explicit template instantiation needed for dll
template class GLUIAPI std::allocator<GLUI_String>;
template class GLUIAPI std::vector<GLUI_String, std::allocator<GLUI_String> >;
#endif`

Is in the glui.h header. I've never seen anything like this; but I had to do something like this for GCC today. (It's really fussy (it is) or really stupid.)

What I do know is GLUI doesn't own std::vector<std::string> which as near as I can tell is what it's implying here. It could be code leftover from a time when GLUI_String was a distinct class. Anyway; even if this works GLUI can't just export these standard types.

[nigels-com]

Do we have a test that is passing std::vectorstd::string to/from DLL? I see no harm in GLUI exposing these in the ABI, for Windows purposes. I'd want to check for side effects.

[m-7761]

Well, I've never heard of converting a header-only object into a module object by exporting it (it must convert every single method to a blind call into the module; so doing size() in a loop calls into the module for every iteration!)

But I'm just pointing out that if this is done, you have no right to convert a std:: structure into a module owned structure. That can of course have side effects. But do something like struct GLUI_String:std::string{}; so it's at least kosher.

[nigels-com]

I don't tend to use Windows much. I'd want to understand the possible consequences of this proposed change.

[m-7761]

Consequences are old GLUI dynamic-library binaries would not link to the std::vector<GLUI_String> methods. So the DLL will not load. It's not good practice because std::vector is designed to be pure-inline, and this forces it to dllexport every method. Every single method. In general "GLUIAPI" should not be in front of structures. That's just lazy and limiting design. It should be in front of specific non-inline methods inside the structures. I'm probably repeating myself.

EDITED: Of course what makes it criminal is typdef std::string GLUI_String. Think about it.

[nigels-com]

Marking this as "wontfix" for now.
It's a reasonably recent change, not clear on the details or the implications.

$ git show 85b8038d
commit 85b8038d9dbe415b327480d6bffd008a0e79c382
Author: Johannes 'josch' Schauer <[email protected]>
Date:   Tue Sep 22 17:41:44 2015 +0200

    fix MSVC problem with template class instantiation
    
     - error C2252 informs that the microsoft compiler wants template
       classes to be instantiated at namespace scope

diff --git a/include/GL/glui.h b/include/GL/glui.h
index 07758b1..5c15080 100644
--- a/include/GL/glui.h
+++ b/include/GL/glui.h
@@ -1694,6 +1694,12 @@ protected:
 /*                                                          */
 /************************************************************/
 
+#ifdef _MSC_VER
+// Explicit template instantiation needed for dll
+template class GLUIAPI std::allocator<GLUI_String>;
+template class GLUIAPI std::vector<GLUI_String, std::allocator<GLUI_String> >;
+#endif
+
 class GLUIAPI GLUI_CommandLine : public GLUI_EditText
 {
 public:
@@ -1701,12 +1707,6 @@ public:
 
     enum { HIST_SIZE = 100 };
 
-    #ifdef _MSC_VER
-    // Explicit template instantiation needed for dll
-    template class GLUIAPI std::allocator<GLUI_String>;
-    template class GLUIAPI std::vector<GLUI_String, std::allocator<GLUI_String> >;
-    #endif
-
     std::vector<GLUI_String> hist_list;
     int  curr_hist;
     int  oldest_hist;

[m-7761]

Working with GLUI the last few nights in occurs to me that #42 should make this practice pointless. Is it there to compile?

Anyway, using new to create controls will break on delete inside the DLL if the DLL is not linked to the same CRT (C++ Runtime) DLL. If that's the case std::allocator<GLUI_String> must also agree on its memory manager.

In truth, to make GLUI work otherwise, it must not use std::string, or std:vector, nor derivatives of them. And it needs an inline virtual method that is able to delete the controls/elements allocated with new.

[m-7761]

I've fixed this problem with this (#91 (comment)) change. The code in the CPP file looks like this:

	newest_hist = ++curr_hist;
	if(newest_hist>=_add_to_history_HIST_SIZE)
	{
		// bump oldest off the list
		//hist_list.erase(hist_list.begin());
		//hist_list.push_back("");
		hist_list[oldest_hist_wrap].clear();
		++oldest_hist_wrap%=_add_to_history_HIST_SIZE;

		oldest_hist++;
	}

That removes the dependency on std::vector. I think it's wise to avoid all but String given how easy it is to do so. Honestly vector was completely a waste here. Without "move-semantics" it would have very bad memory allocation behavior. It's still not great with it. A circular buffer makes the most sense. It's size was always fixed.

I'm not planning on developing a strategy to make std::string cross-module safe with the code I'm working on now. I think that can wait until a round 2.

My thoughts on it are, I think having a virtual interface wrapper on the strings would be useful also for making it work with more than one kind of string. Empty strings could be represented by stubs...

Something I find dismaying is trying to implement a text-editor inside of the control. That seems like a bad strategy. At the same time I can't find any real-simple libraries for doing text-editing without a UI attached to them. Maybe BOOST has something. There has to be something though. If there was a name for one, that would make it easy to find alternatives.

If I can't find a library, I will probably make time to develop one. Editing text is just tricky enough that it shouldn't be reinvented in a control, but not so involved that it can't be implemented in a single header inline library.

If the String had an interface, it could also supply its own text-editor via the interface. A control doesn't need a world-beating editor. But it does need a minimum of expected functionality.

[nigels-com]

So if not std::vector<std::string> then perhaps std::list<std::string>?

Does that address the concern about memory reallocation? It supports insertion and deletion from both ends in constant time, after all.

[m-7761]

No, #91 (comment) is already using C-array. Why use standard container for an array? (The problem here is exporting the standard container definition.)

[nigels-com]

Standard containers are business as usual in modern C++ programming. It's not at all clear that there is something special enough in this case to bother with something custom.

[m-7761]

Exporting standard containers is never done. I don't know why I have to explain this, but that's about the worst/rudest thing a library can possibly do. I don't want to anger you, but you seem to not understand (or be pretending not to) the purpose of this "Issue"? (The problem here is GLUIAPI.)

[nigels-com]

At issue here is a platform-specific workaround, that concerns me little. Most of all, I don't agree with potentially breaking a platform that I don't really want to deal with. It's not that I don't understand your objection, but I'm deciding to be pragmatic.

If there a specific practical problem or issue this change is aimed at resolving, it's not mentioned here.
If this change has been tested and validated for a variety of tool chains or use cases, it's not mentioned here.

Having some evidence that this workaround is only needed for certain obsolete versions of the MS toolchain would be more compelling.

[m-7761]

No offense, but it's absurd comments like this one (nigels-com) that make me to want to just middle-finger GLUI (and opensource, antisocial, idiocy in general) and just go do my own thing. So don't be surprised if this is the last time I share my work here in the interest of making GLUI a better user experience, and opportunity, for developers around the world. (One of us is pro progress and people, and the other is clearly not.)

[nigels-com]

I really don't recommend this sort of conduct as a way to advance an argument. It's normal to have differences of opinion. I've made the reasoning as clear as possible, as a courtesy.

[m-7761]

This topic is several posts long; and perverse in the extreme. It's probably why I walked away in 2017. I have a condition called "aphantasia" that I think impairs my memory. I can't really remember past episodes after a year or two. It can be a blessing and a curse.

If you can't see why being a bureaucratic PITA over a program attempting to export a Standard Library container (which is unheard of) or how dismissing Windows as a platform you "don't want to deal with" is inappropriate, then you're just pretending to not know any better. You're a terrible person in other words. I don't believe you don't understand what dllimport and dllexport mean. You're just being an imp, haunting this project. Which is typical of open-source trolls.

[nigels-com]

Do you think that calling people names is going to win the argument? Seriously?

It's a shame we can't seem to find some common ground to go forward.

In fairness I'm asking you again to be more constructive and respectful.

[m-7761]

(I think arguing wastes time and human potential, and you're, clearly, not acting in good faith. I tell you this only so you might consider reforming your ways, because we've spent a lot of time chatting.)