Infinite Loop when calling a mock function that takes boost::filesystem::path as parameter #521
Comments
|
From @GoogleCodeExporter on August 24, 2015 22:40 Original comment by |
|
From @GoogleCodeExporter on August 24, 2015 22:40 Original comment by |
|
From @GoogleCodeExporter on August 24, 2015 22:40 Original comment by |
|
From @GoogleCodeExporter on August 24, 2015 22:40 Original comment by |
|
From @GoogleCodeExporter on August 24, 2015 22:40 Original comment by |
|
It seems that the root of the problem is the incorrect assumption that I once implemented a trait like this even with C++03 restrictions. Given the declarations within What do you think of such an approach? |
|
Sorry I don't get it. On Sunday, December 20, 2015, Daniel Krügler notifications@github.com
I'd have to see that.
|
|
In the context of this issue I would define a "RecursiveContainer" as a type that is a "Container" (as defined by the simple definition provided by the For example, both I don't understand your assertion regarding operators and The most critical decision would be (IMO) whether the increase of complexity to determine a "RecursiveContainerType", depending on something like the existing |
|
On Sun, Dec 20, 2015 at 10:43 AM, Daniel Krügler notifications@github.com
OH! I didn't know we had such beasts. One quick question: is it possible for such a detection to not terminate?
|
|
Considering that most compilers have now implemented Using gcc or clang, I can reproduce this with a simple file. Save the following code snippet to #include <experimental/filesystem>
#include <gtest/gtest.h>
namespace fs = std::experimental::filesystem;
int main()
{
fs::path p1("/lib/libc.so.6");
fs::path p2("/lib/libnsl.so");
EXPECT_TRUE(p1 == p2); // fine
EXPECT_EQ(p1, p2); // infinite loop
return 0;
}Then, compile and run: $ g++ a.cpp -lstdc++fs -gtest
$ ./a.out
a.cpp:15: Failure
Value of: p1 == p2
Actual: false
Expected: true
Segmentation fault (core dumped)I'm not sure what the best solution would be (maybe SFINAE detection if the |
|
Yes, sorry, my intention was to work on that issue myself, but I lost the track of it. Thanks for the reminder! |
…em::path as parameter google#521: Add is_same type trait
…em::path as parameter google#521: Add is_same type trait and prevent infinite loops for recursive containers
|
I prepared a pull request (Prevent infinite loops for recursive containers like boost::filesystem::path #1186) which has passed all travis tests and I signed the CLA. The solution is simpler than I originally thought: The suggested fix simply prevents "recursive containers" entering the normal container output specialization. The effect is that For example, would output (including quotation marks): Since I'm not aware of other know "recursively-defined" containers than |
* Correct some typos in a comment * fix small typo in comment * add note about different definitions of Test Case There are contradictory definitions of the term "test case", so prepare new users in Primer.md to avoid confusion. * Samples changes upstreaming * Samples changes upstreaming * WIP * WIP, windows testing * WIP, windows testing * WIP, windows testing * WIP, win testing * WIP, win testing * Punctuation Missing periods * be more specific on Test Case * WIP * Added googlemock tests * Added googlemock tests * Infinite Loop when calling a mock function that takes boost::filesystem::path as parameter google#521: Add is_same type trait * Infinite Loop when calling a mock function that takes boost::filesystem::path as parameter google#521: Add is_same type trait and prevent infinite loops for recursive containers * WIP * Fix library install destinations Modify library install destinations to install .dll's to the correct location (`bin`, not `lib`), and to install other artifacts to the correct platform-dependent location by using GNUInstallDirs. This is required for some distributions (e.g. Fedora) and will fix an issue that otherwise requires those distributions to patch the upstream sources. Also, add options to suppress installation, which may be useful for projects that embed Google Test. Since Google Test is trying to support archaic versions of CMake, a brain-dead fallback (which requires that the user set either LIB_SUFFIX or CMAKE_INSTALL_LIBDIR themselves) is included for versions that predate GNUInstallDirs. Fixes google#1161. Co-Authored-By: d3x0r <d3x0r@users.noreply.github.com> * clarify distinction regarding Test Case * Adding a flag option to change the default mock type * Initial Revision, review 164634031 * Added Copyright * Minor style fixes Typos, punctuation & broken links * say "former version" instead of "released version" * Update WORKSPACE Remove comments * Addressing comments * Addressing Comments * Add support for pkgconfig * Add documentation for pkg-config * Use GTEST_LOG instead of printf * Removed extra colon in error log * Change AppVeyor Status Badge to point to new AppVeyor Project Location * Change AppVeyor Status Badge to point to new AppVeyor Project Location * Fix problem installing gtest when gmock enabled Fix a bug deciding whether to enable the option to install Google Test caused by one of the dependent option dependencies not being defined yet. Fixes google#1198; impossible to install Google Test if Google Mock is built. * Add function name to exception if there's no default action * Handling invalid flag values * Update README.md Another AppVeyor move * adds test for NiceMock with unknown return value * Fix test if exceptions are not supported * Switch return type to class without default constructor * Change tabs to spaces in test case * Adding CMake visibility policy setting This policy setting will silence a warning when using with a visibility settings on targets. Due to the forced `cmake_minimum_version`, policy settings in CMakeLists calling this one (including the main CMakeLists) are lost, forcing the change to be made here. * Proposing these changes, please review Slightly better names and cleaner tests. Please review * Added "explicit" as per compiler suggestion * Remove unused variable * Support ref-qualified member functions in Property(). * Support x64 configuration for old VS2010 projects VS2010 solution only to simplify old users (who used these solutions) upgrading to new gtest/gmock, new users should use CMake generated solutions. VS2010 solution can be opened in any new VS. * Remove gtest VS2005 projects * Support x64 configuration for old VS2015 projects * Applying lint checks from upstream google3 * fix typo in comment and string (SetUpTestCase) * remove unused TestCase import * fix typo: xUnit * remove non-existing gmock_build_samples switch * switch on verbose make * run combined build only There is no need for separate 'googlemock' and 'googletest' builds, as the 'googlemock' build includes 'googletest' and it's unit tests. * remove Yob's comma mentioned in issue google#1105 * use plural verb as mentioned in issue google#1105 * Detect Fuchsia, and set GTEST_HAS_PTHREAD on GTEST_OS_FUCHSIA * use build type set in .travis.yml The BUILD_TYPE variable of .travis.yml was ignored up to now. * use upper-case build type While cmake does not complain on build type 'debug', the cmake documentation always spells it 'Debug', so take this. * fix SetUp/TearDownTestCase() in AdvancedGuide fixes issue google#1087 * remove obsolete comment regarding python tests on linux * ask cmake for per-configuration output subdir On single-configuration build systems as Makefile Generators, there is no subdirectory for the configuration in the build tree - therefore ask cmake for the subdir by using CMAKE_CFG_INTDIR, which is just '.' on single-configuration build systems (Linux et al.). * Revert "ask cmake for per-configuration output subdir" This reverts commit 73d58dd. Unfortunately, ${CMAKE_CFG_INTDIR} is set during build only and doesn't help here. * create different python based tests for single and multi configuration build generators * Note that it is preferable for Googlers to create a CL internally first * removed internal link ( not allowed in OSS) * Removed "Trivial" Who knows? may not be very trivial given the code drift between internal and OSS * AppVeyor MinGW-w64 test build * fix example's comment * change links from former code.google.com to current github repository * CMake: use threads if allowed and found, not just if found. If the user's cmakelists.txt first look for threads using find_package(Threads), then set(gtest_disable_pthreads ON), and then include googletest. GoogleTest will not look for threads. But since they have already been found before in user's cmakelists, it will use them regardless. This helped me fix build issue in darktable-org/rawspeed on windows/MSYS2, even though there are threads, and they are usable, googletest build was failing with issues about AutoHandle. I was first looking for threads, and only then including googletest, so no matter the value of gtest_disable_pthreads, it failed. The other obvious solution is for user to first include googletest, and only then look for threads by himself. * switch one build to Release mode This turns on optimization which allows the compiler to discover more problems and omit some more warnings. * treat all warnings as errors for GCC (-Werror) * Allow macros inside of parametrized test names. This allows doing things like TEST_P(TestFixture, MAYBE(TestName)) for nicer conditional test disabling. * Add a non-parametrized test. * avoid -Wshadow warning on GCC When using INSTANTIATE_TEST_CASE_P with a lambda function which uses 'info' as parameter name, GCC complains that this would shadow parameter 'info' used in the macro's VA_ARGS call. * avoid warning about unused variable * cache ccache * set MAKEFLAGS to use multiple processors on Travis CI * install ccache on travis osx build slave * limit processors to use in Travis build to 4 * remove obsolete link_directories command It's not necessary, as the target_link_libraries command contains an absolute path already, and the path given doesn't exist anymore, leading only to linker warnings like: ld: warning: directory not found for option '-L/Users/travis/build/google/googletest/build/googlemock/gtest/src' * add a cast * show ccache statistics in log * call clang via ccache on Linux * reset ccache statistics at install * drop unused valgrind package from installation ... and remove explicit gcc installation (will be installed with g++ automatically) * remove unused variables from travis environment * Removed flush scopes around GTEST_LOG(FATAL) and exit call since FATAL is expected to abort() * use GTEST_ATTRIBUTE_UNUSED_ instead of dummy function * Fix ellipsis position in examples * Make the failure messages from EXPECT_EQ and friends actually symmetric, instead of reading more like reversing the former "expected" and "actual" roles of the LHS and RHS arguments. This patch is manually applied from internal version (125109873) * Add explicit `CMAKE_DEBUG_POSTFIX` option Enable generating different library name to be compatible with CMake's `FindGTest`. * Remove redundant declaration TempDir() function is declared twice, once in `internal/gtest-port.h` and a second time in `gtest.h`. Fixes a warning with GCC when -Wredundant-decls is given. * Swap reinterpret_cast for static_cast Swap reinterpret_cast for static_cast * Revert "Allow macros inside of parametrized test names." * Use gender-neutral pronouns in comments and docs * Updated README with information about C runtime dynamic/static linking issues in Windows * google#1282: Doc typo fix * Workaround for Travis issue travis-ci/travis-ci#8552 * Revert "Workaround for Travis issue https://github.com/travis-ci/travis-ci/is…" * Workaround for Travis issue https://goo.gl/d5eV8o * googletest: Add GTEST_API_ attribute to ThreadLocal class. ThreadLocal class needs to be have default visibility. Root cause is gtest uses typeinfo for the ThreadLocal class. The problem manifests When gtest/gmock are built as a shared library with libc++. When a class is used in typeinfo, it must have default visibility. There is an explanation about typeinfo and visibility here: https://developer.apple.com/library/content/documentation/DeveloperTools/Conceptual/CppRuntimeEnv/Articles/SymbolVisibility.html When libc++ is used with gtest in shared library mode, any tests that are compiled with -fvisibility=hidden and exercise the macro EXPECT_CALL, it results in an abort like: [ FATAL ] /usr/include/gtest/internal/gtest-port.h:1394:: Condition typeid(*base) == typeid(Derived) failed. This is because the typeinfo for ThreadLocal class is not visible. Therefore, linker failed to match it to the shared library symbol, creating a new symbol instead. This fixes google#1207. * Enable C++11 features for VS2015 and VS2017 * Fix tests with VS2015 and VS2017 * Fix gmock tests when std::unary_function unavailable * Remove gcc 6 misleading indentations. * Enable CI for VS2017 * remove markdown stars (bold) from code examples * Fixes issue google#826 by treating MinGW as "non-Windows" when determining colored output
|
#1186 should have addressed this |
We needed this function to workaround a googletest bug (google/googletest#521). This bug was fixed a couple of years ago, but we neglected to remove the workaround until now.
We needed this function to workaround a googletest bug (google/googletest#521). This bug was fixed a couple of years ago, but we neglected to remove the workaround until now.
From @GoogleCodeExporter on August 24, 2015 22:40
Original issue reported on code.google.com by
heinzis...@gmail.comon 18 Nov 2014 at 6:27Copied from original issue: google/googlemock#170
The text was updated successfully, but these errors were encountered: