zequanwu added inline comments.
================
Comment at: clang/lib/Sema/SemaCast.cpp:895
+ if (!Self.getLangOpts().RTTIData) {
+ bool isMSVC = Self.getDiagnostics().getDiagnosticOptions().getFormat() ==
+ DiagnosticOptions::MSVC;
----------------
hans wrote:
> zequanwu wrote:
> > hans wrote:
> > > I'm not sure isMSVC is the best name (it's not clear what aspect is MSVC
> > > exactly -- in this case it's the diagnostics format).
> > >
> > > It's possible to target MSVC both with clang-cl and with regular clang.
> > >
> > > For example, one could use
> > >
> > > clang-cl /c /tmp/a.cpp
> > >
> > > or
> > >
> > > clang -c /tmp/a.cpp -target i686-pc-windows-msvc19.11.0 -fms-extensions
> > >
> > >
> > > My understanding is that the purpose of "isMSVC" here is to try and
> > > detect if we're using clang-cl or clang so that the diagnostic can say
> > > "/GR-" or "-fno-rtti-data". So maybe it's better to call it "isClangCL"
> > > or something like that.
> > >
> > > Also, I don't think we should check "isMSVC" in the if-statement below.
> > > We want the warning to fire both when using clang and clang-cl: as long
> > > as -fno-rtti-data or /GR- is used, the warning makes sense.
> > >
> > > So I think the code could be more like:
> > >
> > > ```
> > > if (!Self.getLangOpts().RTTIData && !DestPointee->isVoidType()) {
> > > bool isClangCL = ...;
> > > Self.Diag(...) << isClangCL;
> > > }
> > > ```
> > MSVC will warn even if the DestPointee is void type. What I thought is if
> > invoked by clang-cl warn regardless of DeskPointee type. If invoked by
> > clang, warn if it's not void type.
> > https://godbolt.org/z/475q5v. I noticed MSVC won't warn at typeid if /GR-
> > is given. Probably I should remove the warning in typeid.
> If it's true the casting to void* doesn't need RTTI data (I think it is, but
> would be good to verify), then it doesn't make sense to warn. We don't have
> to follow MSVC's behavior when it doesn't make sense :)
>
> Similar reasoning for typeid() - I assume it won't work with /GR- also with
> MSVC, so warning about it probably makes sense.
In clang, I believe that dynamic_cast to void* doesn't use RTTI data:
https://godbolt.org/z/Kbr7Mq
Looks like MSVC only warns if the operand of typeid is not pointer:
https://godbolt.org/z/chcMcn
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86369/new/
https://reviews.llvm.org/D86369
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
