graphitemaster / glsl-parser

A GLSL parser
MIT License
260 stars 30 forks source link

Fix ambigous find function #1

Closed karimnaaji closed 8 years ago

karimnaaji commented 8 years ago

std::find (which is somehow coming from #include <vector> on Apple clang 7) is conflicting with glsl:find.

graphitemaster commented 8 years ago

This doesn't make sense. You must be doing something really silly somewhere.

karimnaaji commented 8 years ago

I cloned and ran make, nothing more, nothing less.

christoffer commented 8 years ago

For what it's worth, I'm also seeing this (on OSX) and making the changes here does indeed fix it for me.

julik commented 8 years ago

I don't really see how "compiling on a different OS than Linux" qualifies as "silly", especially when a library states portability as a feature ;-)

graphitemaster commented 8 years ago

This is not the correct solution, util.h contains the implementation of find()

julik commented 8 years ago

What is the correct solution?

graphitemaster commented 8 years ago

The correct solution is to find out why std::find is coming into the global namespace, or more appropriately the glsl namespace. One work around would be to prefix the find calls with glsl:: as in just use glsl::find fully qualified so the compiler cannot get this wrong. The fact that std::find is creeping into the glsl namespace is a compiler/vendor problem however.

julik commented 8 years ago

The reason is that on clang vector pulls in algorithm, which in turn contains std::find. You can also remove your implementation of find and add an explicit include for algorithm in utils. Maybe I am missing something, but I don't see a difference in the binary size once built, so it seems to be seeping in anyway.

vagrant@vagrant-ubuntu-trusty-64:~/glsl-parser$ g++ --version
g++ (Ubuntu 4.8.4-2ubuntu1~14.04.1) 4.8.4
Copyright (C) 2013 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

with algorithm include and removed glsl:find:

-rwxrwxr-x 1 vagrant vagrant 140546 Apr 24 12:09 glsl-parser

without:

-rwxrwxr-x 1 vagrant vagrant 140546 Apr 24 12:10 glsl-parser
graphitemaster commented 8 years ago

Point is not library size, it's less dependencies on the standard library - I fixed it in master