phosek added inline comments.
================
Comment at: clang/include/clang/Driver/Multilib.h:150-152
+ bool parseYaml(llvm::MemoryBufferRef,
+ llvm::SourceMgr::DiagHandlerTy = nullptr,
+ void *DiagHandlerCtxt = nullptr);
----------------
Rather than `parseYaml` modifying the state of `MultilibSet`, can we instead
have a static factory method or a standalone function that parses, constructs
and returns `llvm::Expected<MultilibSet>`. That's more in line with how we
handle YAML deserialization elsewhere in LLVM, and also follows the notion of
`MultilibSet` being immutable.
================
Comment at: clang/lib/Driver/Multilib.cpp:188
+ if (StringRef(V.Dir).starts_with("/"))
+ return "paths must be relative. \"" + V.Dir + "\" starts with \"/\"\n";
+ return std::string{};
----------------
This is just a nit, but the error messages in this file are inconsistent.
Sometimes they are full sentences (capitalized, ending with a dot), sometimes
they are not (not-capitalized, no dot at the end) and in this particular case
it's a mixture (not-capitalized, ending with a dot, although it seems like dot
here is only used as a separator). I don't have a strong preference for one
style over the other, but I think we should pick one and be consistent.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D142932/new/
https://reviews.llvm.org/D142932
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
