[PATCH] D19876: Add an AST matcher for string-literal length

Etienne Bergeron via cfe-commits cfe-commits at lists.llvm.org
Fri May 6 08:18:35 PDT 2016


etienneb added inline comments.

================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:1578
@@ +1577,3 @@
+/// \endcode
+AST_MATCHER_P(StringLiteral, lengthIs, unsigned, N) {
+  return Node.getLength() == N;
----------------
etienneb wrote:
> aaron.ballman wrote:
> > etienneb wrote:
> > > etienneb wrote:
> > > > aaron.ballman wrote:
> > > > > Perhaps we can adjust the `hasSize()` matcher instead? It currently works with ConstantArrayType, but it seems reasonable for it to also work with StringLiteral.
> > > > I didn't like the term "size" as it typically refer to the size in bytes.
> > > > Which is not the same for a wide-string.
> > > > 
> > > > Now, there is two different convention for naming matchers:
> > > >   hasLength   and  lengthIs  ?
> > > > 
> > > > Any toughs on that?
> > > > 
> > > > 
> > > Here is the matcher for hasSize
> > > ```
> > > AST_MATCHER_P(ConstantArrayType, hasSize, unsigned, N) {
> > >   return Node.getSize() == N;
> > > }
> > > ```
> > > 
> > > It's getting the getSize attribute. I believe we should stick with the name of the attribute.
> > > But, I'm not sure if we should use hasLength, or lengthIs.
> > I'm not too worried about size vs length (for instance, std::string has both). I would imagine this being implemented the same way we do other things with variance in API but not concept. See GetBodyMatcher in ASTMatchersInternal.h (and others near there) as an example.
> > 
> > I prefer hasSize because the two concepts are quite similar. For instance, a string literal's type is of a constant array type already.
> I do not have strong opinion too on the naming. I'm curious if others also has opinion on it?
> Then, I'll proceed.
>  For instance, a string literal's type

stringLiteral is matching an expression and not a type.
And, hasSize is currently applied on types.
Do we want this?


http://reviews.llvm.org/D19876





More information about the cfe-commits mailing list