Skip to content

Commit 4c9756a

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 12cfa41 commit 4c9756a

File tree

3 files changed

+223
-11
lines changed

3 files changed

+223
-11
lines changed

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

Lines changed: 25 additions & 3 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 Declaration;
106+
using DeclarationPtr = std::shared_ptr<Declaration>;
107+
108+
struct ValueDetail {
109+
std::string Name;
110+
std::optional<SmallVector<std::string>> DriverArgs;
111+
DeclarationPtr Decl;
112+
};
113+
114+
struct Declaration {
115+
std::string Name;
116+
SmallVector<ValueDetail> ValueList;
117+
std::optional<size_t> DefaultValueIdx;
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::DeclarationPtr> CustomFlagDecls;
124144
IncludeDirsFunc IncludeCallback;
125145
IncludeDirsFunc FilePathsCallback;
126146

127147
public:
128148
MultilibSet() = default;
129149
MultilibSet(multilib_list &&Multilibs,
130-
std::vector<FlagMatcher> &&FlagMatchers = {})
131-
: Multilibs(Multilibs), FlagMatchers(FlagMatchers) {}
150+
SmallVector<FlagMatcher> &&FlagMatchers = {},
151+
SmallVector<custom_flag::DeclarationPtr> &&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: 65 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "clang/Basic/LLVM.h"
1111
#include "clang/Driver/Driver.h"
1212
#include "llvm/ADT/DenseSet.h"
13+
#include "llvm/ADT/SmallSet.h"
1314
#include "llvm/ADT/StringRef.h"
1415
#include "llvm/Support/Compiler.h"
1516
#include "llvm/Support/ErrorHandling.h"
@@ -201,13 +202,20 @@ struct MultilibGroupSerialization {
201202

202203
struct MultilibSetSerialization {
203204
llvm::VersionTuple MultilibVersion;
204-
std::vector<MultilibGroupSerialization> Groups;
205-
std::vector<MultilibSerialization> Multilibs;
206-
std::vector<MultilibSet::FlagMatcher> FlagMatchers;
205+
SmallVector<MultilibGroupSerialization> Groups;
206+
SmallVector<MultilibSerialization> Multilibs;
207+
SmallVector<MultilibSet::FlagMatcher> FlagMatchers;
208+
SmallVector<custom_flag::DeclarationPtr> CustomFlagDeclarations;
207209
};
208210

209211
} // end anonymous namespace
210212

213+
LLVM_YAML_IS_SEQUENCE_VECTOR(MultilibSerialization)
214+
LLVM_YAML_IS_SEQUENCE_VECTOR(MultilibGroupSerialization)
215+
LLVM_YAML_IS_SEQUENCE_VECTOR(MultilibSet::FlagMatcher)
216+
LLVM_YAML_IS_SEQUENCE_VECTOR(custom_flag::ValueDetail)
217+
LLVM_YAML_IS_SEQUENCE_VECTOR(custom_flag::DeclarationPtr)
218+
211219
template <> struct llvm::yaml::MappingTraits<MultilibSerialization> {
212220
static void mapping(llvm::yaml::IO &io, MultilibSerialization &V) {
213221
io.mapOptional("Dir", V.Dir);
@@ -255,11 +263,63 @@ template <> struct llvm::yaml::MappingTraits<MultilibSet::FlagMatcher> {
255263
}
256264
};
257265

266+
template <>
267+
struct llvm::yaml::MappingContextTraits<custom_flag::ValueDetail,
268+
llvm::SmallSet<std::string, 32>> {
269+
static void mapping(llvm::yaml::IO &io, custom_flag::ValueDetail &V,
270+
llvm::SmallSet<std::string, 32> &) {
271+
io.mapRequired("Name", V.Name);
272+
io.mapOptional("DriverArgs", V.DriverArgs);
273+
}
274+
static std::string validate(IO &io, custom_flag::ValueDetail &V,
275+
llvm::SmallSet<std::string, 32> &NameSet) {
276+
if (V.Name.empty())
277+
return "custom flag value requires a name";
278+
if (!NameSet.insert(V.Name).second)
279+
return "duplicate custom flag value name: \"" + V.Name + "\"";
280+
return {};
281+
}
282+
};
283+
284+
template <>
285+
struct llvm::yaml::MappingContextTraits<custom_flag::DeclarationPtr,
286+
llvm::SmallSet<std::string, 32>> {
287+
static void mapping(llvm::yaml::IO &io, custom_flag::DeclarationPtr &V,
288+
llvm::SmallSet<std::string, 32> &NameSet) {
289+
assert(!V);
290+
V = std::make_shared<custom_flag::Declaration>();
291+
io.mapRequired("Name", V->Name);
292+
io.mapRequired("Values", V->ValueList, NameSet);
293+
std::string DefaultValueName;
294+
io.mapRequired("Default", DefaultValueName);
295+
296+
for (auto [Idx, Value] : llvm::enumerate(V->ValueList)) {
297+
Value.Decl = V;
298+
if (Value.Name == DefaultValueName) {
299+
assert(!V->DefaultValueIdx);
300+
V->DefaultValueIdx = Idx;
301+
}
302+
}
303+
}
304+
static std::string validate(IO &io, custom_flag::DeclarationPtr &V,
305+
llvm::SmallSet<std::string, 32> &) {
306+
if (V->Name.empty())
307+
return "custom flag requires a name";
308+
if (V->ValueList.empty())
309+
return "custom flag must have at least one value";
310+
if (!V->DefaultValueIdx)
311+
return "custom flag must have a default value";
312+
return {};
313+
}
314+
};
315+
258316
template <> struct llvm::yaml::MappingTraits<MultilibSetSerialization> {
259317
static void mapping(llvm::yaml::IO &io, MultilibSetSerialization &M) {
260318
io.mapRequired("MultilibVersion", M.MultilibVersion);
261319
io.mapRequired("Variants", M.Multilibs);
262320
io.mapOptional("Groups", M.Groups);
321+
llvm::SmallSet<std::string, 32> NameSet;
322+
io.mapOptionalWithContext("Flags", M.CustomFlagDeclarations, NameSet);
263323
io.mapOptional("Mappings", M.FlagMatchers);
264324
}
265325
static std::string validate(IO &io, MultilibSetSerialization &M) {
@@ -288,10 +348,6 @@ template <> struct llvm::yaml::MappingTraits<MultilibSetSerialization> {
288348
}
289349
};
290350

291-
LLVM_YAML_IS_SEQUENCE_VECTOR(MultilibSerialization)
292-
LLVM_YAML_IS_SEQUENCE_VECTOR(MultilibGroupSerialization)
293-
LLVM_YAML_IS_SEQUENCE_VECTOR(MultilibSet::FlagMatcher)
294-
295351
llvm::ErrorOr<MultilibSet>
296352
MultilibSet::parseYaml(llvm::MemoryBufferRef Input,
297353
llvm::SourceMgr::DiagHandlerTy DiagHandler,
@@ -319,7 +375,8 @@ MultilibSet::parseYaml(llvm::MemoryBufferRef Input,
319375
}
320376
}
321377

322-
return MultilibSet(std::move(Multilibs), std::move(MS.FlagMatchers));
378+
return MultilibSet(std::move(Multilibs), std::move(MS.FlagMatchers),
379+
std::move(MS.CustomFlagDeclarations));
323380
}
324381

325382
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+
DriverArgs: [-DFEATURE_A]
58+
- Name: b
59+
DriverArgs: [-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)