grimar added inline comments.
================
Comment at: llvm/lib/Support/YAMLTraits.cpp:204
if (!is_contained(MN->ValidKeys, NN.first())) {
- setError(NN.second.get(), Twine("unknown key '") + NN.first() + "'");
- break;
+ auto ReportNode = NN.second.get();
+ auto ReportMessage = Twine("unknown key '") + NN.first() + "'";
----------------
Please don't use `auto` when the variable type is not obvious.
Use the real type name instead.
================
Comment at: llvm/lib/Support/YAMLTraits.cpp:205
+ auto ReportNode = NN.second.get();
+ auto ReportMessage = Twine("unknown key '") + NN.first() + "'";
+ if (!AllowUnknownKeys) {
----------------
The same here, but also the result type here is `Twine`, and you shouldn't use
it like this. Documentation says:
"A Twine is not intended for use directly and should not be stored, its
implementation relies on the ability to store pointers to temporary stack
objects which may be deallocated at the end of a statement. Twines should only
be used accepted as const references in arguments, when an API wishes to accept
possibly-concatenated strings."
(https://llvm.org/doxygen/classllvm_1_1Twine.html#details)
You can probably inline it or use `std::string`, since this is a error path
only code.
================
Comment at: llvm/lib/Support/YAMLTraits.cpp:387
+
+void Input::reportWarning(Node *node, const Twine &message) {
+ Strm->printError(node, message, SourceMgr::DK_Warning);
----------------
Why do you need both `reportWarning` methods? I think you can have only one for
now.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86137/new/
https://reviews.llvm.org/D86137
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
