Skip to content

Commit e194bda

Browse files
vhscamposvhscampos
committed
[Multilib] Custom flags YAML parsing
This patch adds support for custom flags in the multilib YAML file. Details about this change can be found in: https://discourse.llvm.org/t/rfc-multilib-custom-flags/81058 CustomFlagDeclaration objects are instantiated using shared_ptr. This is motivated by the fact that each custom flag value, CustomFlagValueDetail, contains a back reference to their corresponding flag declaration. Since the CustomFlagDeclaration objects are transferred from the YAML parser to the MultilibSet instance after the parsing is finished, back references implemented as raw pointers would become dangling. This would need to be remediated in the copy/move constructors by updating the pointer. Therefore it's just simpler and less error-prone to have all references to CustomFlagDeclaration, including the back reference, as shared_ptr. This way dangling pointers are not a concern.
1 parent 82d2df2 commit e194bda

File tree

3 files changed

+230
-13
lines changed

3 files changed

+230
-13
lines changed

‎clang/include/clang/Driver/Multilib.h

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,25 @@ class Multilib {
101101

102102
raw_ostream &operator<<(raw_ostream &OS, const Multilib &M);
103103

104+
namespace custom_flag {
105+
struct CustomFlagDeclaration;
106+
using CustomFlagDeclarationPtr = std::shared_ptr<CustomFlagDeclaration>;
107+
108+
struct CustomFlagValueDetail {
109+
std::string Name;
110+
std::optional<SmallVector<std::string>> ExtraBuildArgs;
111+
CustomFlagDeclarationPtr Decl;
112+
};
113+
114+
struct CustomFlagDeclaration {
115+
std::string Name;
116+
SmallVector<CustomFlagValueDetail> ValueList;
117+
size_t DefaultValueIdx = ~0UL;
118+
};
119+
120+
static constexpr StringRef Prefix = "-fmultilib-flag=";
121+
} // namespace custom_flag
122+
104123
/// See also MultilibSetBuilder for combining multilibs into a set.
105124
class MultilibSet {
106125
public:
@@ -120,15 +139,18 @@ class MultilibSet {
120139

121140
private:
122141
multilib_list Multilibs;
123-
std::vector<FlagMatcher> FlagMatchers;
142+
SmallVector<FlagMatcher> FlagMatchers;
143+
SmallVector<custom_flag::CustomFlagDeclarationPtr> CustomFlagDecls;
124144
IncludeDirsFunc IncludeCallback;
125145
IncludeDirsFunc FilePathsCallback;
126146

127147
public:
128148
MultilibSet() = default;
129-
MultilibSet(multilib_list &&Multilibs,
130-
std::vector<FlagMatcher> &&FlagMatchers = {})
131-
: Multilibs(Multilibs), FlagMatchers(FlagMatchers) {}
149+
MultilibSet(
150+
multilib_list &&Multilibs, SmallVector<FlagMatcher> &&FlagMatchers = {},
151+
SmallVector<custom_flag::CustomFlagDeclarationPtr> &&CustomFlagDecls = {})
152+
: Multilibs(std::move(Multilibs)), FlagMatchers(std::move(FlagMatchers)),
153+
CustomFlagDecls(std::move(CustomFlagDecls)) {}
132154

133155
const multilib_list &getMultilibs() { return Multilibs; }
134156

‎clang/lib/Driver/Multilib.cpp

Lines changed: 71 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
#include "clang/Basic/Version.h"
1212
#include "clang/Driver/Driver.h"
1313
#include "llvm/ADT/DenseSet.h"
14-
#include "llvm/ADT/SmallString.h"
14+
#include "llvm/ADT/SmallSet.h"
1515
#include "llvm/ADT/StringRef.h"
1616
#include "llvm/Support/Compiler.h"
1717
#include "llvm/Support/Error.h"
@@ -205,13 +205,20 @@ struct MultilibGroupSerialization {
205205

206206
struct MultilibSetSerialization {
207207
llvm::VersionTuple MultilibVersion;
208-
std::vector<MultilibGroupSerialization> Groups;
209-
std::vector<MultilibSerialization> Multilibs;
210-
std::vector<MultilibSet::FlagMatcher> FlagMatchers;
208+
SmallVector<MultilibGroupSerialization> Groups;
209+
SmallVector<MultilibSerialization> Multilibs;
210+
SmallVector<MultilibSet::FlagMatcher> FlagMatchers;
211+
SmallVector<custom_flag::CustomFlagDeclarationPtr> CustomFlagDeclarations;
211212
};
212213

213214
} // end anonymous namespace
214215

216+
LLVM_YAML_IS_SEQUENCE_VECTOR(MultilibSerialization)
217+
LLVM_YAML_IS_SEQUENCE_VECTOR(MultilibGroupSerialization)
218+
LLVM_YAML_IS_SEQUENCE_VECTOR(MultilibSet::FlagMatcher)
219+
LLVM_YAML_IS_SEQUENCE_VECTOR(custom_flag::CustomFlagValueDetail)
220+
LLVM_YAML_IS_SEQUENCE_VECTOR(custom_flag::CustomFlagDeclarationPtr)
221+
215222
template <> struct llvm::yaml::MappingTraits<MultilibSerialization> {
216223
static void mapping(llvm::yaml::IO &io, MultilibSerialization &V) {
217224
io.mapOptional("Dir", V.Dir);
@@ -259,11 +266,69 @@ template <> struct llvm::yaml::MappingTraits<MultilibSet::FlagMatcher> {
259266
}
260267
};
261268

269+
template <>
270+
struct llvm::yaml::MappingContextTraits<custom_flag::CustomFlagValueDetail,
271+
llvm::SmallSet<std::string, 32>> {
272+
static void mapping(llvm::yaml::IO &io, custom_flag::CustomFlagValueDetail &V,
273+
llvm::SmallSet<std::string, 32> &) {
274+
io.mapRequired("Name", V.Name);
275+
io.mapOptional("ExtraBuildArgs", V.ExtraBuildArgs);
276+
}
277+
static std::string validate(IO &io, custom_flag::CustomFlagValueDetail &V,
278+
llvm::SmallSet<std::string, 32> &NameSet) {
279+
if (V.Name.empty())
280+
return "custom flag value requires a name";
281+
if (!NameSet.insert(V.Name).second)
282+
return "duplicate custom flag value name: \"" + V.Name + "\"";
283+
return {};
284+
}
285+
};
286+
287+
template <>
288+
struct llvm::yaml::MappingContextTraits<custom_flag::CustomFlagDeclarationPtr,
289+
llvm::SmallSet<std::string, 32>> {
290+
static void mapping(llvm::yaml::IO &io,
291+
custom_flag::CustomFlagDeclarationPtr &V,
292+
llvm::SmallSet<std::string, 32> &NameSet) {
293+
assert(!V);
294+
V = std::make_shared<custom_flag::CustomFlagDeclaration>();
295+
io.mapRequired("Name", V->Name);
296+
io.mapRequired("Values", V->ValueList, NameSet);
297+
std::string DefaultValueName;
298+
io.mapRequired("Default", DefaultValueName);
299+
300+
for (auto [Idx, Value] : llvm::enumerate(V->ValueList)) {
301+
Value.Decl = V;
302+
if (Value.Name == DefaultValueName) {
303+
assert(V->DefaultValueIdx == ~0UL);
304+
V->DefaultValueIdx = Idx;
305+
}
306+
}
307+
}
308+
static std::string validate(IO &io, custom_flag::CustomFlagDeclarationPtr &V,
309+
llvm::SmallSet<std::string, 32> &) {
310+
if (V->Name.empty())
311+
return "custom flag requires a name";
312+
if (V->ValueList.empty())
313+
return "custom flag must have at least one value";
314+
if (V->DefaultValueIdx >= V->ValueList.size())
315+
return "custom flag must have a default value";
316+
if (llvm::any_of(V->ValueList, [&V](const auto &Value) {
317+
return !Value.Decl || Value.Decl != V;
318+
}))
319+
return "custom flag value missing reference to its custom flag "
320+
"declaration";
321+
return {};
322+
}
323+
};
324+
262325
template <> struct llvm::yaml::MappingTraits<MultilibSetSerialization> {
263326
static void mapping(llvm::yaml::IO &io, MultilibSetSerialization &M) {
264327
io.mapRequired("MultilibVersion", M.MultilibVersion);
265328
io.mapRequired("Variants", M.Multilibs);
266329
io.mapOptional("Groups", M.Groups);
330+
llvm::SmallSet<std::string, 32> NameSet;
331+
io.mapOptionalWithContext("Flags", M.CustomFlagDeclarations, NameSet);
267332
io.mapOptional("Mappings", M.FlagMatchers);
268333
}
269334
static std::string validate(IO &io, MultilibSetSerialization &M) {
@@ -292,10 +357,6 @@ template <> struct llvm::yaml::MappingTraits<MultilibSetSerialization> {
292357
}
293358
};
294359

295-
LLVM_YAML_IS_SEQUENCE_VECTOR(MultilibSerialization)
296-
LLVM_YAML_IS_SEQUENCE_VECTOR(MultilibGroupSerialization)
297-
LLVM_YAML_IS_SEQUENCE_VECTOR(MultilibSet::FlagMatcher)
298-
299360
llvm::ErrorOr<MultilibSet>
300361
MultilibSet::parseYaml(llvm::MemoryBufferRef Input,
301362
llvm::SourceMgr::DiagHandlerTy DiagHandler,
@@ -323,7 +384,8 @@ MultilibSet::parseYaml(llvm::MemoryBufferRef Input,
323384
}
324385
}
325386

326-
return MultilibSet(std::move(Multilibs), std::move(MS.FlagMatchers));
387+
return MultilibSet(std::move(Multilibs), std::move(MS.FlagMatchers),
388+
std::move(MS.CustomFlagDeclarations));
327389
}
328390

329391
LLVM_DUMP_METHOD void MultilibSet::dump() const {

‎clang/test/Driver/baremetal-multilib-custom-flags-parsing.yaml

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
# RUN: split-file %s %t
2+
3+
# RUN: %clang --target=arm-none-eabi --multi-lib-config=%t/multilib-without-extra-build-args.yaml %s -### -o /dev/null 2>&1 \
4+
# RUN: | FileCheck %s
5+
# RUN: %clang --target=arm-none-eabi --multi-lib-config=%t/multilib-with-extra-build-args.yaml %s -### -o /dev/null 2>&1 \
6+
# RUN: | FileCheck %s
7+
# CHECK-NOT: error:
8+
9+
# RUN: %clang --target=arm-none-eabi --multi-lib-config=%t/missing-flag-name.yaml %s -### -o /dev/null 2>&1 \
10+
# RUN: | FileCheck %s --check-prefix=CHECK-MISSING-FLAG-NAME
11+
# CHECK-MISSING-FLAG-NAME: error: custom flag requires a name
12+
13+
# RUN: %clang --target=arm-none-eabi --multi-lib-config=%t/missing-flag-values.yaml %s -### -o /dev/null 2>&1 \
14+
# RUN: | FileCheck %s --check-prefix=CHECK-MISSING-FLAG-VALUES
15+
# CHECK-MISSING-FLAG-VALUES: error: custom flag must have at least one value
16+
17+
# RUN: %clang --target=arm-none-eabi --multi-lib-config=%t/missing-flag-value-default.yaml %s -### -o /dev/null 2>&1 \
18+
# RUN: | FileCheck %s --check-prefix=CHECK-MISSING-FLAG-VALUE-DEFAULT
19+
# CHECK-MISSING-FLAG-VALUE-DEFAULT: error: custom flag must have a default value
20+
21+
# RUN: %clang --target=arm-none-eabi --multi-lib-config=%t/missing-flag-value-name.yaml %s -### -o /dev/null 2>&1 \
22+
# RUN: | FileCheck %s --check-prefix=CHECK-MISSING-FLAG-VALUE-NAME
23+
# CHECK-MISSING-FLAG-VALUE-NAME: error: custom flag value requires a name
24+
25+
# RUN: %clang --target=arm-none-eabi --multi-lib-config=%t/duplicate-flag-value-name.yaml %s -### -o /dev/null 2>&1 \
26+
# RUN: | FileCheck %s --check-prefix=CHECK-DUPLICATE-FLAG-VALUE-NAME
27+
# CHECK-DUPLICATE-FLAG-VALUE-NAME: error: duplicate custom flag value name: "value-name"
28+
# CHECK-DUPLICATE-FLAG-VALUE-NAME-NEXT: - Name: value-name
29+
30+
#--- multilib-without-extra-build-args.yaml
31+
---
32+
MultilibVersion: 1.0
33+
34+
Variants:
35+
- Dir: libc
36+
Flags: [-fmultilib-flag=a]
37+
38+
Flags:
39+
- Name: flag
40+
Values:
41+
- Name: a
42+
- Name: b
43+
Default: a
44+
45+
#--- multilib-with-extra-build-args.yaml
46+
---
47+
MultilibVersion: 1.0
48+
49+
Variants:
50+
- Dir: libc
51+
Flags: [-fmultilib-flag=a]
52+
53+
Flags:
54+
- Name: flag
55+
Values:
56+
- Name: a
57+
ExtraBuildArgs: [-DFEATURE_A]
58+
- Name: b
59+
ExtraBuildArgs: [-DFEATURE_B]
60+
Default: a
61+
62+
#--- missing-flag-name.yaml
63+
---
64+
MultilibVersion: 1.0
65+
66+
Variants:
67+
- Dir: libc
68+
Flags: [-fmultilib-flag=a]
69+
70+
Flags:
71+
- Values:
72+
- Name: a
73+
Default: a
74+
75+
#--- missing-flag-values.yaml
76+
---
77+
MultilibVersion: 1.0
78+
79+
Variants:
80+
- Dir: libc
81+
Flags: [-fmultilib-flag=a]
82+
83+
Flags:
84+
- Name: flag
85+
Values:
86+
Default: a
87+
88+
#--- missing-flag-value-default.yaml
89+
---
90+
MultilibVersion: 1.0
91+
92+
Variants:
93+
- Dir: libc
94+
Flags: [-fmultilib-flag=a]
95+
96+
Flags:
97+
- Name: flag
98+
Values:
99+
- Name: a
100+
Default:
101+
102+
#--- missing-flag-value-name.yaml
103+
---
104+
MultilibVersion: 1.0
105+
106+
Variants:
107+
- Dir: libc
108+
Flags: [-fmultilib-flag=a]
109+
110+
Flags:
111+
- Name: flag
112+
Values:
113+
- Name:
114+
Default: a
115+
116+
#--- duplicate-flag-value-name.yaml
117+
---
118+
MultilibVersion: 1.0
119+
120+
Variants:
121+
- Dir: libc
122+
Flags: [-fmultilib-flag=value-name]
123+
124+
Flags:
125+
- Name: a
126+
Values:
127+
- Name: value-name
128+
- Name: value-a
129+
Default: value-name
130+
- Name: b
131+
Values:
132+
- Name: value-name
133+
Default: value-name

0 commit comments

Comments
 (0)