[PATCH] D55552: [Sema] Better static assert diagnostics for expressions involving temporaries.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

LGTM. Tiny style suggestions, which I won't mind if you ignore.



================
Comment at: include/clang/AST/Type.h:995
   void getAsStringInternal(std::string &Str,
-                           const PrintingPolicy &Policy) const {
-    return getAsStringInternal(split(), Str, Policy);
-  }
+                           const PrintingPolicy &Policy) const;
 
----------------
Nit: could this function have been left in-line, and just changed `split()` to `splitAccordingToPolicy(this, Policy)`?
Even simpler, could `splitAccordingToPolicy` be made a member function of `QualType`, so that most of these diffs could be simply `s/split()/splitAccordingToPolicy(Policy)/` without introducing any new temporary variables or anything?  I.e.
```
    void getAsStringInternal(std::string &Str,
                              const PrintingPolicy &Policy) const {
        return getAsStringInternal(splitAccordingToPolicy(Policy), Str, Policy);
    }
```
But if that would make the code harder to read instead of easier, then don't mind me.


================
Comment at: include/clang/AST/Type.h:998
   static void getAsStringInternal(SplitQualType split, std::string &out,
-                                  const PrintingPolicy &policy) {
-    return getAsStringInternal(split.Ty, split.Quals, out, policy);
-  }
+                                  const PrintingPolicy &policy);
 
----------------
Nit: this function's body hasn't changed, so personally I would have left it alone (not out-of-lined it).


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55552/new/

https://reviews.llvm.org/D55552