================
Comment at: loop-convert/LoopActions.cpp:836
@@ +835,3 @@
+
+ ConfidenceLevel = std::min(ConfidenceLevel, Finder.getConfidenceLevel());
+ // We require that a single array/container be indexed into by LoopVar.
----------------
You seem to have a lot of these bare calls to `std::min()` to compute the
`ConfidenceLevel` all over the place. Could you encapsulate this operation and
give it a descriptive name?
================
Comment at: loop-convert/LoopMatchers.cpp:17
@@ +16,3 @@
+
+// FIXME: How best to document complicated matcher expressions? They're fairly
+// self-documenting...but there may be some unintuitive parts.
----------------
I think that most of these "matcher factory" functions would benefit from a
documentation comment containing:
* A list of bound variable names (or whatever you call the things captured by
`id()` and `.bind()`)
* A chunk of example code with the part that would be matched called out in
some way (maybe surround it with `@@@`, `|||` or some other marker which will
clearly stand out)
Also, for these in particular, I would like to see a brief explanation of how
they fit into the larger for-loop conversion process.
================
Comment at: loop-convert/LoopMatchers.cpp:21
@@ +20,3 @@
+StatementMatcher makeArrayLoopMatcher() {
+ static StatementMatcher LHSMatcher =
+ expression(ignoringImpCasts(declarationReference(to(
----------------
Why are these static? (are they generating static constructors... ew...).
================
Comment at: test/loop-convert/Inputs/structures.h:5
@@ +4,3 @@
+extern "C" {
+extern int printf(const char *restrict, ...);
+}
----------------
I don't think it's legal to use `restrict` in C++, even in `extern "C"`.
http://llvm-reviews.chandlerc.com/D19
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
