[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given
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:
https://reviews.llvm.org/D86369?vs=292017=292269#toc
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
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaCast.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/test/SemaCXX/ms-no-rtti-data.cpp
clang/test/SemaCXX/no-rtti-data.cpp
Index: clang/test/SemaCXX/no-rtti-data.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/no-rtti-data.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 %s -triple x86_64-unknown-linux -fno-rtti-data -fsyntax-only -verify
+
+namespace std {
+struct type_info {};
+} // namespace std
+class B {
+public:
+ virtual ~B() = default;
+};
+
+class D1 : public B {
+public:
+ ~D1() = default;
+};
+
+void f() {
+ B *b = new D1();
+ auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not work since RTTI data is disabled by -fno-rtti-data}}
+ void *v = dynamic_cast(b);
+
+ (void)typeid(int);
+ (void)typeid(b);
+ (void)typeid(*b); // expected-warning{{typeid will not work since RTTI data is disabled by -fno-rtti-data}}
+ B b2 = *b;
+ (void)typeid(b2);
+ (void)typeid(*); // expected-warning{{typeid will not work since RTTI data is disabled by -fno-rtti-data}}
+ (void)typeid((B &)b2);
+
+ B = b2;
+ (void)typeid(br); // expected-warning{{typeid will not work since RTTI data is disabled by -fno-rtti-data}}
+ (void)typeid();
+}
\ No newline at end of file
Index: clang/test/SemaCXX/ms-no-rtti-data.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/ms-no-rtti-data.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 %s -triple x86_64-windows-msvc -fdiagnostics-format msvc -fno-rtti-data -fsyntax-only -verify
+
+namespace std {
+struct type_info {};
+} // namespace std
+class B {
+public:
+ virtual ~B() = default;
+};
+
+class D1 : public B {
+public:
+ ~D1() = default;
+};
+
+void f() {
+ B *b = new D1();
+ auto d = dynamic_cast(b);// expected-warning{{dynamic_cast will not work since RTTI data is disabled by /GR-}}
+ void *v = dynamic_cast(b); // expected-warning{{dynamic_cast will not work since RTTI data is disabled by /GR-}}
+
+ (void)typeid(int);
+ (void)typeid(b);
+ (void)typeid(*b); // expected-warning{{typeid will not work since RTTI data is disabled by /GR-}}
+ B b2 = *b;
+ (void)typeid(b2);
+ (void)typeid(*); // expected-warning{{typeid will not work since RTTI data is disabled by /GR-}}
+ (void)typeid((B &)b2);
+
+ B = b2;
+ (void)typeid(br); // expected-warning{{typeid will not work since RTTI data is disabled by /GR-}}
+ (void)typeid();
+}
\ No newline at end of file
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -663,7 +663,16 @@
}
// The operand is an expression.
- return BuildCXXTypeId(TypeInfoType, OpLoc, (Expr*)TyOrExpr, RParenLoc);
+ ExprResult Result =
+ BuildCXXTypeId(TypeInfoType, OpLoc, (Expr *)TyOrExpr, RParenLoc);
+
+ if (!getLangOpts().RTTIData && !Result.isInvalid())
+if (auto *CTE = dyn_cast(Result.get()))
+ if (CTE->isPotentiallyEvaluated() && !CTE->isMostDerived(Context))
+Diag(OpLoc, diag::warn_no_typeid_with_rtti_disabled)
+<< (getDiagnostics().getDiagnosticOptions().getFormat() ==
+DiagnosticOptions::MSVC);
+ return Result;
}
/// Grabs __declspec(uuid()) off a type, or returns 0 if we cannot resolve to
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -889,6 +889,18 @@
return;
}
+ // Warns when dynamic_cast is used with RTTI data disabled.
+ if (!Self.getLangOpts().RTTIData) {
+bool MicrosoftABI =
+Self.getASTContext().getTargetInfo().getCXXABI().isMicrosoft();
+bool isClangCL = Self.getDiagnostics().getDiagnosticOptions().getFormat() ==
+ DiagnosticOptions::MSVC;
+if (MicrosoftABI || !DestPointee->isVoidType())
+ Self.Diag(OpRange.getBegin(),
+diag::warn_no_dynamic_cast_with_rtti_disabled)
+ << isClangCL;
+ }
+
// Done. Everything else is run-time checks.
Kind = CK_Dynamic;
}
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7451,6 +7451,12 @@
"use of typeid
[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given
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 https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given
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
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaCast.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/test/SemaCXX/ms-no-rtti-data.cpp
clang/test/SemaCXX/no-rtti-data.cpp
Index: clang/test/SemaCXX/no-rtti-data.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/no-rtti-data.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 %s -triple x86_64-unknown-linux -fno-rtti-data -fsyntax-only -verify
+
+namespace std {
+struct type_info {};
+} // namespace std
+class B {
+public:
+ virtual ~B() = default;
+};
+
+class D1 : public B {
+public:
+ ~D1() = default;
+};
+
+void f() {
+ B* b = new D1();
+ auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not work since RTTI data is disabled by -fno-rtti-data}}
+ void* v = dynamic_cast(b);
+
+ (void)typeid(int);
+ (void)typeid(b);
+ (void)typeid(*b); // expected-warning{{typeid will not work since RTTI data is disabled by -fno-rtti-data}}
+ B b2 = *b;
+ (void)typeid(b2);
+ (void)typeid(*); // expected-warning{{typeid will not work since RTTI data is disabled by -fno-rtti-data}}
+ (void)typeid((B&)b2);
+
+ B& br = b2;
+ (void)typeid(br); // expected-warning{{typeid will not work since RTTI data is disabled by -fno-rtti-data}}
+ (void)typeid();
+}
\ No newline at end of file
Index: clang/test/SemaCXX/ms-no-rtti-data.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/ms-no-rtti-data.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 %s -triple x86_64-windows-msvc -fdiagnostics-format msvc -fno-rtti-data -fsyntax-only -verify
+
+namespace std {
+struct type_info {};
+} // namespace std
+class B {
+public:
+ virtual ~B() = default;
+};
+
+class D1 : public B {
+public:
+ ~D1() = default;
+};
+
+void f() {
+ B* b = new D1();
+ auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not work since RTTI data is disabled by /GR-}}
+ void* v = dynamic_cast(b); // expected-warning{{dynamic_cast will not work since RTTI data is disabled by /GR-}}
+
+ (void)typeid(int);
+ (void)typeid(b);
+ (void)typeid(*b); // expected-warning{{typeid will not work since RTTI data is disabled by /GR-}}
+ B b2 = *b;
+ (void)typeid(b2);
+ (void)typeid(*); // expected-warning{{typeid will not work since RTTI data is disabled by /GR-}}
+ (void)typeid((B&)b2);
+
+ B& br = b2;
+ (void)typeid(br); // expected-warning{{typeid will not work since RTTI data is disabled by /GR-}}
+ (void)typeid();
+}
\ No newline at end of file
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -663,7 +663,16 @@
}
// The operand is an expression.
- return BuildCXXTypeId(TypeInfoType, OpLoc, (Expr*)TyOrExpr, RParenLoc);
+ ExprResult Result =
+ BuildCXXTypeId(TypeInfoType, OpLoc, (Expr *)TyOrExpr, RParenLoc);
+
+ if (!getLangOpts().RTTIData && !Result.isInvalid())
+if (auto *CTE = dyn_cast(Result.get()))
+ if (CTE->isPotentiallyEvaluated() && !CTE->isMostDerived(Context))
+Diag(OpLoc, diag::warn_no_typeid_with_rtti_disabled)
+<< (getDiagnostics().getDiagnosticOptions().getFormat() ==
+DiagnosticOptions::MSVC);
+ return Result;
}
/// Grabs __declspec(uuid()) off a type, or returns 0 if we cannot resolve to
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -889,6 +889,18 @@
return;
}
+ // Warns when dynamic_cast is used with RTTI data disabled.
+ if (!Self.getLangOpts().RTTIData) {
+bool MicrosoftABI =
+Self.getASTContext().getTargetInfo().getCXXABI().isMicrosoft();
+bool isClangCL = Self.getDiagnostics().getDiagnosticOptions().getFormat() ==
+ DiagnosticOptions::MSVC;
+if (MicrosoftABI || !DestPointee->isVoidType())
+ Self.Diag(OpRange.getBegin(),
+diag::warn_no_dynamic_cast_with_rtti_disabled)
+ << isClangCL;
+ }
+
// Done. Everything else is run-time checks.
Kind = CK_Dynamic;
}
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7451,6 +7451,12 @@
"use of typeid requires -frtti">;
def err_no_dynamic_cast_with_fno_rtti : Error<
"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
[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given
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:
https://reviews.llvm.org/D86369?vs=289802=290374#toc
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
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaCast.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/test/SemaCXX/ms_no_dynamic_cast.cpp
clang/test/SemaCXX/no_dynamic_cast.cpp
Index: clang/test/SemaCXX/no_dynamic_cast.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/no_dynamic_cast.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 %s -fno-rtti-data -fsyntax-only -verify
+
+namespace std {
+struct type_info {};
+} // namespace std
+class B {
+public:
+ virtual ~B() = default;
+};
+
+class D1 : public B {
+public:
+ ~D1() = default;
+};
+
+void f() {
+ B* b = new D1();
+ auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not work since RTTI data is disabled by -fno-rtti-data}}
+ void* v = dynamic_cast(b);
+ (void)typeid(int); // expected-warning{{typeid will not work since RTTI data is disabled by -fno-rtti-data}}
+}
Index: clang/test/SemaCXX/ms_no_dynamic_cast.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/ms_no_dynamic_cast.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 %s -triple x86_64-windows -fdiagnostics-format msvc -fno-rtti-data -fsyntax-only -verify
+
+namespace std {
+struct type_info {};
+} // namespace std
+class B {
+public:
+ virtual ~B() = default;
+};
+
+class D1 : public B {
+public:
+ ~D1() = default;
+};
+
+void f() {
+ B* b = new D1();
+ auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not work since RTTI data is disabled by /GR-}}
+ void* v = dynamic_cast(b); // expected-warning{{dynamic_cast will not work since RTTI data is disabled by /GR-}}
+ (void)typeid(int); // expected-warning{{typeid will not work since RTTI data is disabled by /GR-}}
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -646,6 +646,12 @@
return ExprError(Diag(OpLoc, diag::err_no_typeid_with_fno_rtti));
}
+ // Warns when typeid is used with RTTI data disabled.
+ if (!getLangOpts().RTTIData)
+Diag(OpLoc, diag::warn_no_typeid_with_rtti_disabled)
+<< (getDiagnostics().getDiagnosticOptions().getFormat() ==
+DiagnosticOptions::MSVC);
+
QualType TypeInfoType = Context.getTypeDeclType(CXXTypeInfoDecl);
if (isType) {
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -890,6 +890,18 @@
return;
}
+ // Warns when dynamic_cast is used with RTTI data disabled.
+ if (!Self.getLangOpts().RTTIData) {
+bool MicrosoftABI =
+Self.getASTContext().getTargetInfo().getCXXABI().isMicrosoft();
+bool isClangCL = Self.getDiagnostics().getDiagnosticOptions().getFormat() ==
+ DiagnosticOptions::MSVC;
+if (MicrosoftABI || !DestPointee->isVoidType())
+ Self.Diag(OpRange.getBegin(),
+diag::warn_no_dynamic_cast_with_rtti_disabled)
+ << isClangCL;
+ }
+
// Done. Everything else is run-time checks.
Kind = CK_Dynamic;
}
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7438,6 +7438,12 @@
"use of typeid requires -frtti">;
def err_no_dynamic_cast_with_fno_rtti : Error<
"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 "
+ "%select{-fno-rtti-data|/GR-}0">, InGroup;
+def warn_no_typeid_with_rtti_disabled: Warning<
+ "typeid will not work since RTTI data is disabled by "
+ "%select{-fno-rtti-data|/GR-}0">, InGroup;
def err_cannot_form_pointer_to_member_of_reference_type : Error<
"cannot form a pointer-to-member to member %0 of reference type %1">;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -1235,3 +1235,5 @@
}
def WebAssemblyExceptionSpec : DiagGroup<"wasm-exception-spec">;
+
+def RTTI : DiagGroup<"rtti">;
___
cfe-commits mailing list
[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given
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 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
[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given
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
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaCast.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/test/SemaCXX/ms_no_dynamic_cast.cpp
clang/test/SemaCXX/no_dynamic_cast.cpp
Index: clang/test/SemaCXX/no_dynamic_cast.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/no_dynamic_cast.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -fno-rtti-data -fsyntax-only -verify
+
+namespace std {
+struct type_info {};
+} // namespace std
+class B {
+public:
+ virtual ~B() = default;
+};
+
+class D1 : public B {
+public:
+ ~D1() = default;
+};
+
+void f() {
+ B *b = new D1();
+ auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not work since RTTI data is disabled by -fno-rtti-data}}
+ (void)typeid(int); // expected-warning{{typeid will not work since RTTI data is disabled by -fno-rtti-data}}
+}
Index: clang/test/SemaCXX/ms_no_dynamic_cast.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/ms_no_dynamic_cast.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -fdiagnostics-format msvc -fno-rtti-data -fsyntax-only -verify
+
+namespace std {
+struct type_info {};
+} // namespace std
+class B {
+public:
+ virtual ~B() = default;
+};
+
+class D1 : public B {
+public:
+ ~D1() = default;
+};
+
+void f() {
+ B *b = new D1();
+ auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not work since RTTI data is disabled by /GR-}}
+ (void)typeid(int); // expected-warning{{typeid will not work since RTTI data is disabled by /GR-}}
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -646,6 +646,12 @@
return ExprError(Diag(OpLoc, diag::err_no_typeid_with_fno_rtti));
}
+ // Warns when typeid is used with RTTI data disabled.
+ if (!getLangOpts().RTTIData)
+Diag(OpLoc, diag::warn_no_typeid_with_rtti_disabled)
+<< (getDiagnostics().getDiagnosticOptions().getFormat() ==
+DiagnosticOptions::MSVC);
+
QualType TypeInfoType = Context.getTypeDeclType(CXXTypeInfoDecl);
if (isType) {
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -890,6 +890,18 @@
return;
}
+ // Warns when dynamic_cast is used with RTTI data disabled.
+ if (!Self.getLangOpts().RTTIData) {
+bool MicrosoftABI =
+Self.getASTContext().getTargetInfo().getCXXABI().isMicrosoft();
+bool isClangCL = Self.getDiagnostics().getDiagnosticOptions().getFormat() ==
+ DiagnosticOptions::MSVC;
+if (MicrosoftABI || !DestPointee->isVoidType())
+ Self.Diag(OpRange.getBegin(),
+diag::warn_no_dynamic_cast_with_rtti_disabled)
+ << isClangCL;
+ }
+
// Done. Everything else is run-time checks.
Kind = CK_Dynamic;
}
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7433,6 +7433,12 @@
"use of typeid requires -frtti">;
def err_no_dynamic_cast_with_fno_rtti : Error<
"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 "
+ "%select{-fno-rtti-data|/GR-}0">, InGroup;
+def warn_no_typeid_with_rtti_disabled: Warning<
+ "typeid will not work since RTTI data is disabled by "
+ "%select{-fno-rtti-data|/GR-}0">, InGroup;
def err_cannot_form_pointer_to_member_of_reference_type : Error<
"cannot form a pointer-to-member to member %0 of reference type %1">;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -1229,3 +1229,5 @@
}
def WebAssemblyExceptionSpec : DiagGroup<"wasm-exception-spec">;
+
+def RTTI : DiagGroup<"rtti">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given
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 if-statement is necessary. I thought we concluded we want
> > to warn even for void*? Also note that "isMSVC" here is only checking the
> > driver mode, i.e. just the user interface to the compiler. The user could
> > still be compiling in MSVC mode.
> My bad. I thought you meant to use the previous version, so I reverted this
> region.
> Like we concluded, we want to warn even for void*. This only applies to
> clang-cl, not clang, right? This is the purpose of this if. If it's in
> clang-cl, warn regardless of destination type. If it's in clang, don't warn
> for void*, like line 887 which doesn't emit error for void* destination type.
>
> If RTTI is false, RTTIData is also false
> (https://github.com/llvm/llvm-project/blob/master/clang/lib/Frontend/CompilerInvocation.cpp#L2870).
> There is a test
> https://github.com/llvm/llvm-project/blob/master/clang/test/SemaCXX/no-rtti.cpp#L28,
> which should not be warned, right?
> Like we concluded, we want to warn even for void*. This only applies to
> clang-cl, not clang, right?
Oh, right, we need to do the "even for void*" part only when using the
Microsoft ABI. It's not about clang or clang-cl, those are just different user
interfaces really, but about what platform (processor, abi, etc.) the compiler
is targeting. It's possible to target Windows with regular clang, not just
clang-cl.
Inside Sema, you can check Context.getTargetInfo().getCXXABI().isMicrosoft() to
see if the microsoft abi is being targeted.
Apologies for my confusing comments here.
Comment at: clang/test/SemaCXX/ms_no_dynamic_cast.cpp:19
+ auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not
work since RTTI data is disabled by /GR-}}
+ (void)typeid(int); // expected-warning{{typeid will not work
since RTTI data is disabled by /GR-}}
+}
zequanwu wrote:
> hans wrote:
> > Is the cast to void necessary? Same comment for the next file.
> Yes, to suppress the warning of unused expression.
Ah, okay. Didn't realize that warning was on by default :)
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
[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given
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:
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaCast.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/test/SemaCXX/ms_no_dynamic_cast.cpp
clang/test/SemaCXX/no_dynamic_cast.cpp
Index: clang/test/SemaCXX/no_dynamic_cast.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/no_dynamic_cast.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -fno-rtti-data -fsyntax-only -verify
+
+namespace std {
+struct type_info {};
+} // namespace std
+class B {
+public:
+ virtual ~B() = default;
+};
+
+class D1 : public B {
+public:
+ ~D1() = default;
+};
+
+void f() {
+ B *b = new D1();
+ auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not work since RTTI data is disabled by -fno-rtti-data}}
+ (void)typeid(int); // expected-warning{{typeid will not work since RTTI data is disabled by -fno-rtti-data}}
+}
Index: clang/test/SemaCXX/ms_no_dynamic_cast.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/ms_no_dynamic_cast.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -fdiagnostics-format msvc -fno-rtti-data -fsyntax-only -verify
+
+namespace std {
+struct type_info {};
+} // namespace std
+class B {
+public:
+ virtual ~B() = default;
+};
+
+class D1 : public B {
+public:
+ ~D1() = default;
+};
+
+void f() {
+ B *b = new D1();
+ auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not work since RTTI data is disabled by /GR-}}
+ (void)typeid(int); // expected-warning{{typeid will not work since RTTI data is disabled by /GR-}}
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -646,6 +646,12 @@
return ExprError(Diag(OpLoc, diag::err_no_typeid_with_fno_rtti));
}
+ // Warns when typeid is used with RTTI data disabled.
+ if (!getLangOpts().RTTIData)
+Diag(OpLoc, diag::warn_no_typeid_with_rtti_disabled)
+<< (getDiagnostics().getDiagnosticOptions().getFormat() ==
+DiagnosticOptions::MSVC);
+
QualType TypeInfoType = Context.getTypeDeclType(CXXTypeInfoDecl);
if (isType) {
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -890,6 +890,16 @@
return;
}
+ // Warns when dynamic_cast is used with RTTI data disabled.
+ if (!Self.getLangOpts().RTTIData) {
+bool isClangCL = Self.getDiagnostics().getDiagnosticOptions().getFormat() ==
+ DiagnosticOptions::MSVC;
+if (isClangCL || !DestPointee->isVoidType())
+ Self.Diag(OpRange.getBegin(),
+diag::warn_no_dynamic_cast_with_rtti_disabled)
+ << isClangCL;
+ }
+
// Done. Everything else is run-time checks.
Kind = CK_Dynamic;
}
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7433,6 +7433,12 @@
"use of typeid requires -frtti">;
def err_no_dynamic_cast_with_fno_rtti : Error<
"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 "
+ "%select{-fno-rtti-data|/GR-}0">, InGroup;
+def warn_no_typeid_with_rtti_disabled: Warning<
+ "typeid will not work since RTTI data is disabled by "
+ "%select{-fno-rtti-data|/GR-}0">, InGroup;
def err_cannot_form_pointer_to_member_of_reference_type : Error<
"cannot form a pointer-to-member to member %0 of reference type %1">;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -1229,3 +1229,5 @@
}
def WebAssemblyExceptionSpec : DiagGroup<"wasm-exception-spec">;
+
+def RTTI : DiagGroup<"rtti">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given
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. I thought we concluded we want
> to warn even for void*? Also note that "isMSVC" here is only checking the
> driver mode, i.e. just the user interface to the compiler. The user could
> still be compiling in MSVC mode.
My bad. I thought you meant to use the previous version, so I reverted this
region.
Like we concluded, we want to warn even for void*. This only applies to
clang-cl, not clang, right? This is the purpose of this if. If it's in
clang-cl, warn regardless of destination type. If it's in clang, don't warn for
void*, like line 887 which doesn't emit error for void* destination type.
If RTTI is false, RTTIData is also false
(https://github.com/llvm/llvm-project/blob/master/clang/lib/Frontend/CompilerInvocation.cpp#L2870).
There is a test
https://github.com/llvm/llvm-project/blob/master/clang/test/SemaCXX/no-rtti.cpp#L28,
which should not be warned, right?
Comment at: clang/test/SemaCXX/ms_no_dynamic_cast.cpp:19
+ auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not
work since RTTI data is disabled by /GR-}}
+ (void)typeid(int); // expected-warning{{typeid will not work
since RTTI data is disabled by /GR-}}
+}
hans wrote:
> Is the cast to void necessary? Same comment for the next file.
Yes, to suppress the warning of unused expression.
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
[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given
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. I thought we concluded we want to
warn even for void*? Also note that "isMSVC" here is only checking the driver
mode, i.e. just the user interface to the compiler. The user could still be
compiling in MSVC mode.
Comment at: clang/lib/Sema/SemaExprCXX.cpp:649
+ // Warns when typeid is used with RTTI disabled.
+ if (!getLangOpts().RTTIData)
s/RTTI/RTTI data/
(the "RTTI disabled" case is on line 646)
Comment at: clang/test/SemaCXX/ms_no_dynamic_cast.cpp:19
+ auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not
work since RTTI data is disabled by /GR-}}
+ (void)typeid(int); // expected-warning{{typeid will not work
since RTTI data is disabled by /GR-}}
+}
Is the cast to void necessary? Same comment for the next file.
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
[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given
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:
> > > 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.
> > Couldn't find if `__RTCastToVoid` uses RTTI data internally.
> >
> > For typeid(), it also warn on references. But the behavior is a bit weird
> > (https://godbolt.org/z/jn4Pjx). Seems like it warns only when dereferencing
> > a pointer or argument is a reference.
> Okay, maybe it's safest to warn about dynamic_cast also for void*, like MSVC
> does.
>
> For typeid() maybe we should be conservative like Clang is with -fno-rtti,
> and always warn.
>
> But I'm a little bit surprised about this, because I'd imagine typeid(int)
> could work also with -fno-rtti... if you're curious maybe it would be
> interesting to dig deeper into how this works.
It might just because typeid is a rtti-only thing when introduced,
https://en.cppreference.com/w/cpp/types. gcc also gives error for this.
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
[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given
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:
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaCast.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/test/SemaCXX/ms_no_dynamic_cast.cpp
clang/test/SemaCXX/no_dynamic_cast.cpp
Index: clang/test/SemaCXX/no_dynamic_cast.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/no_dynamic_cast.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -fno-rtti-data -fsyntax-only -verify
+
+namespace std {
+struct type_info {};
+} // namespace std
+class B {
+public:
+ virtual ~B() = default;
+};
+
+class D1 : public B {
+public:
+ ~D1() = default;
+};
+
+void f() {
+ B *b = new D1();
+ auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not work since RTTI data is disabled by -fno-rtti-data}}
+ (void)typeid(int); // expected-warning{{typeid will not work since RTTI data is disabled by -fno-rtti-data}}
+}
Index: clang/test/SemaCXX/ms_no_dynamic_cast.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/ms_no_dynamic_cast.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -fdiagnostics-format msvc -fno-rtti-data -fsyntax-only -verify
+
+namespace std {
+struct type_info {};
+} // namespace std
+class B {
+public:
+ virtual ~B() = default;
+};
+
+class D1 : public B {
+public:
+ ~D1() = default;
+};
+
+void f() {
+ B *b = new D1();
+ auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not work since RTTI data is disabled by /GR-}}
+ (void)typeid(int); // expected-warning{{typeid will not work since RTTI data is disabled by /GR-}}
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -646,6 +646,12 @@
return ExprError(Diag(OpLoc, diag::err_no_typeid_with_fno_rtti));
}
+ // Warns when typeid is used with RTTI disabled.
+ if (!getLangOpts().RTTIData)
+Diag(OpLoc, diag::warn_no_typeid_with_rtti_disabled)
+<< (getDiagnostics().getDiagnosticOptions().getFormat() ==
+DiagnosticOptions::MSVC);
+
QualType TypeInfoType = Context.getTypeDeclType(CXXTypeInfoDecl);
if (isType) {
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -890,6 +890,16 @@
return;
}
+ // Warns when dynamic_cast is used with RTTI disabled.
+ if (!Self.getLangOpts().RTTIData) {
+bool isMSVC = Self.getDiagnostics().getDiagnosticOptions().getFormat() ==
+ DiagnosticOptions::MSVC;
+if (isMSVC || !DestPointee->isVoidType())
+ Self.Diag(OpRange.getBegin(),
+diag::warn_no_dynamic_cast_with_rtti_disabled)
+ << isMSVC;
+ }
+
// Done. Everything else is run-time checks.
Kind = CK_Dynamic;
}
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7433,6 +7433,12 @@
"use of typeid requires -frtti">;
def err_no_dynamic_cast_with_fno_rtti : Error<
"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 "
+ "%select{-fno-rtti-data|/GR-}0">, InGroup;
+def warn_no_typeid_with_rtti_disabled: Warning<
+ "typeid will not work since RTTI data is disabled by "
+ "%select{-fno-rtti-data|/GR-}0">, InGroup;
def err_cannot_form_pointer_to_member_of_reference_type : Error<
"cannot form a pointer-to-member to member %0 of reference type %1">;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -1229,3 +1229,5 @@
}
def WebAssemblyExceptionSpec : DiagGroup<"wasm-exception-spec">;
+
+def RTTI : DiagGroup<"rtti">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given
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:
> > > > 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.
> Couldn't find if `__RTCastToVoid` uses RTTI data internally.
>
> For typeid(), it also warn on references. But the behavior is a bit weird
> (https://godbolt.org/z/jn4Pjx). Seems like it warns only when dereferencing a
> pointer or argument is a reference.
Okay, maybe it's safest to warn about dynamic_cast also for void*, like MSVC
does.
For typeid() maybe we should be conservative like Clang is with -fno-rtti, and
always warn.
But I'm a little bit surprised about this, because I'd imagine typeid(int)
could work also with -fno-rtti... if you're curious maybe it would be
interesting to dig deeper into how this works.
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
[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given
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:
> > > 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.
Couldn't find if `__RTCastToVoid` uses RTTI data internally.
For typeid(), it also warn on references. But the behavior is a bit weird
(https://godbolt.org/z/jn4Pjx). Seems like it warns only when dereferencing a
pointer or argument is a reference.
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
[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given
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
[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given
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
[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given
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:
> > 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.
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
[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given
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:
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaCast.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/test/SemaCXX/ms_no_dynamic_cast.cpp
clang/test/SemaCXX/no_dynamic_cast.cpp
Index: clang/test/SemaCXX/no_dynamic_cast.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/no_dynamic_cast.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -fno-rtti-data -fsyntax-only -verify
+
+namespace std {
+struct type_info {};
+} // namespace std
+class B {
+public:
+ virtual ~B() = default;
+};
+
+class D1 : public B {
+public:
+ ~D1() = default;
+};
+
+void f() {
+ B *b = new D1();
+ auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not work since RTTI data is disabled by -fno-rtti-data}}
+ (void)typeid(int); // expected-warning{{typeid will not work since RTTI data is disabled by -fno-rtti-data}}
+}
Index: clang/test/SemaCXX/ms_no_dynamic_cast.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/ms_no_dynamic_cast.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -fdiagnostics-format msvc -fno-rtti-data -fsyntax-only -verify
+
+namespace std {
+struct type_info {};
+} // namespace std
+class B {
+public:
+ virtual ~B() = default;
+};
+
+class D1 : public B {
+public:
+ ~D1() = default;
+};
+
+void f() {
+ B *b = new D1();
+ auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not work since RTTI data is disabled by /GR-}}
+ (void)typeid(int); // expected-warning{{typeid will not work since RTTI data is disabled by /GR-}}
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -646,6 +646,12 @@
return ExprError(Diag(OpLoc, diag::err_no_typeid_with_fno_rtti));
}
+ // Warns when typeid is used with RTTI disabled.
+ if (!getLangOpts().RTTIData)
+Diag(OpLoc, diag::warn_no_typeid_with_rtti_disabled)
+<< (getDiagnostics().getDiagnosticOptions().getFormat() ==
+DiagnosticOptions::MSVC);
+
QualType TypeInfoType = Context.getTypeDeclType(CXXTypeInfoDecl);
if (isType) {
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -890,6 +890,14 @@
return;
}
+ // Warns when dynamic_cast is used with RTTI disabled.
+ if (!Self.getLangOpts().RTTIData && !DestPointee->isVoidType()) {
+bool isClangCL = Self.getDiagnostics().getDiagnosticOptions().getFormat() ==
+ DiagnosticOptions::MSVC;
+Self.Diag(OpRange.getBegin(), diag::warn_no_dynamic_cast_with_rtti_disabled)
+<< isClangCL;
+ }
+
// Done. Everything else is run-time checks.
Kind = CK_Dynamic;
}
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7433,6 +7433,12 @@
"use of typeid requires -frtti">;
def err_no_dynamic_cast_with_fno_rtti : Error<
"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 "
+ "%select{-fno-rtti-data|/GR-}0">, InGroup;
+def warn_no_typeid_with_rtti_disabled: Warning<
+ "typeid will not work since RTTI data is disabled by "
+ "%select{-fno-rtti-data|/GR-}0">, InGroup;
def err_cannot_form_pointer_to_member_of_reference_type : Error<
"cannot form a pointer-to-member to member %0 of reference type %1">;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -1229,3 +1229,5 @@
}
def WebAssemblyExceptionSpec : DiagGroup<"wasm-exception-spec">;
+
+def RTTI : DiagGroup<"rtti">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given
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 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.
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
[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given
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 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;
}
```
Comment at: clang/test/SemaCXX/ms_no_dynamic_cast.cpp:1
+// RUN: %clang_cl %s /GR- -fsyntax-only 2>&1 | FileCheck %s
+
zequanwu wrote:
> hans wrote:
> > When using %clang_cl, the source file should always come after a "--",
> > otherwise if for example the source file is "/Users/foo/src/test.cc" the
> > filename can get interpreted as a command-line option.
> >
> > But tests outside of Driver/ generally invoke cc1 directly, with %clang_cc1
> > and passing the appropriate flags. I'd suggest doing that here too. (And in
> > the other test.)
> This test is for testing in clang-cl. Another test is for clang. If I use
> %clang_cc1, /GR- can not be passed.
But clang-cl is just a driver mode, all it does is pass flags to the cc1
invocation. So normally we test the driver separately (in Driver/) and the
compiler (cc1) separately.
In this case the warning lives in the compiler (cc1) so it makes sense to
target that directly with the test. If you grep through clang/test/Sema and
clang/test/SemaCXX, you'll see that %clang_cl is not use in any test.
You should be able to run cc1 with and without "-fdiagnostics-format msvc" and
-fno-rtti-data to test the new warnings.
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
[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given
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 example the source file is "/Users/foo/src/test.cc" the > filename can get interpreted as a command-line option. > > But tests outside of Driver/ generally invoke cc1 directly, with %clang_cc1 > and passing the appropriate flags. I'd suggest doing that here too. (And in > the other test.) This test is for testing in clang-cl. Another test is for clang. If I use %clang_cc1, /GR- can not be passed. 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
[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given
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:
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaCast.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/test/SemaCXX/ms_no_dynamic_cast.cpp
clang/test/SemaCXX/no_dynamic_cast.cpp
Index: clang/test/SemaCXX/no_dynamic_cast.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/no_dynamic_cast.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -fno-rtti-data -fsyntax-only -verify
+
+namespace std {
+struct type_info {};
+} // namespace std
+class B {
+public:
+ virtual ~B() = default;
+};
+
+class D1 : public B {
+public:
+ ~D1() = default;
+};
+
+void f() {
+ B *b = new D1();
+ auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not work since RTTI data is disabled by -fno-rtti-data}}
+ (void)typeid(int); // expected-warning{{typeid will not work since RTTI data is disabled by -fno-rtti-data}}
+}
Index: clang/test/SemaCXX/ms_no_dynamic_cast.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/ms_no_dynamic_cast.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cl /GR- -fsyntax-only -- %s 2>&1 | FileCheck %s
+
+namespace std {
+struct type_info {};
+} // namespace std
+class B {
+public:
+ virtual ~B() = default;
+};
+
+class D1 : public B {
+public:
+ ~D1() = default;
+};
+
+void f() {
+ B *b = new D1();
+ auto d = dynamic_cast(b); // CHECK: warning: dynamic_cast will not work since RTTI data is disabled by /GR-
+ (void)typeid(int); // CHECK: warning: typeid will not work since RTTI data is disabled by /GR-
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -646,6 +646,12 @@
return ExprError(Diag(OpLoc, diag::err_no_typeid_with_fno_rtti));
}
+ // Warns when typeid is used with RTTI disabled.
+ if (!getLangOpts().RTTIData)
+Diag(OpLoc, diag::warn_no_typeid_with_rtti_disabled)
+<< (getDiagnostics().getDiagnosticOptions().getFormat() ==
+DiagnosticOptions::MSVC);
+
QualType TypeInfoType = Context.getTypeDeclType(CXXTypeInfoDecl);
if (isType) {
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -890,6 +890,16 @@
return;
}
+ // Warns when dynamic_cast is used with RTTI disabled.
+ if (!Self.getLangOpts().RTTIData) {
+bool isMSVC = Self.getDiagnostics().getDiagnosticOptions().getFormat() ==
+ DiagnosticOptions::MSVC;
+if (isMSVC || !DestPointee->isVoidType())
+ Self.Diag(OpRange.getBegin(),
+diag::warn_no_dynamic_cast_with_rtti_disabled)
+ << isMSVC;
+ }
+
// Done. Everything else is run-time checks.
Kind = CK_Dynamic;
}
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7433,6 +7433,12 @@
"use of typeid requires -frtti">;
def err_no_dynamic_cast_with_fno_rtti : Error<
"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 "
+ "%select{-fno-rtti-data|/GR-}0">, InGroup;
+def warn_no_typeid_with_rtti_disabled: Warning<
+ "typeid will not work since RTTI data is disabled by "
+ "%select{-fno-rtti-data|/GR-}0">, InGroup;
def err_cannot_form_pointer_to_member_of_reference_type : Error<
"cannot form a pointer-to-member to member %0 of reference type %1">;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -1229,3 +1229,5 @@
}
def WebAssemblyExceptionSpec : DiagGroup<"wasm-exception-spec">;
+
+def RTTI : DiagGroup<"rtti">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given
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 other warning names spell rtti in lower-case, so I'd suggest that here too
for consistency.
Comment at: clang/lib/Sema/SemaCast.cpp:895
+ if (!Self.getLangOpts().RTTIData &&
+ !Self.getDiagnostics().isIgnored(diag::warn_no_typeid_with_RTTI_disabled,
+ OpRange.getBegin())) {
Is the isIgnored() check necessary here? I think in general we call Diag()
anyway, and if the warning is ignored, it will be ignored.
The reason we sometimes check isIgnored() explicitly is to avoid expensive
computation when possible, but that's not the case here.
Comment at: clang/lib/Sema/SemaCast.cpp:900
+diag::warn_no_dynamic_cast_with_RTTI_disabled)
+ << Self.getLangOpts().MSVCCompat;
+ }
getLangOpts().MSVCCompat can be true both with clang-cl and regular clang, it
just depends on teh -fms-compatibility flag.
Driver::IsCLMode() is the sure way to check for clang-cl, but I don't think
that information is forwarded to the compiler frontend.
One slightly hacky way to check, and in this case maybe it makes sense, is to
look at the TextDiagnosticFormat. If that's MSVC, it's very likely the driver
was clang-cl.
Comment at: clang/test/SemaCXX/ms_no_dynamic_cast.cpp:1
+// RUN: %clang_cl %s /GR- -fsyntax-only 2>&1 | FileCheck %s
+
When using %clang_cl, the source file should always come after a "--",
otherwise if for example the source file is "/Users/foo/src/test.cc" the
filename can get interpreted as a command-line option.
But tests outside of Driver/ generally invoke cc1 directly, with %clang_cc1 and
passing the appropriate flags. I'd suggest doing that here too. (And in the
other test.)
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
[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given
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
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaCast.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/test/SemaCXX/ms_no_dynamic_cast.cpp
clang/test/SemaCXX/no_dynamic_cast.cpp
Index: clang/test/SemaCXX/no_dynamic_cast.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/no_dynamic_cast.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -fno-rtti-data -fsyntax-only -verify
+
+namespace std {
+struct type_info {};
+} // namespace std
+class B {
+public:
+ virtual ~B() = default;
+};
+
+class D1 : public B {
+public:
+ ~D1() = default;
+};
+
+void f() {
+ B *b = new D1();
+ auto d = dynamic_cast(b); // expected-warning{{dynamic_cast will not work since RTTI data is disabled by -fno-rtti-data}}
+ (void)typeid(int); // expected-warning{{typeid will not work since RTTI data is disabled by -fno-rtti-data}}
+}
\ No newline at end of file
Index: clang/test/SemaCXX/ms_no_dynamic_cast.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/ms_no_dynamic_cast.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cl %s /GR- -fsyntax-only 2>&1 | FileCheck %s
+
+namespace std {
+struct type_info {};
+} // namespace std
+class B {
+public:
+ virtual ~B() = default;
+};
+
+class D1 : public B {
+public:
+ ~D1() = default;
+};
+
+void f() {
+ B *b = new D1();
+ auto d = dynamic_cast(b); // CHECK: warning: dynamic_cast will not work since RTTI data is disabled by /GR-
+ (void)typeid(int); // CHECK: warning: typeid will not work since RTTI data is disabled by /GR-
+}
\ No newline at end of file
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -646,6 +646,13 @@
return ExprError(Diag(OpLoc, diag::err_no_typeid_with_fno_rtti));
}
+ // Warns when typeid is used with RTTI disabled.
+ if (!getLangOpts().RTTIData &&
+ !getDiagnostics().isIgnored(diag::warn_no_typeid_with_RTTI_disabled,
+ OpLoc))
+Diag(OpLoc, diag::warn_no_typeid_with_RTTI_disabled)
+<< getLangOpts().MSVCCompat;
+
QualType TypeInfoType = Context.getTypeDeclType(CXXTypeInfoDecl);
if (isType) {
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -890,6 +890,16 @@
return;
}
+ // Warns when dynamic_cast is used with RTTI disabled.
+ if (!Self.getLangOpts().RTTIData &&
+ !Self.getDiagnostics().isIgnored(diag::warn_no_typeid_with_RTTI_disabled,
+ OpRange.getBegin())) {
+if (Self.getLangOpts().MSVCCompat || !DestPointee->isVoidType())
+ Self.Diag(OpRange.getBegin(),
+diag::warn_no_dynamic_cast_with_RTTI_disabled)
+ << Self.getLangOpts().MSVCCompat;
+ }
+
// Done. Everything else is run-time checks.
Kind = CK_Dynamic;
}
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7433,6 +7433,12 @@
"use of typeid requires -frtti">;
def err_no_dynamic_cast_with_fno_rtti : Error<
"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 "
+ "%select{-fno-rtti-data|/GR-}0">, InGroup;
+def warn_no_typeid_with_RTTI_disabled: Warning<
+ "typeid will not work since RTTI data is disabled by "
+ "%select{-fno-rtti-data|/GR-}0">, InGroup;
def err_cannot_form_pointer_to_member_of_reference_type : Error<
"cannot form a pointer-to-member to member %0 of reference type %1">;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -1229,3 +1229,5 @@
}
def WebAssemblyExceptionSpec : DiagGroup<"wasm-exception-spec">;
+
+def RTTI : DiagGroup<"rtti">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given
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 sense to talk about MSVC/clang-cl flags when normal
> > clang is used.
> > Either the diag should only be used in MSVC compat mode,
> > or it should talk about `-fno-rtti` in non-MSVC compat mode.
> I am not sure either... But `-fno-rtti-data` is only used when clang-cl
> driver translate `/GR-` to `-fno-rtti-data`.
Instead of "should not use" maybe the warning could say something which
suggests it won't work, like "dynamic_cast will not work since RTTI data is
disabled" or something like that.
I'm not sure what to do about the name of the option. -fno-rtti-data was added
to support clang-cl's /GR- option, but it could also be used with regular
Clang. Maybe the warning doesn't need to mention the name of the flag? Or it
could mention both? Or maybe it could figure out if it's in clang-cl or regular
clang mode?
Also, should it also warn about typeid()?
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
[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given
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 about MSVC/clang-cl flags when normal
> clang is used.
> Either the diag should only be used in MSVC compat mode,
> or it should talk about `-fno-rtti` in non-MSVC compat mode.
I am not sure either... But `-fno-rtti-data` is only used when clang-cl driver
translate `/GR-` to `-fno-rtti-data`.
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
[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given
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 flags when normal clang
is used.
Either the diag should only be used in MSVC compat mode,
or it should talk about `-fno-rtti` in non-MSVC compat mode.
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
[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given
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:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaCast.cpp
clang/test/SemaCXX/ms_dynamic_cast.cpp
Index: clang/test/SemaCXX/ms_dynamic_cast.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/ms_dynamic_cast.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -fno-rtti-data -fsyntax-only -verify
+
+class B {
+public:
+virtual ~B() = default;
+};
+
+class D1 : public B {
+public:
+~D1() = default;
+
+};
+
+void f() {
+B *b = new D1();
+auto d = dynamic_cast(b); // expected-warning{{should not use
dynamic_cast with /GR-}}
+}
\ No newline at end of file
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -890,6 +890,11 @@
return;
}
+ // MSVC warns when dynamic_cast is used with /GR-.
+ if (!Self.getLangOpts().RTTIData) {
+Self.Diag(OpRange.getBegin(), diag::warn_no_dynamic_cast_with_no_GR);
+ }
+
// Done. Everything else is run-time checks.
Kind = CK_Dynamic;
}
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7433,6 +7433,8 @@
"use of typeid requires -frtti">;
def err_no_dynamic_cast_with_fno_rtti : Error<
"use of dynamic_cast requires -frtti">;
+def warn_no_dynamic_cast_with_no_GR: Warning<
+ "should not use dynamic_cast with /GR-">;
def err_cannot_form_pointer_to_member_of_reference_type : Error<
"cannot form a pointer-to-member to member %0 of reference type %1">;
Index: clang/test/SemaCXX/ms_dynamic_cast.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/ms_dynamic_cast.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -fno-rtti-data -fsyntax-only -verify
+
+class B {
+public:
+virtual ~B() = default;
+};
+
+class D1 : public B {
+public:
+~D1() = default;
+
+};
+
+void f() {
+B *b = new D1();
+auto d = dynamic_cast(b); // expected-warning{{should not use dynamic_cast with /GR-}}
+}
\ No newline at end of file
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -890,6 +890,11 @@
return;
}
+ // MSVC warns when dynamic_cast is used with /GR-.
+ if (!Self.getLangOpts().RTTIData) {
+Self.Diag(OpRange.getBegin(), diag::warn_no_dynamic_cast_with_no_GR);
+ }
+
// Done. Everything else is run-time checks.
Kind = CK_Dynamic;
}
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7433,6 +7433,8 @@
"use of typeid requires -frtti">;
def err_no_dynamic_cast_with_fno_rtti : Error<
"use of dynamic_cast requires -frtti">;
+def warn_no_dynamic_cast_with_no_GR: Warning<
+ "should not use dynamic_cast with /GR-">;
def err_cannot_form_pointer_to_member_of_reference_type : Error<
"cannot form a pointer-to-member to member %0 of reference type %1">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
