Use -Rblah, not -Wblah, to control remark diagnostics. This was always the

intent when we added remark support, but was never implemented in the general
case, because the first -R flags didn't need it. (-Rpass= had special handling
to accomodate its argument.)

-Rno-foo, -Reverything, and -Rno-everything can be used to turn off a remark,
or to turn on or off all remarks. Per discussion on cfe-commits, -Weverything
does not affect remarks, and -Reverything does not affect warnings or errors.

The only "real" -R flag we have right now is -Rmodule-build; that flag is
effectively renamed from -Wmodule-build to -Rmodule-build by this change.

-Wpass and -Wno-pass (and their friends) are also renamed to -Rpass and
-Rno-pass by this change; it's not completely clear whether we intended to have
a -Rpass (with no =pattern), but that is unchanged by this commit, other than
the flag name. The default pattern is effectively one which matches no passes.
In future, we may want to make the default pattern be .*, so that -Reverything
works for -Rpass properly.


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@215046 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/Basic/Diagnostic.h

@@ -542,9 +542,13 @@ public:
   /// \returns true (and ignores the request) if "Group" was unknown, false
   /// otherwise.
   ///
+  /// \param Flavor The flavor of group to affect. -Rfoo does not affect the
+  /// state of the -Wfoo group and vice versa.
+  ///
   /// \param Loc The source location that this change of diagnostic state should
   /// take affect. It can be null if we are setting the state from command-line.
-  bool setSeverityForGroup(StringRef Group, diag::Severity Map,
+  bool setSeverityForGroup(diag::Flavor Flavor, StringRef Group,
+                           diag::Severity Map,
                            SourceLocation Loc = SourceLocation());
 
   /// \brief Set the warning-as-error flag for the given diagnostic group.
@@ -561,11 +565,12 @@ public:
   /// \returns True if the given group is unknown, false otherwise.
   bool setDiagnosticGroupErrorAsFatal(StringRef Group, bool Enabled);
 
-  /// \brief Add the specified mapping to all diagnostics.
+  /// \brief Add the specified mapping to all diagnostics of the specified
+  /// flavor.
   ///
   /// Mainly to be used by -Wno-everything to disable all warnings but allow
   /// subsequent -W options to enable specific warnings.
-  void setSeverityForAll(diag::Severity Map,
+  void setSeverityForAll(diag::Flavor Flavor, diag::Severity Map,
                          SourceLocation Loc = SourceLocation());
 
   bool hasErrorOccurred() const { return ErrorOccurred; }

include/clang/Basic/DiagnosticFrontendKinds.td

@@ -36,11 +36,11 @@ def remark_fe_backend_plugin: Remark<"%0">, BackendInfo, InGroup<RemarkBackendPl
 def note_fe_backend_plugin: Note<"%0">, BackendInfo;
 
 def remark_fe_backend_optimization_remark : Remark<"%0">, BackendInfo,
-    InGroup<BackendOptimizationRemark>, DefaultRemark;
+    InGroup<BackendOptimizationRemark>;
 def remark_fe_backend_optimization_remark_missed : Remark<"%0">, BackendInfo,
-    InGroup<BackendOptimizationRemarkMissed>, DefaultRemark;
+    InGroup<BackendOptimizationRemarkMissed>;
 def remark_fe_backend_optimization_remark_analysis : Remark<"%0">, BackendInfo,
-    InGroup<BackendOptimizationRemarkAnalysis>, DefaultRemark;
+    InGroup<BackendOptimizationRemarkAnalysis>;
 def warn_fe_backend_optimization_failure : Warning<"%0">, BackendInfo,
     InGroup<BackendOptimizationFailure>, DefaultWarn;
 def note_fe_backend_optimization_remark_invalid_loc : Note<"could "
@@ -126,17 +126,8 @@ def err_relocatable_without_isysroot : Error<
     "must specify system root with -isysroot when building a relocatable "
     "PCH file">;
 
-def warn_unknown_warning_option : Warning<
-    "unknown warning option '%0'">,
-    InGroup<UnknownWarningOption>;
-def warn_unknown_negative_warning_option : Warning<
-    "unknown warning option '%0'">,
-    InGroup<UnknownWarningOption>;
-def warn_unknown_warning_option_suggest : Warning<
-    "unknown warning option '%0'; did you mean '%1'?">,
-    InGroup<UnknownWarningOption>;
-def warn_unknown_negative_warning_option_suggest : Warning<
-    "unknown warning option '%0'; did you mean '%1'?">,
+def warn_unknown_diag_option : Warning<
+    "unknown %select{warning|remark}0 option '%1'%select{|; did you mean '%3'?}2">,
     InGroup<UnknownWarningOption>;
 def warn_unknown_warning_specifier : Warning<
     "unknown %0 warning specifier: '%1'">,
@@ -176,7 +167,7 @@ def warn_module_config_macro_undef : Warning<
 def note_module_def_undef_here : Note<
   "macro was %select{defined|#undef'd}0 here">;
 def remark_module_build : Remark<"building module '%0' as '%1'">,
-  InGroup<DiagGroup<"module-build">>, DefaultIgnore;
+  InGroup<DiagGroup<"module-build">>;
 
 def err_conflicting_module_names : Error<
   "conflicting module names specified: '-fmodule-name=%0' and "

include/clang/Basic/DiagnosticIDs.h

@@ -67,6 +67,15 @@ namespace clang {
       Error = 4,   ///< Present this diagnostic as an error.
       Fatal = 5    ///< Present this diagnostic as a fatal error.
     };
+
+    /// Flavors of diagnostics we can emit. Used to filter for a particular
+    /// kind of diagnostic (for instance, for -W/-R flags).
+    enum class Flavor {
+      WarningOrError, ///< A diagnostic that indicates a problem or potential
+                      ///< problem. Can be made fatal by -Werror.
+      Remark          ///< A diagnostic that indicates normal progress through
+                      ///< compilation.
+    };
   }
 
 class DiagnosticMapping {
@@ -228,15 +237,16 @@ public:
   ///
   /// \param[out] Diags - On return, the diagnostics in the group.
   /// \returns \c true if the given group is unknown, \c false otherwise.
-  bool getDiagnosticsInGroup(StringRef Group,
+  bool getDiagnosticsInGroup(diag::Flavor Flavor, StringRef Group,
                              SmallVectorImpl<diag::kind> &Diags) const;
 
   /// \brief Get the set of all diagnostic IDs.
-  void getAllDiagnostics(SmallVectorImpl<diag::kind> &Diags) const;
+  void getAllDiagnostics(diag::Flavor Flavor,
+                         SmallVectorImpl<diag::kind> &Diags) const;
 
-  /// \brief Get the warning option with the closest edit distance to the given
-  /// group name.
-  static StringRef getNearestWarningOption(StringRef Group);
+  /// \brief Get the diagnostic option with the closest edit distance to the
+  /// given group name.
+  static StringRef getNearestOption(diag::Flavor Flavor, StringRef Group);
 
 private:
   /// \brief Classify the specified diagnostic ID into a Level, consumable by

include/clang/Basic/DiagnosticOptions.h

@@ -58,6 +58,10 @@ public:
   /// prefixes removed.
   std::vector<std::string> Warnings;
 
+  /// The list of -R... options used to alter the diagnostic mappings, with the
+  /// prefixes removed.
+  std::vector<std::string> Remarks;
+
 public:
   // Define accessors/mutators for diagnostic options of enumeration type.
 #define DIAGOPT(Name, Bits, Default)

include/clang/Driver/Options.td

@@ -64,7 +64,9 @@ def M_Group               : OptionGroup<"<M group>">, Group<CompileOnly_Group>;
 def T_Group               : OptionGroup<"<T group>">;
 def O_Group               : OptionGroup<"<O group>">, Group<CompileOnly_Group>;
 def R_Group               : OptionGroup<"<R group>">, Group<CompileOnly_Group>;
+def R_value_Group         : OptionGroup<"<R (with value) group>">, Group<R_Group>;
 def W_Group               : OptionGroup<"<W group>">, Group<CompileOnly_Group>;
+def W_value_Group         : OptionGroup<"<W (with value) group>">, Group<W_Group>;
 def d_Group               : OptionGroup<"<d group>">;
 def f_Group               : OptionGroup<"<f group>">, Group<CompileOnly_Group>;
 def f_clang_Group         : OptionGroup<"<f (clang-only) group>">, Group<CompileOnly_Group>;
@@ -261,17 +263,19 @@ def Qn : Flag<["-"], "Qn">;
 def Qunused_arguments : Flag<["-"], "Qunused-arguments">, Flags<[DriverOption, CoreOption]>,
   HelpText<"Don't emit warning for unused driver arguments">;
 def Q : Flag<["-"], "Q">;
-def Rpass_EQ : Joined<["-"], "Rpass=">, Group<R_Group>, Flags<[CC1Option]>,
+def Rpass_EQ : Joined<["-"], "Rpass=">, Group<R_value_Group>, Flags<[CC1Option]>,
   HelpText<"Report transformations performed by optimization passes whose "
            "name matches the given POSIX regular expression">;
-def Rpass_missed_EQ : Joined<["-"], "Rpass-missed=">, Group<R_Group>,
+def Rpass_missed_EQ : Joined<["-"], "Rpass-missed=">, Group<R_value_Group>,
   Flags<[CC1Option]>,
   HelpText<"Report missed transformations by optimization passes whose "
            "name matches the given POSIX regular expression">;
-def Rpass_analysis_EQ : Joined<["-"], "Rpass-analysis=">, Group<R_Group>,
+def Rpass_analysis_EQ : Joined<["-"], "Rpass-analysis=">, Group<R_value_Group>,
   Flags<[CC1Option]>,
   HelpText<"Report transformation analysis from optimization passes whose "
            "name matches the given POSIX regular expression">;
+def R_Joined : Joined<["-"], "R">, Group<R_Group>, Flags<[CC1Option, CoreOption]>,
+  MetaVarName<"<remark>">, HelpText<"Enable the specified remark">;
 def S : Flag<["-"], "S">, Flags<[DriverOption,CC1Option]>, Group<Action_Group>,
   HelpText<"Only run preprocess and compilation steps">;
 def Tbss : JoinedOrSeparate<["-"], "Tbss">, Group<T_Group>;
@@ -290,6 +294,7 @@ def Wextra : Flag<["-"], "Wextra">, Group<W_Group>, Flags<[CC1Option]>;
 def Wl_COMMA : CommaJoined<["-"], "Wl,">, Flags<[LinkerInput, RenderAsInput]>,
   HelpText<"Pass the comma separated arguments in <arg> to the linker">,
   MetaVarName<"<arg>">;
+// FIXME: This is broken; these should not be Joined arguments.
 def Wno_nonportable_cfstrings : Joined<["-"], "Wno-nonportable-cfstrings">, Group<W_Group>,
   Flags<[CC1Option]>;
 def Wnonportable_cfstrings : Joined<["-"], "Wnonportable-cfstrings">, Group<W_Group>,

lib/Basic/Diagnostic.cpp

@@ -228,11 +228,12 @@ void DiagnosticsEngine::setSeverity(diag::kind Diag, diag::Severity Map,
                                                FullSourceLoc(Loc, *SourceMgr)));
 }
 
-bool DiagnosticsEngine::setSeverityForGroup(StringRef Group, diag::Severity Map,
+bool DiagnosticsEngine::setSeverityForGroup(diag::Flavor Flavor,
+                                            StringRef Group, diag::Severity Map,
                                             SourceLocation Loc) {
   // Get the diagnostics in this group.
   SmallVector<diag::kind, 8> GroupDiags;
-  if (Diags->getDiagnosticsInGroup(Group, GroupDiags))
+  if (Diags->getDiagnosticsInGroup(Flavor, Group, GroupDiags))
     return true;
 
   // Set the mapping.
@@ -247,14 +248,16 @@ bool DiagnosticsEngine::setDiagnosticGroupWarningAsError(StringRef Group,
   // If we are enabling this feature, just set the diagnostic mappings to map to
   // errors.
   if (Enabled)
-    return setSeverityForGroup(Group, diag::Severity::Error);
+    return setSeverityForGroup(diag::Flavor::WarningOrError, Group,
+                               diag::Severity::Error);
 
   // Otherwise, we want to set the diagnostic mapping's "no Werror" bit, and
   // potentially downgrade anything already mapped to be a warning.
 
   // Get the diagnostics in this group.
   SmallVector<diag::kind, 8> GroupDiags;
-  if (Diags->getDiagnosticsInGroup(Group, GroupDiags))
+  if (Diags->getDiagnosticsInGroup(diag::Flavor::WarningOrError, Group,
+                                   GroupDiags))
     return true;
 
   // Perform the mapping change.
@@ -276,14 +279,16 @@ bool DiagnosticsEngine::setDiagnosticGroupErrorAsFatal(StringRef Group,
   // If we are enabling this feature, just set the diagnostic mappings to map to
   // fatal errors.
   if (Enabled)
-    return setSeverityForGroup(Group, diag::Severity::Fatal);
+    return setSeverityForGroup(diag::Flavor::WarningOrError, Group,
+                               diag::Severity::Fatal);
 
   // Otherwise, we want to set the diagnostic mapping's "no Werror" bit, and
   // potentially downgrade anything already mapped to be an error.
 
   // Get the diagnostics in this group.
   SmallVector<diag::kind, 8> GroupDiags;
-  if (Diags->getDiagnosticsInGroup(Group, GroupDiags))
+  if (Diags->getDiagnosticsInGroup(diag::Flavor::WarningOrError, Group,
+                                   GroupDiags))
     return true;
 
   // Perform the mapping change.
@@ -299,11 +304,12 @@ bool DiagnosticsEngine::setDiagnosticGroupErrorAsFatal(StringRef Group,
   return false;
 }
 
-void DiagnosticsEngine::setSeverityForAll(diag::Severity Map,
+void DiagnosticsEngine::setSeverityForAll(diag::Flavor Flavor,
+                                          diag::Severity Map,
                                           SourceLocation Loc) {
   // Get all the diagnostics.
   SmallVector<diag::kind, 64> AllDiags;
-  Diags->getAllDiagnostics(AllDiags);
+  Diags->getAllDiagnostics(Flavor, AllDiags);
 
   // Set the mapping.
   for (unsigned i = 0, e = AllDiags.size(); i != e; ++i)

lib/Basic/DiagnosticIDs.cpp

@@ -58,6 +58,11 @@ struct StaticDiagInfoRec {
     return StringRef(DescriptionStr, DescriptionLen);
   }
 
+  diag::Flavor getFlavor() const {
+    return Class == CLASS_REMARK ? diag::Flavor::Remark
+                                 : diag::Flavor::WarningOrError;
+  }
+
   bool operator<(const StaticDiagInfoRec &RHS) const {
     return DiagID < RHS.DiagID;
   }
@@ -522,40 +527,57 @@ StringRef DiagnosticIDs::getWarningOptionForDiag(unsigned DiagID) {
   return StringRef();
 }
 
-static void getDiagnosticsInGroup(const WarningOption *Group,
+/// Return \c true if any diagnostics were found in this group, even if they
+/// were filtered out due to having the wrong flavor.
+static bool getDiagnosticsInGroup(diag::Flavor Flavor,
+                                  const WarningOption *Group,
                                   SmallVectorImpl<diag::kind> &Diags) {
+  // An empty group is considered to be a warning group: we have empty groups
+  // for GCC compatibility, and GCC does not have remarks.
+  if (!Group->Members && !Group->SubGroups)
+    return Flavor == diag::Flavor::Remark ? true : false;
+
+  bool NotFound = true;
+
   // Add the members of the option diagnostic set.
   const int16_t *Member = DiagArrays + Group->Members;
-  for (; *Member != -1; ++Member)
-    Diags.push_back(*Member);
+  for (; *Member != -1; ++Member) {
+    if (GetDiagInfo(*Member)->getFlavor() == Flavor) {
+      NotFound = false;
+      Diags.push_back(*Member);
+    }
+  }
 
   // Add the members of the subgroups.
   const int16_t *SubGroups = DiagSubGroups + Group->SubGroups;
   for (; *SubGroups != (int16_t)-1; ++SubGroups)
-    getDiagnosticsInGroup(&OptionTable[(short)*SubGroups], Diags);
+    NotFound &= getDiagnosticsInGroup(Flavor, &OptionTable[(short)*SubGroups],
+                                      Diags);
+
+  return NotFound;
 }
 
-bool DiagnosticIDs::getDiagnosticsInGroup(
-    StringRef Group,
-    SmallVectorImpl<diag::kind> &Diags) const {
-  const WarningOption *Found =
-  std::lower_bound(OptionTable, OptionTable + OptionTableSize, Group,
-                   WarningOptionCompare);
+bool
+DiagnosticIDs::getDiagnosticsInGroup(diag::Flavor Flavor, StringRef Group,
+                                     SmallVectorImpl<diag::kind> &Diags) const {
+  const WarningOption *Found = std::lower_bound(
+      OptionTable, OptionTable + OptionTableSize, Group, WarningOptionCompare);
   if (Found == OptionTable + OptionTableSize ||
       Found->getName() != Group)
     return true; // Option not found.
 
-  ::getDiagnosticsInGroup(Found, Diags);
-  return false;
+  return ::getDiagnosticsInGroup(Flavor, Found, Diags);
 }
 
-void DiagnosticIDs::getAllDiagnostics(
-                               SmallVectorImpl<diag::kind> &Diags) const {
+void DiagnosticIDs::getAllDiagnostics(diag::Flavor Flavor,
+                                     SmallVectorImpl<diag::kind> &Diags) const {
   for (unsigned i = 0; i != StaticDiagInfoSize; ++i)
-    Diags.push_back(StaticDiagInfo[i].DiagID);
+    if (StaticDiagInfo[i].getFlavor() == Flavor)
+      Diags.push_back(StaticDiagInfo[i].DiagID);
 }
 
-StringRef DiagnosticIDs::getNearestWarningOption(StringRef Group) {
+StringRef DiagnosticIDs::getNearestOption(diag::Flavor Flavor,
+                                          StringRef Group) {
   StringRef Best;
   unsigned BestDistance = Group.size() + 1; // Sanity threshold.
   for (const WarningOption *i = OptionTable, *e = OptionTable + OptionTableSize;
@@ -565,6 +587,14 @@ StringRef DiagnosticIDs::getNearestWarningOption(StringRef Group) {
       continue;
 
     unsigned Distance = i->getName().edit_distance(Group, true, BestDistance);
+    if (Distance > BestDistance)
+      continue;
+
+    // Don't suggest groups that are not of this kind.
+    llvm::SmallVector<diag::kind, 8> Diags;
+    if (::getDiagnosticsInGroup(Flavor, i, Diags) || Diags.empty())
+      continue;
+
     if (Distance == BestDistance) {
       // Two matches with the same distance, don't prefer one over the other.
       Best = "";

lib/Basic/Warnings.cpp

@@ -20,6 +20,8 @@
 // Given a warning option 'foo', the following are valid:
 //    -Wfoo, -Wno-foo, -Werror=foo, -Wfatal-errors=foo
 //
+// Remark options are also handled here, analogously, except that they are much
+// simpler because a remark can't be promoted to an error.
 #include "clang/Basic/AllDiagnostics.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticOptions.h"
@@ -31,17 +33,12 @@ using namespace clang;
 // EmitUnknownDiagWarning - Emit a warning and typo hint for unknown warning
 // opts
 static void EmitUnknownDiagWarning(DiagnosticsEngine &Diags,
-                                  StringRef Prefix, StringRef Opt,
-                                  bool isPositive) {
-  StringRef Suggestion = DiagnosticIDs::getNearestWarningOption(Opt);
-  if (!Suggestion.empty())
-    Diags.Report(isPositive? diag::warn_unknown_warning_option_suggest :
-                             diag::warn_unknown_negative_warning_option_suggest)
-      << (Prefix.str() += Opt) << (Prefix.str() += Suggestion);
-  else
-    Diags.Report(isPositive? diag::warn_unknown_warning_option :
-                             diag::warn_unknown_negative_warning_option)
-      << (Prefix.str() += Opt);
+                                   diag::Flavor Flavor, StringRef Prefix,
+                                   StringRef Opt) {
+  StringRef Suggestion = DiagnosticIDs::getNearestOption(Flavor, Opt);
+  Diags.Report(diag::warn_unknown_diag_option)
+    << (Flavor == diag::Flavor::WarningOrError ? 0 : 1) << (Prefix.str() += Opt)
+    << !Suggestion.empty() << (Prefix.str() += Suggestion);
 }
 
 void clang::ProcessWarningOptions(DiagnosticsEngine &Diags,
@@ -89,6 +86,7 @@ void clang::ProcessWarningOptions(DiagnosticsEngine &Diags,
       break;
 
     for (unsigned i = 0, e = Opts.Warnings.size(); i != e; ++i) {
+      const auto Flavor = diag::Flavor::WarningOrError;
       StringRef Opt = Opts.Warnings[i];
       StringRef OrigOpt = Opts.Warnings[i];
 
@@ -125,7 +123,7 @@ void clang::ProcessWarningOptions(DiagnosticsEngine &Diags,
             Diags.setEnableAllWarnings(true);
           } else {
             Diags.setEnableAllWarnings(false);
-            Diags.setSeverityForAll(diag::Severity::Ignored);
+            Diags.setSeverityForAll(Flavor, diag::Severity::Ignored);
           }
         }
         continue;
@@ -154,8 +152,8 @@ void clang::ProcessWarningOptions(DiagnosticsEngine &Diags,
         if (SetDiagnostic) {
           // Set the warning as error flag for this specifier.
           Diags.setDiagnosticGroupWarningAsError(Specifier, isPositive);
-        } else if (DiagIDs->getDiagnosticsInGroup(Specifier, _Diags)) {
-          EmitUnknownDiagWarning(Diags, "-Werror=", Specifier, isPositive);
+        } else if (DiagIDs->getDiagnosticsInGroup(Flavor, Specifier, _Diags)) {
+          EmitUnknownDiagWarning(Diags, Flavor, "-Werror=", Specifier);
         }
         continue;
       }
@@ -182,19 +180,50 @@ void clang::ProcessWarningOptions(DiagnosticsEngine &Diags,
         if (SetDiagnostic) {
           // Set the error as fatal flag for this specifier.
           Diags.setDiagnosticGroupErrorAsFatal(Specifier, isPositive);
-        } else if (DiagIDs->getDiagnosticsInGroup(Specifier, _Diags)) {
-          EmitUnknownDiagWarning(Diags, "-Wfatal-errors=", Specifier,
-                                 isPositive);
+        } else if (DiagIDs->getDiagnosticsInGroup(Flavor, Specifier, _Diags)) {
+          EmitUnknownDiagWarning(Diags, Flavor, "-Wfatal-errors=", Specifier);
         }
         continue;
       }
       
       if (Report) {
-        if (DiagIDs->getDiagnosticsInGroup(Opt, _Diags))
-          EmitUnknownDiagWarning(Diags, isPositive ? "-W" : "-Wno-", Opt,
-                                 isPositive);
+        if (DiagIDs->getDiagnosticsInGroup(Flavor, Opt, _Diags))
+          EmitUnknownDiagWarning(Diags, Flavor, isPositive ? "-W" : "-Wno-",
+                                 Opt);
+      } else {
+        Diags.setSeverityForGroup(Flavor, Opt, Mapping);
+      }
+    }
+
+    for (unsigned i = 0, e = Opts.Remarks.size(); i != e; ++i) {
+      StringRef Opt = Opts.Remarks[i];
+      const auto Flavor = diag::Flavor::Remark;
+
+      // Check to see if this warning starts with "no-", if so, this is a
+      // negative form of the option.
+      bool IsPositive = !Opt.startswith("no-");
+      if (!IsPositive) Opt = Opt.substr(3);
+
+      auto Severity = IsPositive ? diag::Severity::Remark
+                                 : diag::Severity::Ignored;
+
+      // -Reverything sets the state of all remarks. Note that all remarks are
+      // in remark groups, so we don't need a separate 'all remarks enabled'
+      // flag.
+      if (Opt == "everything") {
+        if (SetDiagnostic)
+          Diags.setSeverityForAll(Flavor, Severity);
+        continue;
+      }
+
+      if (Report) {
+        if (DiagIDs->getDiagnosticsInGroup(Flavor, Opt, _Diags))
+          EmitUnknownDiagWarning(Diags, Flavor, IsPositive ? "-R" : "-Rno-",
+                                 Opt);
       } else {
-        Diags.setSeverityForGroup(Opt, Mapping);
+        Diags.setSeverityForGroup(Flavor, Opt,
+                                  IsPositive ? diag::Severity::Remark
+                                             : diag::Severity::Ignored);
       }
     }
   }

lib/Driver/Tools.cpp

@@ -3374,6 +3374,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
   // precompiling.
   Args.ClaimAllArgs(options::OPT_flto);
 
+  Args.AddAllArgs(CmdArgs, options::OPT_R_Group);
   Args.AddAllArgs(CmdArgs, options::OPT_W_Group);
   if (Args.hasFlag(options::OPT_pedantic, options::OPT_no_pedantic, false))
     CmdArgs.push_back("-pedantic");
@@ -3735,15 +3736,6 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
       A->render(Args, CmdArgs);
   }
 
-  if (Arg *A = Args.getLastArg(options::OPT_Rpass_EQ))
-    A->render(Args, CmdArgs);
-
-  if (Arg *A = Args.getLastArg(options::OPT_Rpass_missed_EQ))
-    A->render(Args, CmdArgs);
-
-  if (Arg *A = Args.getLastArg(options::OPT_Rpass_analysis_EQ))
-    A->render(Args, CmdArgs);
-
   if (Args.hasArg(options::OPT_mkernel)) {
     if (!Args.hasArg(options::OPT_fapple_kext) && types::isCXX(InputType))
       CmdArgs.push_back("-fapple-kext");

lib/Frontend/CompilerInvocation.cpp

@@ -111,25 +111,21 @@ static unsigned getOptimizationLevelSize(ArgList &Args) {
   return 0;
 }
 
-static void addWarningArgs(ArgList &Args, std::vector<std::string> &Warnings) {
-  for (arg_iterator I = Args.filtered_begin(OPT_W_Group),
-         E = Args.filtered_end(); I != E; ++I) {
-    Arg *A = *I;
-    // If the argument is a pure flag, add its name (minus the "W" at the beginning)
-    // to the warning list. Else, add its value (for the OPT_W case).
+static void addDiagnosticArgs(ArgList &Args, OptSpecifier Group,
+                              OptSpecifier GroupWithValue,
+                              std::vector<std::string> &Diagnostics) {
+  for (Arg *A : Args.filtered(Group)) {
     if (A->getOption().getKind() == Option::FlagClass) {
-      Warnings.push_back(A->getOption().getName().substr(1));
+      // The argument is a pure flag (such as OPT_Wall or OPT_Wdeprecated). Add
+      // its name (minus the "W" or "R" at the beginning) to the warning list.
+      Diagnostics.push_back(A->getOption().getName().drop_front(1));
+    } else if (A->getOption().matches(GroupWithValue)) {
+      // This is -Wfoo= or -Rfoo=, where foo is the name of the diagnostic group.
+      Diagnostics.push_back(A->getOption().getName().drop_front(1).rtrim("=-"));
     } else {
-      for (unsigned Idx = 0, End = A->getNumValues();
-           Idx < End; ++Idx) {
-        StringRef V = A->getValue(Idx);
-        // "-Wl," and such are not warning options.
-        // FIXME: Should be handled by putting these in separate flags.
-        if (V.startswith("l,") || V.startswith("a,") || V.startswith("p,"))
-          continue;
-
-        Warnings.push_back(V);
-      }
+      // Otherwise, add its value (for OPT_W_Joined and similar).
+      for (const char *Arg : A->getValues())
+        Diagnostics.push_back(Arg);
     }
   }
 }
@@ -700,7 +696,8 @@ bool clang::ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
       << Opts.TabStop << DiagnosticOptions::DefaultTabStop;
   }
   Opts.MessageLength = getLastArgIntValue(Args, OPT_fmessage_length, 0, Diags);
-  addWarningArgs(Args, Opts.Warnings);
+  addDiagnosticArgs(Args, OPT_W_Group, OPT_W_value_Group, Opts.Warnings);
+  addDiagnosticArgs(Args, OPT_R_Group, OPT_R_value_Group, Opts.Remarks);
 
   return Success;
 }

lib/Frontend/FrontendActions.cpp

@@ -470,10 +470,11 @@ namespace {
       Out.indent(4) << #Name << ": " << DiagOpts->Name << "\n";
 #include "clang/Basic/DiagnosticOptions.def"
 
-      Out.indent(4) << "Warning options:\n";
-      for (const std::string &Warning : DiagOpts->Warnings) {
+      Out.indent(4) << "Diagnostic flags:\n";
+      for (const std::string &Warning : DiagOpts->Warnings)
         Out.indent(6) << "-W" << Warning << "\n";
-      }
+      for (const std::string &Remark : DiagOpts->Remarks)
+        Out.indent(6) << "-R" << Remark << "\n";
 
       return false;
     }

lib/Lex/PPMacroExpansion.cpp

@@ -1446,6 +1446,8 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) {
         break;
       }
 
+      // FIXME: Should we accept "-R..." flags here, or should that be handled
+      // by a separate __has_remark?
       if (WarningName.size() < 3 || WarningName[0] != '-' ||
           WarningName[1] != 'W') {
         Diag(StrStartLoc, diag::warn_has_warning_invalid_option);
@@ -1458,7 +1460,8 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) {
       // worth special casing.
       SmallVector<diag::kind, 10> Diags;
       Value = !getDiagnostics().getDiagnosticIDs()->
-        getDiagnosticsInGroup(WarningName.substr(2), Diags);
+        getDiagnosticsInGroup(diag::Flavor::WarningOrError,
+                              WarningName.substr(2), Diags);
     } while (false);
 
     OS << (int)Value;

lib/Lex/Pragma.cpp

@@ -993,13 +993,15 @@ public:
     }
 
     if (WarningName.size() < 3 || WarningName[0] != '-' ||
-        WarningName[1] != 'W') {
+        (WarningName[1] != 'W' && WarningName[1] != 'R')) {
       PP.Diag(StringLoc, diag::warn_pragma_diagnostic_invalid_option);
       return;
     }
 
-    if (PP.getDiagnostics().setSeverityForGroup(WarningName.substr(2), SV,
-                                                DiagLoc))
+    if (PP.getDiagnostics().setSeverityForGroup(
+            WarningName[1] == 'W' ? diag::Flavor::WarningOrError
+                                  : diag::Flavor::Remark,
+            WarningName.substr(2), SV, DiagLoc))
       PP.Diag(StringLoc, diag::warn_pragma_diagnostic_unknown_warning)
         << WarningName;
     else if (Callbacks)

lib/Serialization/ASTReader.cpp

@@ -4639,9 +4639,10 @@ bool ASTReader::ParseDiagnosticOptions(const RecordData &Record, bool Complain,
   DiagOpts->set##Name(static_cast<Type>(Record[Idx++]));
 #include "clang/Basic/DiagnosticOptions.def"
 
-  for (unsigned N = Record[Idx++]; N; --N) {
+  for (unsigned N = Record[Idx++]; N; --N)
     DiagOpts->Warnings.push_back(ReadString(Record, Idx));
-  }
+  for (unsigned N = Record[Idx++]; N; --N)
+    DiagOpts->Remarks.push_back(ReadString(Record, Idx));
 
   return Listener.ReadDiagnosticOptions(DiagOpts, Complain);
 }

lib/Serialization/ASTWriter.cpp

@@ -1226,6 +1226,9 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, ASTContext &Context,
   Record.push_back(DiagOpts.Warnings.size());
   for (unsigned I = 0, N = DiagOpts.Warnings.size(); I != N; ++I)
     AddString(DiagOpts.Warnings[I], Record);
+  Record.push_back(DiagOpts.Remarks.size());
+  for (unsigned I = 0, N = DiagOpts.Remarks.size(); I != N; ++I)
+    AddString(DiagOpts.Remarks[I], Record);
   // Note: we don't serialize the log or serialization file names, because they
   // are generally transient files and will almost always be overridden.
   Stream.EmitRecord(DIAGNOSTIC_OPTIONS, Record);

test/Frontend/optimization-remark.c

@@ -6,6 +6,21 @@
 // RUN: %clang_cc1 %s -Rpass=inline -Rpass-analysis=inline -Rpass-missed=inline -O0 -emit-llvm-only -verify
 // RUN: %clang_cc1 %s -Rpass=inline -Rpass-analysis=inline -Rpass-missed=inline -O0 -emit-llvm-only -gline-tables-only -verify
 // RUN: %clang_cc1 %s -Rpass=inline -emit-llvm -o - 2>/dev/null | FileCheck %s
+//
+// Check that we can override -Rpass= with -Rno-pass.
+// RUN: %clang_cc1 %s -Rpass=inline -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+// RUN: %clang_cc1 %s -Rpass=inline -Rno-pass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
+// RUN: %clang_cc1 %s -Rpass=inline -Rno-everything -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
+// RUN: %clang_cc1 %s -Rpass=inline -Rno-everything -Reverything -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+//
+// FIXME: -Reverything should imply -Rpass=.*.
+// RUN: %clang_cc1 %s -Reverything -emit-llvm -o - 2>/dev/null | FileCheck %s --check-prefix=CHECK-NO-REMARKS
+//
+// FIXME: -Rpass should either imply -Rpass=.* or should be rejected.
+// RUN: %clang_cc1 %s -Rpass -emit-llvm -o - 2>/dev/null | FileCheck %s --check-prefix=CHECK-NO-REMARKS
+
+// CHECK-REMARKS: remark:
+// CHECK-NO-REMARKS-NOT: remark:
 
 // -Rpass should produce source location annotations, exclusively (just
 // like -gmlt).

test/Frontend/warning-options.cpp

@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 -Wmonkey -Wno-monkey -Wno-unused-command-line-arguments \
-// RUN:        -Wno-unused-command-line-argument %s 2>&1 | FileCheck %s
+// RUN:        -Wno-unused-command-line-argument -Wmodule-build -Rmodule-built %s 2>&1 | FileCheck %s
 // CHECK: unknown warning option '-Wmonkey'
 // CHECK: unknown warning option '-Wno-monkey'
 // CHECK: unknown warning option '-Wno-unused-command-line-arguments'; did you mean '-Wno-unused-command-line-argument'?
+// CHECK: unknown warning option '-Wmodule-build'; did you mean '-Wmodule-conflict'?
+// CHECK: unknown remark option '-Rmodule-built'; did you mean '-Rmodule-build'?

test/Modules/Rmodule-build.m

@@ -7,7 +7,7 @@
 // RUN: echo 'module B { header "B.h" }' >> %t/module.modulemap
 
 // RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fsyntax-only %s -verify \
-// RUN:            -I %t -Wmodule-build
+// RUN:            -I %t -Rmodule-build
 
 @import A; // expected-remark{{building module 'A' as}}
 @import B; // expected-remark{{building module 'B' as}}
@@ -16,7 +16,19 @@
 
 // RUN: echo ' ' >> %t/B.h
 // RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fsyntax-only %s -I %t \
-// RUN:            -Wmodule-build 2>&1 | FileCheck %s
+// RUN:            -Rmodule-build 2>&1 | FileCheck %s
+
+// RUN: echo ' ' >> %t/B.h
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fsyntax-only %s -I %t \
+// RUN:            -Reverything 2>&1 | FileCheck %s
+
+// RUN: echo ' ' >> %t/B.h
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fsyntax-only %s -I %t \
+// RUN:            2>&1 | count 0
+
+// RUN: echo ' ' >> %t/B.h
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fsyntax-only %s -I %t \
+// RUN:            -Rmodule-build -Rno-everything 2>&1 | count 0
 
 // CHECK-NOT: building module 'A'
 // CHECK: building module 'B'

test/Modules/module_file_info.m

@@ -22,7 +22,7 @@
 
 // CHECK: Diagnostic options:
 // CHECK:   IgnoreWarnings: Yes
-// CHECK:   Warning options:
+// CHECK:   Diagnostic flags:
 // CHECK:     -Wunused
 
 // CHECK: Header search options:

test/Modules/no-stale-modtime.m

@@ -13,18 +13,18 @@
 // RUN: echo 'module r { header "r.h" } module t { header "t.h" }' >> %t/module.map
 
 // RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fdisable-module-hash \
-// RUN:     -I %t -fsyntax-only %s -Wmodule-build 2>&1 \
+// RUN:     -I %t -fsyntax-only %s -Rmodule-build 2>&1 \
 // RUN: | FileCheck -check-prefix=REBUILD-ALL %s
 // RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fdisable-module-hash \
-// RUN:     -I %t -fsyntax-only %s -Wmodule-build -verify
+// RUN:     -I %t -fsyntax-only %s -Rmodule-build -verify
 
 // Add an identifier to ensure everything depending on t is out of date
 // RUN: echo 'extern int a;' >> %t/t.h
 // RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fdisable-module-hash \
-// RUN:     -I %t -fsyntax-only %s -Wmodule-build 2>&1 \
+// RUN:     -I %t -fsyntax-only %s -Rmodule-build 2>&1 \
 // RUN: | FileCheck -check-prefix=REBUILD-ALL %s
 // RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fdisable-module-hash \
-// RUN:     -I %t -fsyntax-only %s -Wmodule-build -verify
+// RUN:     -I %t -fsyntax-only %s -Rmodule-build -verify
 
 // REBUILD-ALL: building module 'b'
 // REBUILD-ALL: building module 'l'