<div>One overall concern I would have is that a parser struct is quadrupling in size. Can you measure the peak memory usage impact of this change on a hefty compilation?</div><div><br></div><div>Some nits below...</div><div>
<br></div><div>Index: test/SemaCXX/return.cpp</div><div>===================================================================</div><div>--- test/SemaCXX/return.cpp<span class="Apple-tab-span" style="white-space:pre">      </span>(revision 126218)</div>
<div>+++ test/SemaCXX/return.cpp<span class="Apple-tab-span" style="white-space:pre">   </span>(working copy)</div><div>@@ -24,5 +24,14 @@</div><div> const volatile S class_cv();</div><div> </div><div> const int scalar_c(); // expected-warning{{'const' type qualifier on return type has no effect}}</div>
<div>+</div><div>+const</div><div>+char*</div><div>+const // expected-warning{{'const' type qualifier on return type has no effect}}</div><div>+f();</div><div><br></div><div>It would be good to test "T const" and "T const * const" as well for completeness...</div>
<div><br></div><div>Index: include/clang/Sema/DeclSpec.h</div><div>===================================================================</div><div>--- include/clang/Sema/DeclSpec.h<span class="Apple-tab-span" style="white-space:pre">       </span>(revision 126218)</div>
<div>+++ include/clang/Sema/DeclSpec.h<span class="Apple-tab-span" style="white-space:pre">     </span>(working copy)</div><div>@@ -825,6 +825,25 @@</div><div>   struct PointerTypeInfo : TypeInfoCommon {</div><div>     /// The type qualifiers: const/volatile/restrict.</div>
<div>     unsigned TypeQuals : 3;</div><div>+</div><div>+    /// The location of the const-qualifier, if any.</div><div>+    unsigned ConstQualLoc;</div><div>+</div><div>+    /// The location of the volatile-qualifier, if any.</div>
<div>+    unsigned VolatileQualLoc;</div><div>+</div><div>+    /// The location of the restrict-qualifier, if any.</div><div>+    unsigned RestrictQualLoc;</div><div>+</div><div>+    SourceLocation constQualLoc() const {</div>
<div>+      return SourceLocation::getFromRawEncoding(ConstQualLoc);</div><div>+    }</div><div>+    SourceLocation volatileQualLoc() const {</div><div>+      return SourceLocation::getFromRawEncoding(VolatileQualLoc);</div>
<div>+    }</div><div>+    SourceLocation restrictQualLoc() const {</div><div>+      return SourceLocation::getFromRawEncoding(RestrictQualLoc);</div><div>+    }</div><div><br></div><div>I find the getters here a bit odd. As these are only accessed on one place, I wonder if we should do the raw encoding -> SourceLocation transition there.</div>
<div><br></div><div>Perhaps most odd to me is that the members are public and doxymented, but they have non-trivial getters and those aren't doxymented. If you want getters on this struct, I'd make the members private and move the comments.</div>
<div><br></div><div>Index: lib/Sema/SemaType.cpp</div><div>===================================================================</div><div>--- lib/Sema/SemaType.cpp<span class="Apple-tab-span" style="white-space:pre">       </span>(revision 126218)</div>
<div>+++ lib/Sema/SemaType.cpp<span class="Apple-tab-span" style="white-space:pre">     </span>(working copy)</div><div>@@ -1395,6 +1395,49 @@</div><div>   return QT;</div><div> }</div><div> </div><div>+static void DiagnoseIgnoredQualifiers(unsigned Quals,</div>
<div>+                                      SourceLocation constQualLoc,</div><div>+                                      SourceLocation volatileQualLoc,</div><div>+                                      SourceLocation restrictQualLoc,</div>
<div>+                                      Sema& S) {</div><div>+  std::string QualStr;</div><div>+  unsigned NumQuals = 0;</div><div>+  SourceLocation Loc;</div><div>+</div><div>+  if (Quals & Qualifiers::Const) {</div>
<div>+    Loc = constQualLoc;</div><div>+    ++NumQuals;</div><div>+    QualStr = "const";</div><div>+  }</div><div>+  if (Quals & Qualifiers::Volatile) {</div><div>+    if (NumQuals == 0) {</div><div>+      Loc = volatileQualLoc;</div>
<div>+      QualStr = "volatile";</div><div>+    } else</div><div>+      QualStr += " volatile";</div><div><br></div><div>I think this section is moved verbatim, but while we're here, can we have {}s around this else block? Especially followed by a preincrement, I had to read in 3 times...</div>
<div><br></div><div>+    ++NumQuals;</div><div>+  }</div><div>+  if (Quals & Qualifiers::Restrict) {</div><div>+    if (NumQuals == 0) {</div><div>+      Loc = restrictQualLoc;</div><div>+      QualStr = "restrict";</div>
<div>+    } else</div><div>+      QualStr += " restrict";</div><div><br></div><div>Same here.</div><div><br></div><div>+    ++NumQuals;</div><div>+  }</div><div>+</div><div>+  assert(NumQuals > 0 && "No known qualifiers?");</div>
<div>+  Sema::SemaDiagnosticBuilder DB = S.Diag(Loc, diag::warn_qual_return_type);</div><div>+</div><div>+  DB << QualStr << NumQuals;</div><div>+  if (Quals & Qualifiers::Const)</div><div>+    DB << FixItHint::CreateRemoval(constQualLoc);</div>
<div>+  if (Quals & Qualifiers::Volatile)</div><div>+    DB << FixItHint::CreateRemoval(volatileQualLoc);</div><div>+  if (Quals & Qualifiers::Restrict)</div><div>+    DB << FixItHint::CreateRemoval(restrictQualLoc);</div>
<div><br></div><div>And I think we simplify this by having three empty FixItHint variable, and initializing them in each branch where we do the original checking. That should also avoid the need to store the diagnostic builder, and we can just stream the lot of this in directly.</div>