This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGebf267b87d4b: [Sema][MSVC] warn at dynamic_cast/typeid when
/GR- is given (authored by zequanwu).
Changed prior to commit:
hans accepted this revision.
hans added a comment.
lgtm
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
zequanwu updated this revision to Diff 292017.
zequanwu added a comment.
reopen the diff and update.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86369/new/
https://reviews.llvm.org/D86369
Files:
clang/include/clang/Basic/DiagnosticGroups.td
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3e782bf8090c: [Sema][MSVC] warn at dynamic_cast when /GR- is
given (authored by zequanwu).
Changed prior to commit:
hans accepted this revision.
hans added a comment.
This revision is now accepted and ready to land.
LGTM
But please add a -triple parameter to the test files, and check the
dynamic_cast behavior in each case before committing.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
zequanwu updated this revision to Diff 289802.
zequanwu added a comment.
address comment.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86369/new/
https://reviews.llvm.org/D86369
Files:
clang/include/clang/Basic/DiagnosticGroups.td
hans added inline comments.
Comment at: clang/lib/Sema/SemaCast.cpp:897
+ DiagnosticOptions::MSVC;
+if (isMSVC || !DestPointee->isVoidType())
+ Self.Diag(OpRange.getBegin(),
zequanwu wrote:
> hans wrote:
> > I don't think the
zequanwu updated this revision to Diff 289769.
zequanwu marked an inline comment as done.
zequanwu added a comment.
address comment.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86369/new/
https://reviews.llvm.org/D86369
Files:
zequanwu added inline comments.
Comment at: clang/lib/Sema/SemaCast.cpp:897
+ DiagnosticOptions::MSVC;
+if (isMSVC || !DestPointee->isVoidType())
+ Self.Diag(OpRange.getBegin(),
hans wrote:
> I don't think the if-statement is necessary.
hans added a comment.
Okay, almost there..
Comment at: clang/lib/Sema/SemaCast.cpp:897
+ DiagnosticOptions::MSVC;
+if (isMSVC || !DestPointee->isVoidType())
+ Self.Diag(OpRange.getBegin(),
I don't think the if-statement is necessary.
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
zequanwu updated this revision to Diff 289603.
zequanwu added a comment.
warn at dynamic_cast like MSVC in clang-cl, but not in clang.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86369/new/
https://reviews.llvm.org/D86369
Files:
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
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
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
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
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
zequanwu updated this revision to Diff 288380.
zequanwu marked an inline comment as done.
zequanwu added a comment.
address comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86369/new/
https://reviews.llvm.org/D86369
Files:
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:
> I'm not
hans added inline comments.
Comment at: clang/lib/Sema/SemaCast.cpp:895
+ if (!Self.getLangOpts().RTTIData) {
+bool isMSVC = Self.getDiagnostics().getDiagnosticOptions().getFormat() ==
+ DiagnosticOptions::MSVC;
I'm not sure isMSVC is the
zequanwu added inline comments.
Comment at: clang/test/SemaCXX/ms_no_dynamic_cast.cpp:1
+// RUN: %clang_cl %s /GR- -fsyntax-only 2>&1 | FileCheck %s
+
hans wrote:
> When using %clang_cl, the source file should always come after a "--",
> otherwise if for
zequanwu updated this revision to Diff 288062.
zequanwu marked 3 inline comments as done.
zequanwu added a comment.
Address cmments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86369/new/
https://reviews.llvm.org/D86369
Files:
hans added inline comments.
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7436
"use of dynamic_cast requires -frtti">;
+def warn_no_dynamic_cast_with_RTTI_disabled: Warning<
+ "dynamic_cast will not work since RTTI data is disabled by "
The
zequanwu updated this revision to Diff 287835.
zequanwu added a comment.
address comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86369/new/
https://reviews.llvm.org/D86369
Files:
clang/include/clang/Basic/DiagnosticGroups.td
hans added inline comments.
Comment at: clang/test/SemaCXX/ms_dynamic_cast.cpp:16
+B *b = new D1();
+auto d = dynamic_cast(b); // expected-warning{{should not use
dynamic_cast with /GR-}}
+}
zequanwu wrote:
> lebedev.ri wrote:
> > I'm not sure it makes
zequanwu added inline comments.
Comment at: clang/test/SemaCXX/ms_dynamic_cast.cpp:16
+B *b = new D1();
+auto d = dynamic_cast(b); // expected-warning{{should not use
dynamic_cast with /GR-}}
+}
lebedev.ri wrote:
> I'm not sure it makes sense to talk
lebedev.ri added inline comments.
Comment at: clang/test/SemaCXX/ms_dynamic_cast.cpp:16
+B *b = new D1();
+auto d = dynamic_cast(b); // expected-warning{{should not use
dynamic_cast with /GR-}}
+}
I'm not sure it makes sense to talk about MSVC/clang-cl
zequanwu created this revision.
zequanwu added a reviewer: hans.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
zequanwu requested review of this revision.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D86369
Files:
28 matches
Mail list logo
