rsmith added a comment.
In D123345#3452496 <https://reviews.llvm.org/D123345#3452496>, @aaron.ballman
wrote:
> Do you have ideas on how we can improve the debugging checkpoint behavior (if
> at all)?
I think we just live with it, like we do for other builtin functions. (There
might be things we can do by emitting inlining info into the debug info. If we
do that, we should presumably do it for all builtin lib functions.)
================
Comment at: clang/test/SemaCXX/builtin-std-move.cpp:12
+
+ template<typename T> CONSTEXPR T &&move(T &x) {
+ static_assert(T::moveable, "instantiated move"); // expected-error {{no
member named 'moveable' in 'B'}}
----------------
aaron.ballman wrote:
> rsmith wrote:
> > aaron.ballman wrote:
> > > Formatting looks like it's gone wonky in this file.
> > What are you referring to? The lines longer than 80 columns, or something
> > else?
> >
> > FWIW, long lines are common in our `test/` files. Substantially more than
> > half of them go over 80 columns:
> > ```
> > $ rgrep '.\{81\}' clang/test --files-with-match | wc -l
> > 12481
> > $ rgrep '.\{81\}' clang/test --files-without-match | wc -l
> > 7194
> > ```
> Oh, the line length doesn't bother me (we smash all over that in tests, as
> you've seen). It was more the extra indentation within a namespace -- we
> don't usually indent like that and it caught me off guard. It's not a major
> deal though (the same indentation happens in
> clang/test/SemaCXX/builtin-std-move-nobuiltin.cpp).
Gotcha.
This is just what my editor / fingers happened to do, but now I'm consciously
thinking about it, I see a bit of nuance here: I think it makes sense to not
indent a namespace that spans a whole file, because the indent doesn't add
anything for the reader and loses two columns of useful screen real estate. But
when the namespace is covering a relatively small scope it seems useful to me
to indent it to contrast the contents of the namespace against the surrounding
code that's outside the namespace, in the same way it makes sense to indent a
class definition to contrast the class scope against surrounding code.
It looks like indenting namespace bodies is pretty common in the clang test
suite: of the 1917 files that contain a line matching /^namespace.*$/, 1171 of
those files have a next line that is indented, which I think makes sense
because namespaces in tests will tend to be short, and intended to semantically
separate a portion of the test from the rest of the file. (By contrast, only
~10% of the clang sources and headers have indented namespaces, and from a
quick sampling it looks like they basically follow the same pattern: indented
namespaces are mostly used for short scopes and unindented ones are mostly used
for longer / whole-file scopes.)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D123345/new/
https://reviews.llvm.org/D123345
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
