hans added inline comments.
================
Comment at: clang/lib/Sema/SemaCast.cpp:895
+ if (!Self.getLangOpts().RTTIData) {
+ bool isMSVC = Self.getDiagnostics().getDiagnosticOptions().getFormat() ==
+ DiagnosticOptions::MSVC;
----------------
zequanwu wrote:
> 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
>
When targeting Windows, dynamic_cast to void* is implemented with in a runtime
function, RTCastToVoid: https://godbolt.org/z/Kecr7z
I wonder if that uses RTTI data internally though...
For typeid() I guess it would also warn on references? Maybe we should do the
same.
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
