|
ce8bb2fa
|
2014-12-30T13:32:25
|
|
Improve standards conformance of ANGLE's testing code.
ANGLE's testing code recently got enabled in Chromium's builds. While it
builds fine with cl.exe, it isn't quite standards-conformant and doesn't
build with clang. Fix this.
There were three issues:
1. ANGLE_TYPED_TEST_CASE() is a variadic macro that tries to use
__VA_ARGS__ as argument to a variadic template and then pass
that template to another macro. However, [cpp.replace] describes
that ANGLE_TYPED_TEST_CASE(Test, int, float) should be expanded
to TYPED_TEST_CASE(Test, ::testing::Types<int, float>) which
should be interpreted as a "call" of TYPED_TEST_CASE with the
3 macro arguments `Test`, `::testing::Types<int`, and `float>`.
As a fix, use a typedef for the variadic template and refer to
it through the typedef in the macro call.
2. `#version` was used on its own line in a substitution of the
SHADER_SOURCE macro. [cpp]p1 says that every line starting with
a `#` is a preprocessing directive, and [cpp.replace]p11 says
"If there are sequences of preprocessing tokens within the list
of arguments that would otherwise act as preprocessing directives,
the behavior is undefined" (with a footnote that this includes
non-directives -- # followed by unknown text). As a fix, merge the
`#version` line with the previous line. Now the line doesn't start
with `#` and things are fine.
3. Unqualified lookup usually doesn't look into dependent bases. If
this is desired, one usually has to make the call qualified, a
good explanation for this is at
http://eli.thegreenplace.net/2012/02/06/dependent-name-lookup-for-c-templates
cl.exe doesn't implement this rule, and clang tries to emulate
cl.exe's behavior to a certain extend when in Microsoft mode.
However, that seems to not work for member templates with explicit
types (filed http://llvm.org/PR22066 for this, but since it's not
needed to parse Microsoft headers and not standards-conformant,
I'm not sure if we'll fix that). As a fix, don't provide an explicit
type, the inferred type is the same. This is also consistent with
all the other tests in this file. (We might clean up -Wmicrosoft
warnings in the future; if so I'll add the explicit this->s that
are missing in this file when we do.)
BUG=chromium:445406
Change-Id: I77a2f3ab9601a1f0f39b56ed3d05217f123155b8
Reviewed-on: https://chromium-review.googlesource.com/238090
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Brandon Jones <bajones@chromium.org>
Tested-by: Geoff Lang <geofflang@chromium.org>
|
