[clang][cli] Report result of ParseLangArgs

[jansvoboda11]

Summary

This patch correctly reports success/failure of ParseLangArgs. Besides being consistent with other Parse functions, this is required to make round-tripping of LangOptions work.

[dexonsmith]

clang/lib/Frontend/CompilerInvocation.cpp

3082

Would this avoid the changes above?

return Success && !Diags.hasErrorOccurred();

?

Alternatively, might it be cleaner to shove the Success = false into a helper lambda or something?

[jansvoboda11]

Rely on DiagnosticsEngine when reporting parsing result

[jansvoboda11]

clang/lib/Frontend/CompilerInvocation.cpp

3082

I've implemented something similar in my latest update. I think it works out pretty well, thanks for the suggestion.

[dexonsmith]

LGTM.

clang/include/clang/Basic/Diagnostic.h

public:
   }
   bool hasFatalErrorOccurred() const { return FatalErrorOccurred; }
 
   /// Determine whether any kind of unrecoverable error has occurred.
   bool hasUnrecoverableErrorOccurred() const {
     return FatalErrorOccurred || UnrecoverableErrorOccurred;
   }
 
+  unsigned getNumErrors() const { return NumErrors; }
   unsigned getNumWarnings() const { return NumWarnings; }
 
   void setNumWarnings(unsigned NumWarnings) {
     this->NumWarnings = NumWarnings;
   }
 
   /// Return an ID for a diagnostic with the specified format string and
   /// level.

clang/include/clang/Frontend/CompilerInvocation.h

private:
   ///
   /// \param Args - The argument list containing the arguments to parse
   /// \param Diags - The DiagnosticsEngine associated with CreateFromArgs
   /// \returns - True if parsing was successful, false otherwise
   bool parseSimpleArgs(const llvm::opt::ArgList &Args,
                        DiagnosticsEngine &Diags);
 
   /// Parse command line options that map to LangOptions.
-  static void ParseLangArgs(LangOptions &Opts, llvm::opt::ArgList &Args,
+  static bool ParseLangArgs(LangOptions &Opts, llvm::opt::ArgList &Args,
                             InputKind IK, const llvm::Triple &T,
                             std::vector<std::string> &Includes,
                             DiagnosticsEngine &Diags);
 
   /// Parse command line options that map to CodeGenOptions.
   static bool ParseCodeGenArgs(CodeGenOptions &Opts, llvm::opt::ArgList &Args,
                                InputKind IK, DiagnosticsEngine &Diags,
                                const llvm::Triple &T,

clang/lib/Frontend/CompilerInvocation.cpp

static void GenerateLangArgs(const LangOptions &Opts,
                              SmallVectorImpl<const char *> &Args,
                              CompilerInvocation::StringAllocator SA) {
   if (Opts.IncludeDefaultHeader)
     GenerateArg(Args, OPT_finclude_default_header, SA);
   if (Opts.DeclareOpenCLBuiltins)
     GenerateArg(Args, OPT_fdeclare_opencl_builtins, SA);
 }
 
-void CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
+bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
                                        InputKind IK, const llvm::Triple &T,
                                        std::vector<std::string> &Includes,
                                        DiagnosticsEngine &Diags) {
+  unsigned NumErrorsBefore = Diags.getNumErrors();
+
   // FIXME: Cleanup per-file based stuff.
   LangStandard::Kind LangStd = LangStandard::lang_unspecified;
   if (const Arg *A = Args.getLastArg(OPT_std_EQ)) {
     LangStd = LangStandard::getLangKind(A->getValue());
     if (LangStd == LangStandard::lang_unspecified) {
       Diags.Report(diag::err_drv_invalid_value)
         << A->getAsString(Args) << A->getValue();
       // Report supported standards with short description.
if (Arg *A = Args.getLastArg(OPT_msign_return_address_key_EQ)) {
           Opts.setSignReturnAddressKey(
               LangOptions::SignReturnAddressKeyKind::BKey);
         else
           Diags.Report(diag::err_drv_invalid_value)
               << A->getAsString(Args) << SignKey;
       }
     }
   }
+
+  return Success && Diags.getNumErrors() == NumErrorsBefore;
 }
 
 static bool isStrictlyPreprocessorAction(frontend::ActionKind Action) {
   switch (Action) {
   case frontend::ASTDeclList:
   case frontend::ASTDump:
   case frontend::ASTPrint:
   case frontend::ASTView:
if (DashX.getFormat() == InputKind::Precompiled ||
     // set regardless of the input type.
     LangOpts.PICLevel = getLastArgIntValue(Args, OPT_pic_level, 0, Diags);
     LangOpts.PIE = Args.hasArg(OPT_pic_is_pie);
     parseSanitizerKinds("-fsanitize=", Args.getAllArgValues(OPT_fsanitize_EQ),
                         Diags, LangOpts.Sanitize);
   } else {
     // Other LangOpts are only initialized when the input is not AST or LLVM IR.
     // FIXME: Should we really be calling this for an Language::Asm input?
-    ParseLangArgs(LangOpts, Args, DashX, T, Res.getPreprocessorOpts().Includes,
-                  Diags);
+    Success &= ParseLangArgs(LangOpts, Args, DashX, T,
+                             Res.getPreprocessorOpts().Includes, Diags);
     if (Res.getFrontendOpts().ProgramAction == frontend::RewriteObjC)
       LangOpts.ObjCExceptions = 1;
     if (T.isOSDarwin() && DashX.isPreprocessed()) {
       // Supress the darwin-specific 'stdlibcxx-not-found' diagnostic for
       // preprocessed input as we don't expect it to be used with -std=libc++
       // anyway.
       Res.getDiagnosticOpts().Warnings.push_back("no-stdlibcxx-not-found");
     }