Add ClassID::Unique template and TINT_INSTANTIATE_CLASS_ID

We're seeing some chrome bots fail unittests in ways that suspiciously
look like dynamic casts are doing Wrong Things.

The ClassID::Of() logic depends on the linker folding away duplicate
compilation unit definitions based on ODR rules. If we were to somehow
end up with different definitions, then we'd have two or more different
ClassIDs for the same T type - leading to issues similar to what we're
seeing.

I'm not entirely sure why/how this could happen - and we've so far been
entirely unable to locally reproduce - but it _might_ have something to
do with the goma cache.

In an attempt to work around this, move the static symbol definition out
of a header-local-static and into the .cc file for each of the types.

Change-Id: If914d3045b9dac6fbe8824dac71153a768cfceb9
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/34563
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
This commit is contained in:
Ben Clayton 2020-12-02 18:19:28 +00:00 committed by Commit Bot service account
parent 41083cbca4
commit 89ea705766
79 changed files with 186 additions and 19 deletions

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::AccessDecoration);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::ArrayAccessorExpression);
namespace tint {
namespace ast {

View File

@ -18,6 +18,8 @@
#include "src/ast/stride_decoration.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::ArrayDecoration);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::AssignmentStatement);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::BinaryExpression);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::BindingDecoration);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::BitcastExpression);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::BlockStatement);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::BoolLiteral);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::BreakStatement);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::BuiltinDecoration);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::CallExpression);
namespace tint {
namespace ast {

View File

@ -18,6 +18,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::CallStatement);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::CaseStatement);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::ConstantIdDecoration);
namespace tint {
namespace ast {

View File

@ -16,6 +16,8 @@
#include <assert.h>
TINT_INSTANTIATE_CLASS_ID(tint::ast::ConstructorExpression);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::ContinueStatement);
namespace tint {
namespace ast {

View File

@ -22,6 +22,8 @@
#include "src/ast/location_decoration.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::DecoratedVariable);
namespace tint {
namespace ast {

View File

@ -14,6 +14,8 @@
#include "src/ast/decoration.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::Decoration);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::DiscardStatement);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::ElseStatement);
namespace tint {
namespace ast {

View File

@ -14,6 +14,8 @@
#include "src/ast/expression.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::Expression);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::FallthroughStatement);
namespace tint {
namespace ast {

View File

@ -20,6 +20,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::FloatLiteral);
namespace tint {
namespace ast {

View File

@ -25,6 +25,8 @@
#include "src/ast/type/texture_type.h"
#include "src/ast/workgroup_decoration.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::Function);
namespace tint {
namespace ast {

View File

@ -14,6 +14,8 @@
#include "src/ast/function_decoration.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::FunctionDecoration);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::IdentifierExpression);
namespace tint {
namespace ast {

View File

@ -18,6 +18,8 @@
#include "src/ast/else_statement.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::IfStatement);
namespace tint {
namespace ast {

View File

@ -14,6 +14,8 @@
#include "src/ast/int_literal.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::IntLiteral);
namespace tint {
namespace ast {

View File

@ -14,6 +14,8 @@
#include "src/ast/literal.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::Literal);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::LocationDecoration);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::LoopStatement);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::MemberAccessorExpression);
namespace tint {
namespace ast {

View File

@ -16,6 +16,8 @@
#include <sstream>
TINT_INSTANTIATE_CLASS_ID(tint::ast::Node);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::NullLiteral);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::ReturnStatement);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::ScalarConstructorExpression);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::SetDecoration);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::SintLiteral);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::StageDecoration);
namespace tint {
namespace ast {

View File

@ -31,6 +31,8 @@
#include "src/ast/switch_statement.h"
#include "src/ast/variable_decl_statement.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::Statement);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::StrideDecoration);
namespace tint {
namespace ast {

View File

@ -18,6 +18,8 @@
#include "src/ast/module.h"
#include "src/ast/struct_block_decoration.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::Struct);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::StructBlockDecoration);
namespace tint {
namespace ast {

View File

@ -14,6 +14,8 @@
#include "src/ast/struct_decoration.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::StructDecoration);
namespace tint {
namespace ast {

View File

@ -18,6 +18,8 @@
#include "src/ast/module.h"
#include "src/ast/struct_member_offset_decoration.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::StructMember);
namespace tint {
namespace ast {

View File

@ -18,6 +18,8 @@
#include "src/ast/struct_member_offset_decoration.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::StructMemberDecoration);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::StructMemberOffsetDecoration);
namespace tint {
namespace ast {

View File

@ -18,6 +18,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::SwitchStatement);
namespace tint {
namespace ast {

View File

@ -19,6 +19,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::type::AccessControl);
namespace tint {
namespace ast {
namespace type {

View File

@ -19,6 +19,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::type::Alias);
namespace tint {
namespace ast {
namespace type {

View File

@ -22,6 +22,8 @@
#include "src/ast/stride_decoration.h"
#include "src/ast/type/vector_type.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::type::Array);
namespace tint {
namespace ast {
namespace type {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::type::Bool);
namespace tint {
namespace ast {
namespace type {

View File

@ -20,6 +20,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::type::DepthTexture);
namespace tint {
namespace ast {
namespace type {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::type::F32);
namespace tint {
namespace ast {
namespace type {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::type::I32);
namespace tint {
namespace ast {
namespace type {

View File

@ -21,6 +21,8 @@
#include "src/ast/type/array_type.h"
#include "src/ast/type/vector_type.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::type::Matrix);
namespace tint {
namespace ast {
namespace type {

View File

@ -20,6 +20,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::type::MultisampledTexture);
namespace tint {
namespace ast {
namespace type {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::type::Pointer);
namespace tint {
namespace ast {
namespace type {

View File

@ -20,6 +20,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::type::SampledTexture);
namespace tint {
namespace ast {
namespace type {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::type::Sampler);
namespace tint {
namespace ast {
namespace type {

View File

@ -20,6 +20,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::type::StorageTexture);
namespace tint {
namespace ast {
namespace type {

View File

@ -24,6 +24,8 @@
#include "src/ast/type/matrix_type.h"
#include "src/ast/type/vector_type.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::type::Struct);
namespace tint {
namespace ast {
namespace type {

View File

@ -19,6 +19,8 @@
#include "src/ast/type/multisampled_texture_type.h"
#include "src/ast/type/sampled_texture_type.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::type::Texture);
namespace tint {
namespace ast {
namespace type {

View File

@ -31,6 +31,8 @@
#include "src/ast/type/vector_type.h"
#include "src/ast/type/void_type.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::type::Type);
namespace tint {
namespace ast {
namespace type {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::type::U32);
namespace tint {
namespace ast {
namespace type {

View File

@ -20,6 +20,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::type::Vector);
namespace tint {
namespace ast {
namespace type {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::type::Void);
namespace tint {
namespace ast {
namespace type {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::TypeConstructorExpression);
namespace tint {
namespace ast {

View File

@ -18,6 +18,8 @@
#include "src/ast/access_decoration.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::TypeDecoration);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::UintLiteral);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::UnaryOpExpression);
namespace tint {
namespace ast {

View File

@ -20,6 +20,8 @@
#include "src/ast/decorated_variable.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::Variable);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::VariableDeclStatement);
namespace tint {
namespace ast {

View File

@ -16,6 +16,8 @@
#include <assert.h>
TINT_INSTANTIATE_CLASS_ID(tint::ast::VariableDecoration);
namespace tint {
namespace ast {

View File

@ -17,6 +17,8 @@
#include "src/ast/clone_context.h"
#include "src/ast/module.h"
TINT_INSTANTIATE_CLASS_ID(tint::ast::WorkgroupDecoration);
namespace tint {
namespace ast {

View File

@ -14,6 +14,8 @@
#include "src/castable.h"
TINT_INSTANTIATE_CLASS_ID(tint::CastableBase);
namespace tint {
bool CastableBase::Is(ClassID id) const {

View File

@ -21,20 +21,28 @@
namespace tint {
/// Helper macro to instantiate the ClassID for `CLASS`.
#define TINT_INSTANTIATE_CLASS_ID(CLASS) \
template <> \
const char tint::ClassID::Unique<CLASS>::token = 0
/// ClassID represents a unique, comparable identifier for a C++ type.
class ClassID {
private:
/// Helper template that holds a single static field, which is used by Of()
/// to obtain a unique identifier by taking the field's address.
template <typename T>
struct Unique {
static const char token;
};
public:
/// @returns the unique ClassID for the type T.
template <typename T>
static ClassID Of() {
// Take the address of a static local variable to produce a unique
// identifier for the type T.
// We use a short name here as this method is instantiated for every
// castable class, and so we'll have a fully qualified name in the
// executable symbol table for every castable type. A shorter name produces
// a smaller executable when unstripped.
static char s;
return ClassID{reinterpret_cast<uintptr_t>(&s)};
// Take the address of a static variable to produce a unique identifier for
// the type T.
return ClassID{reinterpret_cast<uintptr_t>(&Unique<T>::token)};
}
/// Equality operator

View File

@ -19,39 +19,44 @@
#include "gtest/gtest.h"
namespace tint {
namespace {
struct Animal : public Castable<Animal> {
struct Animal : public tint::Castable<Animal> {
explicit Animal(std::string n) : name(n) {}
const std::string name;
};
struct Amphibian : public Castable<Amphibian, Animal> {
struct Amphibian : public tint::Castable<Amphibian, Animal> {
explicit Amphibian(std::string n) : Base(n) {}
};
struct Mammal : public Castable<Mammal, Animal> {
struct Mammal : public tint::Castable<Mammal, Animal> {
explicit Mammal(std::string n) : Base(n) {}
};
struct Reptile : public Castable<Reptile, Animal> {
struct Reptile : public tint::Castable<Reptile, Animal> {
explicit Reptile(std::string n) : Base(n) {}
};
struct Frog : public Castable<Frog, Amphibian> {
struct Frog : public tint::Castable<Frog, Amphibian> {
Frog() : Base("Frog") {}
};
struct Bear : public Castable<Bear, Mammal> {
struct Bear : public tint::Castable<Bear, Mammal> {
Bear() : Base("Bear") {}
};
struct Gecko : public Castable<Gecko, Reptile> {
struct Gecko : public tint::Castable<Gecko, Reptile> {
Gecko() : Base("Gecko") {}
};
} // namespace
TINT_INSTANTIATE_CLASS_ID(Animal);
TINT_INSTANTIATE_CLASS_ID(Amphibian);
TINT_INSTANTIATE_CLASS_ID(Mammal);
TINT_INSTANTIATE_CLASS_ID(Reptile);
TINT_INSTANTIATE_CLASS_ID(Frog);
TINT_INSTANTIATE_CLASS_ID(Bear);
TINT_INSTANTIATE_CLASS_ID(Gecko);
namespace tint {
TEST(CastableBase, Is) {
std::unique_ptr<CastableBase> frog = std::make_unique<Frog>();