Skip to content

Commit

Permalink
Fix ODR violations due to abuse of LLVM_YAML_IS_(FLOW_)?SEQUENCE_VECTOR
Browse files Browse the repository at this point in the history
This is a short-term fix for PR33650 aimed to get the modules build bots green again.

Remove all the places where we use the LLVM_YAML_IS_(FLOW_)?SEQUENCE_VECTOR
macros to try to locally specialize a global template for a global type. That's
not how C++ works.

Instead, we now centrally define how to format vectors of fundamental types and
of string (std::string and StringRef). We use flow formatting for the former
cases, since that's the obvious right thing to do; in the latter case, it's
less clear what the right choice is, but flow formatting is really bad for some
cases (due to very long strings), so we pick block formatting. (Many of the
cases that were using flow formatting for strings are improved by this change.)

Other than the flow -> block formatting change for some vectors of strings,
this should result in no functionality change.

Differential Revision: https://reviews.llvm.org/D34907

Corresponding updates to clang, clang-tools-extra, and lld to follow.


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@306878 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
zygoloid committed Jun 30, 2017
1 parent e33b6de commit 638ba5a
Show file tree
Hide file tree
Showing 15 changed files with 109 additions and 75 deletions.
3 changes: 0 additions & 3 deletions include/llvm/IR/ModuleSummaryIndexYAML.h
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,6 @@ struct FunctionSummaryYaml {
} // End yaml namespace
} // End llvm namespace

LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(uint64_t)

namespace llvm {
namespace yaml {

Expand Down Expand Up @@ -188,7 +186,6 @@ template <> struct MappingTraits<FunctionSummaryYaml> {

LLVM_YAML_IS_STRING_MAP(TypeIdSummary)
LLVM_YAML_IS_SEQUENCE_VECTOR(FunctionSummaryYaml)
LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(std::string)

namespace llvm {
namespace yaml {
Expand Down
2 changes: 0 additions & 2 deletions include/llvm/ObjectYAML/DWARFYAML.h
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,7 @@ struct Data {
} // namespace llvm::DWARFYAML
} // namespace llvm

LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(uint8_t)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::yaml::Hex64)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::StringRef)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::yaml::Hex8)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::DWARFYAML::AttributeAbbrev)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::DWARFYAML::Abbrev)
Expand Down
1 change: 0 additions & 1 deletion include/llvm/ObjectYAML/MachOYAML.h
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ struct UniversalBinary {

LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::MachOYAML::LoadCommand)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::MachOYAML::Section)
LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(int64_t)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::MachOYAML::RebaseOpcode)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::MachOYAML::BindOpcode)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::MachOYAML::ExportEntry)
Expand Down
1 change: 0 additions & 1 deletion include/llvm/ObjectYAML/WasmYAML.h
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,6 @@ LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::WasmYAML::LocalDecl)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::WasmYAML::Relocation)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::WasmYAML::NameEntry)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::WasmYAML::SymbolInfo)
LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(uint32_t)

namespace llvm {
namespace yaml {
Expand Down
112 changes: 74 additions & 38 deletions include/llvm/Support/YAMLTraits.h
Original file line number Diff line number Diff line change
Expand Up @@ -180,17 +180,17 @@ struct BlockScalarTraits {
/// to/from a YAML sequence. For example:
///
/// template<>
/// struct SequenceTraits< std::vector<MyType>> {
/// static size_t size(IO &io, std::vector<MyType> &seq) {
/// struct SequenceTraits<MyContainer> {
/// static size_t size(IO &io, MyContainer &seq) {
/// return seq.size();
/// }
/// static MyType& element(IO &, std::vector<MyType> &seq, size_t index) {
/// static MyType& element(IO &, MyContainer &seq, size_t index) {
/// if ( index >= seq.size() )
/// seq.resize(index+1);
/// return seq[index];
/// }
/// };
template<typename T>
template<typename T, typename EnableIf = void>
struct SequenceTraits {
// Must provide:
// static size_t size(IO &io, T &seq);
Expand All @@ -201,6 +201,14 @@ struct SequenceTraits {
// static const bool flow = true;
};

/// This class should be specialized by any type for which vectors of that
/// type need to be converted to/from a YAML sequence.
template<typename T, typename EnableIf = void>
struct SequenceElementTraits {
// Must provide:
// static const bool flow;
};

/// This class should be specialized by any type that needs to be converted
/// to/from a list of YAML documents.
template<typename T>
Expand Down Expand Up @@ -1542,18 +1550,59 @@ operator<<(Output &yout, T &seq) {
return yout;
}

template <typename T> struct SequenceTraitsImpl {
using _type = typename T::value_type;
template <bool B> struct IsFlowSequenceBase {};
template <> struct IsFlowSequenceBase<true> { static const bool flow = true; };

template <typename T, bool Flow>
struct SequenceTraitsImpl : IsFlowSequenceBase<Flow> {
private:
using type = typename T::value_type;

public:
static size_t size(IO &io, T &seq) { return seq.size(); }

static _type &element(IO &io, T &seq, size_t index) {
static type &element(IO &io, T &seq, size_t index) {
if (index >= seq.size())
seq.resize(index + 1);
return seq[index];
}
};

// Simple helper to check an expression can be used as a bool-valued template
// argument.
template <bool> struct CheckIsBool { static const bool value = true; };

// If T has SequenceElementTraits, then vector<T> and SmallVector<T, N> have
// SequenceTraits that do the obvious thing.
template <typename T>
struct SequenceTraits<std::vector<T>,
typename std::enable_if<CheckIsBool<
SequenceElementTraits<T>::flow>::value>::type>
: SequenceTraitsImpl<std::vector<T>, SequenceElementTraits<T>::flow> {};
template <typename T, unsigned N>
struct SequenceTraits<SmallVector<T, N>,
typename std::enable_if<CheckIsBool<
SequenceElementTraits<T>::flow>::value>::type>
: SequenceTraitsImpl<SmallVector<T, N>, SequenceElementTraits<T>::flow> {};

// Sequences of fundamental types use flow formatting.
template <typename T>
struct SequenceElementTraits<
T, typename std::enable_if<std::is_fundamental<T>::value>::type> {
static const bool flow = true;
};

// Sequences of strings use block formatting.
template<> struct SequenceElementTraits<std::string> {
static const bool flow = false;
};
template<> struct SequenceElementTraits<StringRef> {
static const bool flow = false;
};
template<> struct SequenceElementTraits<std::pair<std::string, std::string>> {
static const bool flow = false;
};

/// Implementation of CustomMappingTraits for std::map<std::string, T>.
template <typename T> struct StdMapStringCustomMappingTraitsImpl {
using map_type = std::map<std::string, T>;
Expand All @@ -1571,42 +1620,29 @@ template <typename T> struct StdMapStringCustomMappingTraitsImpl {
} // end namespace yaml
} // end namespace llvm

/// Utility for declaring that a std::vector of a particular type
/// should be considered a YAML sequence.
#define LLVM_YAML_IS_SEQUENCE_VECTOR(_type) \
#define LLVM_YAML_IS_SEQUENCE_VECTOR_IMPL(TYPE, FLOW) \
namespace llvm { \
namespace yaml { \
template <> \
struct SequenceTraits<std::vector<_type>> \
: public SequenceTraitsImpl<std::vector<_type>> {}; \
template <unsigned N> \
struct SequenceTraits<SmallVector<_type, N>> \
: public SequenceTraitsImpl<SmallVector<_type, N>> {}; \
static_assert( \
!std::is_fundamental<TYPE>::value && \
!std::is_same<TYPE, std::string>::value && \
!std::is_same<TYPE, llvm::StringRef>::value, \
"only use LLVM_YAML_IS_SEQUENCE_VECTOR for types you control"); \
template <> struct SequenceElementTraits<TYPE> { \
static const bool flow = FLOW; \
}; \
} \
}

/// Utility for declaring that a std::vector of a particular type
/// should be considered a YAML sequence.
#define LLVM_YAML_IS_SEQUENCE_VECTOR(type) \
LLVM_YAML_IS_SEQUENCE_VECTOR_IMPL(type, false)

/// Utility for declaring that a std::vector of a particular type
/// should be considered a YAML flow sequence.
/// We need to do a partial specialization on the vector version, not a full.
/// If this is a full specialization, the compiler is a bit too "smart" and
/// decides to warn on -Wunused-const-variable. This workaround can be
/// removed and we can do a full specialization on std::vector<T> once
/// PR28878 is fixed.
#define LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(_type) \
namespace llvm { \
namespace yaml { \
template <unsigned N> \
struct SequenceTraits<SmallVector<_type, N>> \
: public SequenceTraitsImpl<SmallVector<_type, N>> { \
static const bool flow = true; \
}; \
template <typename Allocator> \
struct SequenceTraits<std::vector<_type, Allocator>> \
: public SequenceTraitsImpl<std::vector<_type, Allocator>> { \
static const bool flow = true; \
}; \
} \
}
#define LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(type) \
LLVM_YAML_IS_SEQUENCE_VECTOR_IMPL(type, true)

#define LLVM_YAML_DECLARE_MAPPING_TRAITS(Type) \
namespace llvm { \
Expand Down Expand Up @@ -1653,10 +1689,10 @@ template <typename T> struct StdMapStringCustomMappingTraitsImpl {
namespace yaml { \
template <unsigned N> \
struct DocumentListTraits<SmallVector<_type, N>> \
: public SequenceTraitsImpl<SmallVector<_type, N>> {}; \
: public SequenceTraitsImpl<SmallVector<_type, N>, false> {}; \
template <> \
struct DocumentListTraits<std::vector<_type>> \
: public SequenceTraitsImpl<std::vector<_type>> {}; \
: public SequenceTraitsImpl<std::vector<_type>, false> {}; \
} \
}

Expand Down
2 changes: 0 additions & 2 deletions lib/ObjectYAML/CodeViewYAMLDebugSections.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,7 @@ LLVM_YAML_IS_SEQUENCE_VECTOR(InlineeSite)
LLVM_YAML_IS_SEQUENCE_VECTOR(InlineeInfo)
LLVM_YAML_IS_SEQUENCE_VECTOR(CrossModuleExport)
LLVM_YAML_IS_SEQUENCE_VECTOR(YAMLCrossModuleImport)
LLVM_YAML_IS_SEQUENCE_VECTOR(StringRef)
LLVM_YAML_IS_SEQUENCE_VECTOR(YAMLFrameData)
LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(uint32_t)

LLVM_YAML_DECLARE_SCALAR_TRAITS(HexFormattedString, false)
LLVM_YAML_DECLARE_ENUM_TRAITS(DebugSubsectionKind)
Expand Down
1 change: 0 additions & 1 deletion lib/ObjectYAML/CodeViewYAMLSymbols.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ using namespace llvm::CodeViewYAML;
using namespace llvm::CodeViewYAML::detail;
using namespace llvm::yaml;

LLVM_YAML_IS_SEQUENCE_VECTOR(StringRef)
LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(TypeIndex)

// We only need to declare these, the definitions are in CodeViewYAMLTypes.cpp
Expand Down
1 change: 0 additions & 1 deletion lib/ObjectYAML/CodeViewYAMLTypes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ using namespace llvm::CodeViewYAML::detail;
using namespace llvm::yaml;

LLVM_YAML_IS_SEQUENCE_VECTOR(OneMethodRecord)
LLVM_YAML_IS_SEQUENCE_VECTOR(StringRef)
LLVM_YAML_IS_SEQUENCE_VECTOR(VFTableSlotKind)
LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(TypeIndex)

Expand Down
2 changes: 0 additions & 2 deletions lib/Support/AMDGPUCodeObjectMetadata.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@
using namespace llvm::AMDGPU;
using namespace llvm::AMDGPU::CodeObject;

LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(uint32_t)
LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(std::string)
LLVM_YAML_IS_SEQUENCE_VECTOR(Kernel::Arg::Metadata)
LLVM_YAML_IS_SEQUENCE_VECTOR(Kernel::Metadata)

Expand Down
10 changes: 6 additions & 4 deletions test/CodeGen/AMDGPU/code-object-metadata-from-llvm-ir-full.ll
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@

; CHECK: ---
; CHECK: Version: [ 1, 0 ]
; CHECK: Printf: [ '1:1:4:%d\n', '2:1:8:%g\n' ]
; CHECK: Printf:
; CHECK: - '1:1:4:%d\n'
; CHECK: - '2:1:8:%g\n'
; CHECK: Kernels:

; CHECK: - Name: test_char
Expand Down Expand Up @@ -1253,8 +1255,8 @@ define amdgpu_kernel void @test_pointee_align(i64 addrspace(1)* %a,
; NOTES-NEXT: Owner Data size Description
; NOTES-NEXT: AMD 0x00000008 Unknown note type: (0x00000001)
; NOTES-NEXT: AMD 0x0000001b Unknown note type: (0x00000003)
; GFX700: AMD 0x00008b06 Unknown note type: (0x0000000a)
; GFX800: AMD 0x00008e6a Unknown note type: (0x0000000a)
; GFX900: AMD 0x00008b06 Unknown note type: (0x0000000a)
; GFX700: AMD 0x00008b0a Unknown note type: (0x0000000a)
; GFX800: AMD 0x00008e6e Unknown note type: (0x0000000a)
; GFX900: AMD 0x00008b0a Unknown note type: (0x0000000a)

; PARSER: AMDGPU Code Object Metadata Parser Test: PASS
4 changes: 3 additions & 1 deletion test/MC/AMDGPU/code-object-metadata-kernel-args.s
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@

// CHECK: .amdgpu_code_object_metadata
// CHECK: Version: [ 1, 0 ]
// CHECK: Printf: [ '1:1:4:%d\n', '2:1:8:%g\n' ]
// CHECK: Printf:
// CHECK: - '1:1:4:%d\n'
// CHECK: - '2:1:8:%g\n'
// CHECK: Kernels:
// CHECK: - Name: test_kernel
// CHECK: Language: OpenCL C
Expand Down
4 changes: 3 additions & 1 deletion test/MC/AMDGPU/code-object-metadata-kernel-attrs.s
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@

// CHECK: .amdgpu_code_object_metadata
// CHECK: Version: [ 1, 0 ]
// CHECK: Printf: [ '1:1:4:%d\n', '2:1:8:%g\n' ]
// CHECK: Printf:
// CHECK: - '1:1:4:%d\n'
// CHECK: - '2:1:8:%g\n'
// CHECK: Kernels:
// CHECK: - Name: test_kernel
// CHECK: Language: OpenCL C
Expand Down
9 changes: 7 additions & 2 deletions test/Transforms/LowerTypeTests/export-icall.ll
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ declare !type !8 void @f(i32 %x)
; SUMMARY-NEXT: SizeM1BitWidth: 0
; SUMMARY-NEXT: WPDRes:

; SUMMARY: CfiFunctionDefs: [ f, g, h ]
; SUMMARY-NEXT: CfiFunctionDecls: [ external, external_weak ]
; SUMMARY: CfiFunctionDefs:
; SUMMARY-NEXT: - f
; SUMMARY-NEXT: - g
; SUMMARY-NEXT: - h
; SUMMARY-NEXT: CfiFunctionDecls:
; SUMMARY-NEXT: - external
; SUMMARY-NEXT: - external_weak
; SUMMARY-NEXT: ...
2 changes: 0 additions & 2 deletions tools/llvm-pdbutil/PdbYaml.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ using namespace llvm::pdb;
using namespace llvm::pdb::yaml;
using namespace llvm::yaml;

LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(uint32_t)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::StringRef)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::pdb::yaml::NamedStreamMapping)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::pdb::yaml::PdbDbiModuleInfo)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::pdb::yaml::StreamBlockList)
Expand Down
Loading

0 comments on commit 638ba5a

Please sign in to comment.