Skip to content

Commit b300f36

Browse files
zygoloidzygoloid
authored
Use inline constexpr where appropriate. (#6374)
This fixes various violations of C++'s One Definition Rule, where we accidentally gave the same static data member multiple definitions in different translation units. Clang happens to emit such definitions with weak linkage, which allows us to get away with this without link errors, but it's still formally incorrect. Also switch keyword order around for a handful of instances of `constexpr inline`, per agreement in open discussion. This happens to reduce the size of a `-c dbg` toolchain binary by 7.2 MiB, presumably by making more of our symbols and especially debug info discardable.
1 parent 2b8fdf3 commit b300f36

18 files changed

+96
-83
lines changed

‎common/enum_base.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ class EnumBase : public Printable<DerivedT> {
210210
// Use this immediately after the Carbon enum class body to define each named
211211
// constant.
212212
#define CARBON_ENUM_CONSTANT_DEFINITION(EnumClassName, Name) \
213-
constexpr EnumClassName EnumClassName::Name = \
213+
inline constexpr EnumClassName EnumClassName::Name = \
214214
EnumClassName::Make(RawEnumType::Name);
215215

216216
// Use this in the `.cpp` file for an enum class to start the definition of the

‎common/raw_hashtable.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ namespace Carbon::RawHashtable {
147147

148148
// If allocating storage, allocate a minimum of one cacheline of group metadata
149149
// or a minimum of one group, whichever is larger.
150-
constexpr ssize_t MinAllocatedSize = std::max<ssize_t>(64, MaxGroupSize);
150+
inline constexpr ssize_t MinAllocatedSize = std::max<ssize_t>(64, MaxGroupSize);
151151

152152
// An entry in the hashtable storage of a `KeyT` and `ValueT` object.
153153
//

‎common/raw_hashtable_metadata_group.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ namespace Carbon::RawHashtable {
4343
// We define a constant max group size. The particular group size used in
4444
// practice may vary, but we want to have some upper bound used to ensure
4545
// memory allocation is done consistently across different architectures.
46-
constexpr ssize_t MaxGroupSize = 16;
46+
inline constexpr ssize_t MaxGroupSize = 16;
4747

4848
// This takes a collection of bits representing the results of looking for a
4949
// particular tag in this metadata group and determines the first position with

‎toolchain/base/int.h‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -206,12 +206,12 @@ class IntId : public Printable<IntId> {
206206
int32_t id_;
207207
};
208208

209-
constexpr IntId IntId::None(IntId::NoneId);
209+
inline constexpr IntId IntId::None(IntId::NoneId);
210210

211211
// Note that we initialize the `None` index in a constexpr context which
212212
// ensures there is no UB in forming it. This helps ensure all the ID -> index
213213
// conversions are correct because the `None` ID is at the limit of that range.
214-
constexpr int32_t IntId::NoneIndex = None.AsIndex();
214+
inline constexpr int32_t IntId::NoneIndex = None.AsIndex();
215215

216216
// A canonicalizing value store with deep optimizations for integers.
217217
//
@@ -428,7 +428,8 @@ class IntStore {
428428
CanonicalValueStore<APIntId, llvm::APInt> values_;
429429
};
430430

431-
constexpr IntStore::APIntId IntStore::APIntId::None(IntId::None.AsIndex());
431+
inline constexpr IntStore::APIntId IntStore::APIntId::None(
432+
IntId::None.AsIndex());
432433

433434
} // namespace Carbon
434435

‎toolchain/base/llvm_tools.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,12 @@ class LLVMTool : public CARBON_ENUM_BASE(LLVMTool) {
6363
CARBON_ENUM_CONSTANT_DEFINITION(LLVMTool, Identifier)
6464
#include "toolchain/base/llvm_tools.def"
6565

66-
constexpr LLVMTool LLVMTool::ToolsStorage[] = {
66+
inline constexpr LLVMTool LLVMTool::ToolsStorage[] = {
6767
#define CARBON_LLVM_TOOL(Identifier, Name, BinName, MainFn) \
6868
LLVMTool::Identifier,
6969
#include "toolchain/base/llvm_tools.def"
7070
};
71-
constexpr llvm::ArrayRef<LLVMTool> LLVMTool::Tools = ToolsStorage;
71+
inline constexpr llvm::ArrayRef<LLVMTool> LLVMTool::Tools = ToolsStorage;
7272

7373
} // namespace Carbon
7474

‎toolchain/base/runtime_sources.bzl‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -54,34 +54,34 @@ _TEMPLATE = """
5454
5555
namespace Carbon::RuntimeSources {{
5656
57-
constexpr inline llvm::StringLiteral CrtBegin = {crtbegin_src};
58-
constexpr inline llvm::StringLiteral CrtEnd = {crtend_src};
57+
inline constexpr llvm::StringLiteral CrtBegin = {crtbegin_src};
58+
inline constexpr llvm::StringLiteral CrtEnd = {crtend_src};
5959
60-
constexpr inline llvm::StringLiteral BuiltinsGenericSrcs[] = {{
60+
inline constexpr llvm::StringLiteral BuiltinsGenericSrcs[] = {{
6161
{generic_srcs}
6262
}};
63-
constexpr inline llvm::StringLiteral BuiltinsMacosSrcs[] = {{
63+
inline constexpr llvm::StringLiteral BuiltinsMacosSrcs[] = {{
6464
{macos_srcs}
6565
}};
66-
constexpr inline llvm::StringLiteral BuiltinsBf16Srcs[] = {{
66+
inline constexpr llvm::StringLiteral BuiltinsBf16Srcs[] = {{
6767
{bf16_srcs}
6868
}};
69-
constexpr inline llvm::StringLiteral BuiltinsTfSrcs[] = {{
69+
inline constexpr llvm::StringLiteral BuiltinsTfSrcs[] = {{
7070
{tf_srcs}
7171
}};
72-
constexpr inline llvm::StringLiteral BuiltinsX86ArchSrcs[] = {{
72+
inline constexpr llvm::StringLiteral BuiltinsX86ArchSrcs[] = {{
7373
{x86_arch_srcs}
7474
}};
75-
constexpr inline llvm::StringLiteral BuiltinsX86Fp80Srcs[] = {{
75+
inline constexpr llvm::StringLiteral BuiltinsX86Fp80Srcs[] = {{
7676
{x86_fp80_srcs}
7777
}};
78-
constexpr inline llvm::StringLiteral BuiltinsAarch64Srcs[] = {{
78+
inline constexpr llvm::StringLiteral BuiltinsAarch64Srcs[] = {{
7979
{aarch64_srcs}
8080
}};
81-
constexpr inline llvm::StringLiteral BuiltinsX86_64Srcs[] = {{
81+
inline constexpr llvm::StringLiteral BuiltinsX86_64Srcs[] = {{
8282
{x86_64_srcs}
8383
}};
84-
constexpr inline llvm::StringLiteral BuiltinsI386Srcs[] = {{
84+
inline constexpr llvm::StringLiteral BuiltinsI386Srcs[] = {{
8585
{i386_srcs}
8686
}};
8787

‎toolchain/base/value_ids.h‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ struct FloatId : public IdBase<FloatId> {
4949
static const FloatId None;
5050
using IdBase::IdBase;
5151
};
52-
constexpr FloatId FloatId::None(FloatId::NoneIndex);
52+
inline constexpr FloatId FloatId::None(FloatId::NoneIndex);
5353

5454
// Corresponds to a Real value.
5555
struct RealId : public IdBase<RealId> {
@@ -62,7 +62,7 @@ struct RealId : public IdBase<RealId> {
6262
static const RealId None;
6363
using IdBase::IdBase;
6464
};
65-
constexpr RealId RealId::None(RealId::NoneIndex);
65+
inline constexpr RealId RealId::None(RealId::NoneIndex);
6666

6767
// Corresponds to StringRefs for identifiers.
6868
//
@@ -73,7 +73,7 @@ struct IdentifierId : public IdBase<IdentifierId> {
7373
static const IdentifierId None;
7474
using IdBase::IdBase;
7575
};
76-
constexpr IdentifierId IdentifierId::None(IdentifierId::NoneIndex);
76+
inline constexpr IdentifierId IdentifierId::None(IdentifierId::NoneIndex);
7777

7878
// The name of a package, which is either an identifier or the special `Core`
7979
// package name.
@@ -122,17 +122,18 @@ struct PackageNameId : public IdBase<PackageNameId> {
122122
}
123123
}
124124
};
125-
constexpr PackageNameId PackageNameId::None(PackageNameId::NoneIndex);
126-
constexpr PackageNameId PackageNameId::Core(PackageNameId::NoneIndex - 1);
127-
constexpr PackageNameId PackageNameId::Cpp(PackageNameId::NoneIndex - 2);
125+
inline constexpr PackageNameId PackageNameId::None(PackageNameId::NoneIndex);
126+
inline constexpr PackageNameId PackageNameId::Core(PackageNameId::NoneIndex -
127+
1);
128+
inline constexpr PackageNameId PackageNameId::Cpp(PackageNameId::NoneIndex - 2);
128129

129130
// Corresponds to StringRefs for string literals.
130131
struct StringLiteralValueId : public IdBase<StringLiteralValueId> {
131132
static constexpr llvm::StringLiteral Label = "string";
132133
static const StringLiteralValueId None;
133134
using IdBase::IdBase;
134135
};
135-
constexpr StringLiteralValueId StringLiteralValueId::None(
136+
inline constexpr StringLiteralValueId StringLiteralValueId::None(
136137
StringLiteralValueId::NoneIndex);
137138

138139
} // namespace Carbon

‎toolchain/check/convert.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ struct TypeExpr {
149149
SemIR::TypeId type_id;
150150
};
151151

152-
constexpr inline TypeExpr TypeExpr::None = {.inst_id = SemIR::TypeInstId::None,
152+
inline constexpr TypeExpr TypeExpr::None = {.inst_id = SemIR::TypeInstId::None,
153153
.type_id = SemIR::TypeId::None};
154154

155155
// Converts an expression for use as a type.

‎toolchain/check/eval_inst.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,13 +86,13 @@ class ConstantEvalResult {
8686
bool same_phase_as_inst_;
8787
};
8888

89-
constexpr ConstantEvalResult ConstantEvalResult::Error =
89+
inline constexpr ConstantEvalResult ConstantEvalResult::Error =
9090
Existing(SemIR::ErrorInst::ConstantId);
9191

92-
constexpr ConstantEvalResult ConstantEvalResult::NotConstant =
92+
inline constexpr ConstantEvalResult ConstantEvalResult::NotConstant =
9393
ConstantEvalResult(SemIR::ConstantId::NotConstant);
9494

95-
constexpr ConstantEvalResult ConstantEvalResult::TODO = NotConstant;
95+
inline constexpr ConstantEvalResult ConstantEvalResult::TODO = NotConstant;
9696

9797
// Implementation details to compute the type of the `EvalConstantInst`
9898
// functions.

‎toolchain/check/keyword_modifier_set.h‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -112,15 +112,19 @@ class KeywordModifierSet : public CARBON_ENUM_MASK_BASE(KeywordModifierSet) {
112112
CARBON_KEYWORD_MODIFIER_SET(CARBON_KEYWORD_MODIFIER_SET_WITH_TYPE)
113113
#undef CARBON_KEYWORD_MODIFIER_SET_WITH_TYPE
114114

115-
constexpr KeywordModifierSet KeywordModifierSet::Access(Private | Protected);
116-
constexpr KeywordModifierSet KeywordModifierSet::Class(Abstract | Base);
117-
constexpr KeywordModifierSet KeywordModifierSet::Method(Abstract | Override |
118-
Virtual);
119-
constexpr KeywordModifierSet KeywordModifierSet::ImplDecl(Extend | Final);
120-
constexpr KeywordModifierSet KeywordModifierSet::Interface(Default | Final);
121-
constexpr KeywordModifierSet KeywordModifierSet::Decl(Class | Method | Impl |
122-
Interface | Export |
123-
Returned);
115+
inline constexpr KeywordModifierSet KeywordModifierSet::Access(Private |
116+
Protected);
117+
inline constexpr KeywordModifierSet KeywordModifierSet::Class(Abstract | Base);
118+
inline constexpr KeywordModifierSet KeywordModifierSet::Method(Abstract |
119+
Override |
120+
Virtual);
121+
inline constexpr KeywordModifierSet KeywordModifierSet::ImplDecl(Extend |
122+
Final);
123+
inline constexpr KeywordModifierSet KeywordModifierSet::Interface(Default |
124+
Final);
125+
inline constexpr KeywordModifierSet KeywordModifierSet::Decl(Class | Method |
126+
Impl | Interface |
127+
Export | Returned);
124128

125129
static_assert(
126130
!KeywordModifierSet::Access.HasAnyOf(KeywordModifierSet::Extern) &&

0 commit comments

Comments
 (0)