grimar added inline comments.
================
Comment at: clang/unittests/Format/FormatTest.cpp:16079
+ auto Style9 = getStyle("file", "/d/.clang-format", "LLVM", "", &FS, true);
+ ASSERT_TRUE((bool)Style9);
}
----------------
It looks like it is very similar to "Test 7:" and can be a part of it?
E.g:
```
...
llvm::consumeError(Style7a.takeError());
// <comment>
auto Style7b = getStyle("file", "/d/.clang-format", "LLVM", "", &FS, true);
ASSERT_TRUE((bool)Style8);
```
================
Comment at: llvm/unittests/ObjectYAML/YAMLTest.cpp:39
+
+TEST(ObjectYAML, UnkownOption) {
+ std::string InputYAML = "Binary: AAAA\n"
----------------
Unk**n**ownOption
================
Comment at: llvm/unittests/ObjectYAML/YAMLTest.cpp:40
+TEST(ObjectYAML, UnkownOption) {
+ std::string InputYAML = "Binary: AAAA\n"
+ "InvalidKey: InvalidValue";
----------------
`std::string`->`StringRef`?
================
Comment at: llvm/unittests/ObjectYAML/YAMLTest.cpp:41
+ std::string InputYAML = "Binary: AAAA\n"
+ "InvalidKey: InvalidValue";
+ BinaryHolder BH;
----------------
Will this work if we switch the order of `InvalidKey` and `Binary`?
```
std::string InputYAML = "InvalidKey: InvalidValue\n"
"Binary: AAAA";
```
I expect that yes and I think it is reasonable to do it to show that we don't
stop parsing an YAML
when there is an unknown key.
================
Comment at: llvm/unittests/ObjectYAML/YAMLTest.cpp:44
+ llvm::yaml::Input Input(InputYAML);
+ // test 1: default in trying to parse invalid key is an error case
+ Input >> BH;
----------------
No full stop at the end.
================
Comment at: llvm/unittests/ObjectYAML/YAMLTest.cpp:57
+ EXPECT_EQ(BH2.Binary, BH_GT.Binary);
+ EXPECT_EQ(Input2.error().value(), 0);
+}
----------------
Do you need `BH_GT`? Can it be just:
```
// test 2: only warn about invalid key if actively set.
llvm::yaml::Input Input2(InputYAML);
BinaryHolder BH2;
Input2.setAllowUnknownKeys(true);
Input2 >> BH2;
EXPECT_EQ(Input2.error().value(), 0);
EXPECT_EQ(BH2.Binary, yaml::BinaryRef("AAAA"));
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86137/new/
https://reviews.llvm.org/D86137
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
