[PATCH] D130906: [clang] format string checking for conpile-time evaluated str literal

Timm B├Ąder via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 2 02:42:17 PDT 2022


tbaeder added inline comments.


================
Comment at: clang/lib/Sema/SemaChecking.cpp:8507
+    if (Result.Val.isLValue()) {
+      auto *LVE = Result.Val.getLValueBase().dyn_cast<const Expr *>();
+      if (LVE && LVE->getStmtClass() == Stmt::StringLiteralClass) {
----------------
inclyc wrote:
> tbaeder wrote:
> > tbaeder wrote:
> > > inclyc wrote:
> > > > tbaeder wrote:
> > > > > I think you should be able to unify the two `if` statements.
> > > > > 
> > > > > Can you not `dyn_cast_or_null<StringLiteral>(Result.Val.getLValueBase())` here instead of casting to `Expr*` and checking the `StmtClass`?
> > > > `LValueBase` seems to be a wrapper of a pointer that has its own dyn_cast method.
> > > > 
> > > > I have changed code here like this, but it cannot compile
> > > > ```
> > > > auto *MaybeStringLiteral =
> > > >         dyn_cast_or_null<StringLiteral *>(Result.Val.getLValueBase());
> > > >     if (MaybeStringLiteral) {
> > > >       return checkFormatStringExpr(S, MaybeStringLiteral, Args, APK, format_idx,
> > > >                                    firstDataArg, Type, CallType,
> > > >                                    /*InFunctionCall*/ false, CheckedVarArgs,
> > > >                                    UncoveredArg, Offset);
> > > >     }
> > > > ```
> > > > 
> > > > ```
> > > > <workspace>/llvm-project/llvm/include/llvm/Support/Casting.h:64:53: error: type 'clang::StringLiteral *' cannot be used prior to '::' because it has no members
> > > >   static inline bool doit(const From &Val) { return To::classof(&Val); }
> > > > ```
> > > Ah, shame. This ends up calling `dyn_cast` on a `llvm::Pointerunion` as far as I can see, which seems to return `nullptr` in case the cast is unsuccessful anyway, so you could try just exploiting that: https://llvm.org/doxygen/classllvm_1_1PointerUnion.html#a9147352f78d98ee246f25d3300e0aaba
> > What about this? I.e. usong `StringLiteral*` directly in the `dyn_cast` call?
> ```
> FAILED: tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaChecking.cpp.o 
> /usr/lib/llvm/14/bin/clang++ -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I<llvm-project>/build/tools/clang/lib/Sema -I<llvm-project>/clang/lib/Sema -I<llvm-project>/clang/include -I<llvm-project>/build/tools/clang/include -I<llvm-project>/build/include -I<llvm-project>/llvm/include -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -fdiagnostics-color -fno-common -Woverloaded-virtual -Wno-nested-anon-types -g  -fno-exceptions -fno-rtti -std=c++14 -MD -MT tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaChecking.cpp.o -MF tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaChecking.cpp.o.d -o tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaChecking.cpp.o -c <llvm-project>/clang/lib/Sema/SemaChecking.cpp
> In file included from <llvm-project>/clang/lib/Sema/SemaChecking.cpp:14:
> In file included from <llvm-project>/clang/include/clang/AST/APValue.h:18:
> In file included from <llvm-project>/llvm/include/llvm/ADT/APFloat.h:19:
> In file included from <llvm-project>/llvm/include/llvm/ADT/ArrayRef.h:15:
> <llvm-project>/llvm/include/llvm/ADT/STLExtras.h:199:42: error: implicit instantiation of undefined template 'llvm::FirstIndexOfType<const clang::StringLiteral *>'
>     : std::integral_constant<size_t, 1 + FirstIndexOfType<T, Us...>::value> {};
>                                          ^
> <llvm-project>/llvm/include/llvm/ADT/STLExtras.h:199:42: note: in instantiation of template class 'llvm::FirstIndexOfType<const clang::StringLiteral *, clang::DynamicAllocLValue>' requested here
> <llvm-project>/llvm/include/llvm/ADT/STLExtras.h:199:42: note: in instantiation of template class 'llvm::FirstIndexOfType<const clang::StringLiteral *, clang::TypeInfoLValue, clang::DynamicAllocLValue>' requested here
> <llvm-project>/llvm/include/llvm/ADT/STLExtras.h:199:42: note: in instantiation of template class 'llvm::FirstIndexOfType<const clang::StringLiteral *, const clang::Expr *, clang::TypeInfoLValue, clang::DynamicAllocLValue>' requested here
> <llvm-project>/llvm/include/llvm/ADT/PointerUnion.h:230:30: note: in instantiation of template class 'llvm::FirstIndexOfType<const clang::StringLiteral *, const clang::ValueDecl *, const clang::Expr *, clang::TypeInfoLValue, clang::DynamicAllocLValue>' requested here
>     return F.Val.getInt() == FirstIndexOfType<To, PTs...>::value;
>                              ^
> <llvm-project>/llvm/include/llvm/ADT/PointerUnion.h:248:27: note: in instantiation of function template specialization 'llvm::CastInfoPointerUnionImpl<const clang::ValueDecl *, const clang::Expr *, clang::TypeInfoLValue, clang::DynamicAllocLValue>::isPossible<const clang::StringLiteral *>' requested here
>     return Impl::template isPossible<To>(f);
>                           ^
> <llvm-project>/llvm/include/llvm/Support/Casting.h:312:19: note: in instantiation of member function 'llvm::CastInfo<const clang::StringLiteral *, llvm::PointerUnion<const clang::ValueDecl *, const clang::Expr *, clang::TypeInfoLValue, clang::DynamicAllocLValue>>::isPossible' requested here
>     if (!Derived::isPossible(f))
>                   ^
> <llvm-project>/llvm/include/llvm/Support/Casting.h:407:23: note: in instantiation of member function 'llvm::DefaultDoCastIfPossible<const clang::StringLiteral *, llvm::PointerUnion<const clang::ValueDecl *, const clang::Expr *, clang::TypeInfoLValue, clang::DynamicAllocLValue>, llvm::CastInfo<const clang::StringLiteral *, llvm::PointerUnion<const clang::ValueDecl *, const clang::Expr *, clang::TypeInfoLValue, clang::DynamicAllocLValue>>>::doCastIfPossible' requested here
>     return ForwardTo::doCastIfPossible(const_cast<NonConstFrom>(f));
>                       ^
> <llvm-project>/clang/include/clang/AST/APValue.h:167:37: note: in instantiation of function template specialization 'llvm::PointerUnion<const clang::ValueDecl *, const clang::Expr *, clang::TypeInfoLValue, clang::DynamicAllocLValue>::dyn_cast<const clang::StringLiteral *>' requested here
>     T dyn_cast() const { return Ptr.dyn_cast<T>(); }
>                                     ^
> <llvm-project>/clang/lib/Sema/SemaChecking.cpp:8509:44: note: in instantiation of function template specialization 'clang::APValue::LValueBase::dyn_cast<const clang::StringLiteral *>' requested here
>     auto *LVE = Result.Val.getLValueBase().dyn_cast<const StringLiteral *>();
>                                            ^
> <llvm-project>/llvm/include/llvm/ADT/STLExtras.h:196:46: note: template is declared here
> template <typename T, typename... Us> struct FirstIndexOfType;
>                                              ^
> In file included from <llvm-project>/clang/lib/Sema/SemaChecking.cpp:14:
> In file included from <llvm-project>/clang/include/clang/AST/APValue.h:22:
> <llvm-project>/llvm/include/llvm/ADT/PointerUnion.h:234:12: error: no matching function for call to 'isPossible'
>     assert(isPossible<To>(F) && "cast to an incompatible type !");
>            ^~~~~~~~~~~~~~
> /usr/include/assert.h:90:27: note: expanded from macro 'assert'
>      (static_cast <bool> (expr)                                         \
>                           ^~~~
> <llvm-project>/llvm/include/llvm/ADT/PointerUnion.h:251:53: note: in instantiation of function template specialization 'llvm::CastInfoPointerUnionImpl<const clang::ValueDecl *, const clang::Expr *, clang::TypeInfoLValue, clang::DynamicAllocLValue>::doCast<const clang::StringLiteral *>' requested here
>   static To doCast(From &f) { return Impl::template doCast<To>(f); }
>                                                     ^
> <llvm-project>/llvm/include/llvm/Support/Casting.h:314:21: note: in instantiation of member function 'llvm::CastInfo<const clang::StringLiteral *, llvm::PointerUnion<const clang::ValueDecl *, const clang::Expr *, clang::TypeInfoLValue, clang::DynamicAllocLValue>>::doCast' requested here
>     return Derived::doCast(f);
>                     ^
> <llvm-project>/llvm/include/llvm/Support/Casting.h:407:23: note: in instantiation of member function 'llvm::DefaultDoCastIfPossible<const clang::StringLiteral *, llvm::PointerUnion<const clang::ValueDecl *, const clang::Expr *, clang::TypeInfoLValue, clang::DynamicAllocLValue>, llvm::CastInfo<const clang::StringLiteral *, llvm::PointerUnion<const clang::ValueDecl *, const clang::Expr *, clang::TypeInfoLValue, clang::DynamicAllocLValue>>>::doCastIfPossible' requested here
>     return ForwardTo::doCastIfPossible(const_cast<NonConstFrom>(f));
>                       ^
> <llvm-project>/clang/include/clang/AST/APValue.h:167:37: note: in instantiation of function template specialization 'llvm::PointerUnion<const clang::ValueDecl *, const clang::Expr *, clang::TypeInfoLValue, clang::DynamicAllocLValue>::dyn_cast<const clang::StringLiteral *>' requested here
>     T dyn_cast() const { return Ptr.dyn_cast<T>(); }
>                                     ^
> <llvm-project>/clang/lib/Sema/SemaChecking.cpp:8509:44: note: in instantiation of function template specialization 'clang::APValue::LValueBase::dyn_cast<const clang::StringLiteral *>' requested here
>     auto *LVE = Result.Val.getLValueBase().dyn_cast<const StringLiteral *>();
>                                            ^
> <llvm-project>/llvm/include/llvm/ADT/PointerUnion.h:229:45: note: candidate template ignored: substitution failure [with To = const clang::StringLiteral *]
>   template <typename To> static inline bool isPossible(From &F) {
>                                             ^
> 2 errors generated.
> [1908/2420] Building CXX object tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaExpr.cpp.o
> ninja: build stopped: subcommand failed.
> ```
> 
> oops, did I misunderstand you? this is my source code:
> ```
>   // If this expression can be evaluated at compile-time,
>   // check if the result is a StringLiteral and retry
>   Expr::EvalResult Result;
>   if (E->EvaluateAsRValue(Result, S.Context) && Result.Val.isLValue()) {
>     auto *LVE = Result.Val.getLValueBase().dyn_cast<const StringLiteral *>();
>     if (LVE)
>       return checkFormatStringExpr(
>           S, LVE, Args, APK, format_idx, firstDataArg, Type, CallType,
>           /*InFunctionCall*/ false, CheckedVarArgs, UncoveredArg, Offset);
>   }
> ```
Okay, the documentation tricked me, sorry. Should be fine as-is.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130906/new/

https://reviews.llvm.org/D130906



More information about the cfe-commits mailing list