Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix FlatTuple compilation on older msvc. #2569

Merged
merged 1 commit into from
Nov 20, 2019
Merged

Fix FlatTuple compilation on older msvc. #2569

merged 1 commit into from
Nov 20, 2019

Conversation

bgianfo
Copy link
Contributor

@bgianfo bgianfo commented Nov 16, 2019

googletest 1.10.0 fails to compile on msvc version 19.00.23917
with one compilation error:

src\googletest\include\gtest\internal\gtest-internal.h(1188) : error C2039:
'FlatTupleBase<testing::internal::FlatTuple<bool,bool>,testing::internal::IndexSequence<0,1> >':
is not a member of 'testing::internal::FlatTuple<bool,bool>'

This PR fixes the compilation error by explicitly specifying the full type that Indices is
located in the base type.

googletest 1.10.0 fails to compile on msvc version 19.00.23917
with one compilation error:

src\googletest\include\gtest\internal\gtest-internal.h(1188) : error C2039:
'FlatTupleBase<testing::internal::FlatTuple<bool,bool>,testing::internal::IndexSequence<0,1> >':
is not a member of 'testing::internal::FlatTuple<bool,bool>'

This PR fixes the compilation error by explicitly specifying the full type that Indices is
located in the base type.
@@ -1185,7 +1185,7 @@ struct FlatTupleBase<FlatTuple<T...>, IndexSequence<Idx...>>

// Analog to std::tuple but with different tradeoffs.
// This class minimizes the template instantiation depth, thus allowing more
// elements that std::tuple would. std::tuple has been seen to require an
// elements than std::tuple would. std::tuple has been seen to require an
// instantiation depth of more than 10x the number of elements in some
// implementations.
// FlatTuple and ElemFromList are not recursive and have a fixed depth

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"it is" a few lines below should be "is".
"ln" after that should probably be "lg". i.e. log base 2 instead of log base e. Just guessing.

@gennadiycivil
Copy link
Contributor

gennadiycivil commented Nov 18, 2019

@bgianfo Thank you for the patch. Could you please explain exactly what version of Visual Studio, etc this is?
Thanks
(281066649)

@gennadiycivil gennadiycivil self-assigned this Nov 18, 2019
@gennadiycivil gennadiycivil reopened this Nov 18, 2019
@bgianfo
Copy link
Contributor Author

bgianfo commented Nov 18, 2019

@gennadiycivil The MSVC compiler I was using is from Visual Studio 2015, version 14.0. https://en.wikipedia.org/wiki/Microsoft_Visual_C%2B%2B

I didn't do an exhaustive test to see at which MSVC version master starts building clean.

@gennadiycivil
Copy link
Contributor

Thank you, we have started internal review. Please don't push any more changes into this PR as they might be overwritten.

gennadiycivil added a commit that referenced this pull request Nov 19, 2019
PiperOrigin-RevId: 281321427
gennadiycivil added a commit that referenced this pull request Nov 20, 2019
PiperOrigin-RevId: 281321427
@gennadiycivil gennadiycivil merged commit 0c469a5 into google:master Nov 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants