alexfh added inline comments.
================
Comment at: lib/Tooling/CommonOptionsParser.cpp:122
+ FixedCompilationDatabase::loadFromCommandLine(argc, argv, ErrorMessage);
+ if (!Compilations && StringRef(ErrorMessage).startswith("error"))
+ llvm::errs() << ErrorMessage;
----------------
Getting into the business of encoding the type of error into the string is not
what we want in this particular case ;] Let's just check whether ErrorMessage
is empty (we could go further and make it an Optional<std::string>, but an
empty string seems to be equally reasonable here).
================
Comment at: lib/Tooling/CompilationDatabase.cpp:292
+ if (Argc == 0) {
+ ErrorMsg = "error: no arguments specified\n";
+ return nullptr;
----------------
sepavloff wrote:
> alexfh wrote:
> > The lack of the arguments (or the `--` below) shouldn't be treated as an
> > error (at least an error that is worth showing to the user). The caller
> > will just move on to trying the next kind of a compilation database. The
> > only actual errors we can get here may be coming from `stripPositionalArgs`.
> >
> > The caller should be modified accordingly (i.e. not output anything when
> > both the return value is `nullptr` and `ErrorMsg` is empty).
> What if error message contains a prefix that indicates if this is error or
> warning? Errors could be printed and warnings ignored in
> `CommonOptionsParser.cpp:122`. Such solution would allow clients to
> investigate failures in more details.
>
I don't think it's valuable in this specific code. I'd just return nullptr
without touching ErrorMsg and change the caller to only print something when
the ErrorMsg is not empty.
The lack of the `--` in the command line is not an error, it's just a way to
automatically find a compilation database from the file system (which is
probably even more frequently used feature than the fixed compilation database,
so we definitely should not treat this case as an error).
https://reviews.llvm.org/D33272
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
