[clang-tools-extra] r215799 - AvoidCStyleCastsCheck: don't warn on casts in macros

[Alexander Kornienko]

On Sat, Aug 16, 2014 at 4:15 AM, David Blaikie <dblaikie at gmail.com> wrote:

> On Fri, Aug 15, 2014 at 5:53 PM, Alexander Kornienko <alexfh at google.com>
> wrote:
> > Author: alexfh
> > Date: Fri Aug 15 19:53:20 2014
> > New Revision: 215799
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=215799&view=rev
> > Log:
> > AvoidCStyleCastsCheck: don't warn on casts in macros
>
> What's the particular justification?
>

The main reason is that some libraries provide macros containing casts in
their API (e.g. APR_ARRAY_PUSH from APR or MAP_FAILED from mmap.h which is
defined as "((void*)(-1))"). We don't want to warn on them and I doubt we
can stuff all these libraries with // NOLINTs to avoid warnings in code
using them.


>
> c-style casts, even hidden in macros, seem like they'd still be a bit
> subtle/surprising/etc.
>
> >
> > Modified:
> >     clang-tools-extra/trunk/clang-tidy/google/AvoidCStyleCastsCheck.cpp
> >     clang-tools-extra/trunk/test/clang-tidy/avoid-c-style-casts.cpp
> >
> > Modified:
> clang-tools-extra/trunk/clang-tidy/google/AvoidCStyleCastsCheck.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/AvoidCStyleCastsCheck.cpp?rev=215799&r1=215798&r2=215799&view=diff
> >
> ==============================================================================
> > --- clang-tools-extra/trunk/clang-tidy/google/AvoidCStyleCastsCheck.cpp
> (original)
> > +++ clang-tools-extra/trunk/clang-tidy/google/AvoidCStyleCastsCheck.cpp
> Fri Aug 15 19:53:20 2014
> > @@ -60,6 +60,12 @@ bool pointedTypesAreEqual(QualType Sourc
> >  void AvoidCStyleCastsCheck::check(const MatchFinder::MatchResult
> &Result) {
> >    const auto *CastExpr = Result.Nodes.getNodeAs<CStyleCastExpr>("cast");
> >
> > +  auto ParenRange =
> CharSourceRange::getTokenRange(CastExpr->getLParenLoc(),
> > +
>  CastExpr->getRParenLoc());
> > +  // Ignore casts in macros.
> > +  if (ParenRange.getBegin().isMacroID() ||
> ParenRange.getEnd().isMacroID())
> > +    return;
> > +
> >    // Casting to void is an idiomatic way to mute "unused variable" and
> similar
> >    // warnings.
> >    if (CastExpr->getTypeAsWritten()->isVoidType())
> > @@ -69,8 +75,6 @@ void AvoidCStyleCastsCheck::check(const
> >        CastExpr->getSubExprAsWritten()->getType().getCanonicalType();
> >    QualType DestType = CastExpr->getTypeAsWritten().getCanonicalType();
> >
> > -  auto ParenRange =
> CharSourceRange::getTokenRange(CastExpr->getLParenLoc(),
> > -
>  CastExpr->getRParenLoc());
> >    if (SourceType == DestType) {
> >      diag(CastExpr->getLocStart(), "Redundant cast to the same type.")
> >          << FixItHint::CreateRemoval(ParenRange);
> > @@ -84,8 +88,6 @@ void AvoidCStyleCastsCheck::check(const
> >
> >    auto ReplaceWithCast = [&](StringRef CastType) {
> >      diag_builder << ("Use " + CastType + ".").str();
> > -    if (ParenRange.getBegin().isMacroID() ||
> ParenRange.getEnd().isMacroID())
> > -      return;
> >
> >      const Expr *SubExpr =
> CastExpr->getSubExprAsWritten()->IgnoreImpCasts();
> >      std::string CastText = (CastType + "<" + DestTypeString +
> ">").str();
> >
> > Modified: clang-tools-extra/trunk/test/clang-tidy/avoid-c-style-casts.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/avoid-c-style-casts.cpp?rev=215799&r1=215798&r2=215799&view=diff
> >
> ==============================================================================
> > --- clang-tools-extra/trunk/test/clang-tidy/avoid-c-style-casts.cpp
> (original)
> > +++ clang-tools-extra/trunk/test/clang-tidy/avoid-c-style-casts.cpp Fri
> Aug 15 19:53:20 2014
> > @@ -105,9 +105,6 @@ void test_templates() {
> >  #define CAST(type, value) (type)(value)
> >  void macros(double d) {
> >    int i = CAST(int, d);
> > -  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: C-style casts are
> discouraged. Use static_cast.
> > -  // CHECK-FIXES: #define CAST(type, value) (type)(value)
> > -  // CHECK-FIXES: int i = CAST(int, d);
> >  }
> >
> >  enum E { E1 = 1 };
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140816/b9c070e0/attachment.html>