[PATCH] D55552: [Sema] Better static assert diagnostics for expressions involving temporaries.
Arthur O'Dwyer via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Dec 19 09:25:15 PST 2018
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
More information about the cfe-commits
mailing list