[PATCH] D37521: [Sema][Typo correction] Assertion failure in typo correction if chain of member function calls has multiple typos
iid_iunknown created this revision.
This is a patch proposal for PR34507.
Typo resolution can create new TypoExprs while transforming an expression.
These TypoExprs are not transformed, they are present in the resulting
expression and cause the `DelayedTypos.empty() && "Uncorrected typos!"`
assertion failure in `clang::Sema::~Sema()`. If clang is built without
assertions, these TypoExprs are not reported causing incomplete diagnostics.
The patch checks the transformed expression for new TypoExprs and, if any
found, transforms the expression again until either all TypoExprs are handled
or the result becomes invalid.
Repository:
rL LLVM
https://reviews.llvm.org/D37521
Files:
lib/Sema/SemaExprCXX.cpp
test/Sema/typo-correction-multiple-typos.cpp
Index: test/Sema/typo-correction-multiple-typos.cpp
===
--- test/Sema/typo-correction-multiple-typos.cpp
+++ test/Sema/typo-correction-multiple-typos.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// This file contains typo correction test which ensures that
+// multiple typos in a single member calls chain are correctly
+// diagnosed.
+
+class X
+{
+public:
+ void foo() const; // expected-note {{'foo' declared here}}
+};
+
+class Y
+{
+public:
+ const X& getX() const { return m_x; } // expected-note {{'getX' declared
here}}
+ int getN() const { return m_n; }
+private:
+ X m_x;
+ int m_n;
+};
+
+class Z
+{
+public:
+ const Y& getY0() const { return m_y0; } // expected-note {{'getY0' declared
here}}
+ const Y& getY1() const { return m_y1; }
+
+private:
+ Y m_y0;
+ Y m_y1;
+};
+
+Z z_obj;
+
+void test()
+{
+ z_obj.getY2(). // expected-error {{no member named 'getY2' in 'Z'; did you
mean 'getY0'}}
+getM(). // expected-error {{no member named 'getM' in 'Y'; did you
mean 'getX'}}
+foe(); // expected-error {{no member named 'foe' in 'X'; did you
mean 'foo'}}
+}
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -7362,6 +7362,30 @@
break;
}
+// The transform is able to produce new TypoExprs while resolving the
typos.
+// These new TypoExprs are not resolved by the transform, they do not get
+// into the TypoExprs container and are not reported, so they need to be
+// handled separately.
+// If the transform result is valid and contains newly created TypoExprs,
+// transform the result expression again until no new TypoExprs get created
+// or the result becomes an invalid expression. Return the longest valid
+// expression to report as many typos as possible.
+if (!Res.isInvalid()) {
+ while (true) {
+unsigned TyposCount = TypoExprs.size();
+FindTypoExprs(TypoExprs).TraverseStmt(Res.get());
+if (TypoExprs.size() == TyposCount)
+ // No new TypoExprs created by the transform
+ break;
+ExprResult TmpRes = TryTransform(Res.get());
+if (TmpRes.isInvalid())
+ // Further transform prodices an invalid Expr.
+ // Stop with the last valid result.
+ break;
+Res = TmpRes;
+ }
+}
+
// Ensure none of the TypoExprs have multiple typo correction candidates
// with the same edit length that pass all the checks and filters.
// TODO: Properly handle various permutations of possible corrections when
Index: test/Sema/typo-correction-multiple-typos.cpp
===
--- test/Sema/typo-correction-multiple-typos.cpp
+++ test/Sema/typo-correction-multiple-typos.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// This file contains typo correction test which ensures that
+// multiple typos in a single member calls chain are correctly
+// diagnosed.
+
+class X
+{
+public:
+ void foo() const; // expected-note {{'foo' declared here}}
+};
+
+class Y
+{
+public:
+ const X& getX() const { return m_x; } // expected-note {{'getX' declared here}}
+ int getN() const { return m_n; }
+private:
+ X m_x;
+ int m_n;
+};
+
+class Z
+{
+public:
+ const Y& getY0() const { return m_y0; } // expected-note {{'getY0' declared here}}
+ const Y& getY1() const { return m_y1; }
+
+private:
+ Y m_y0;
+ Y m_y1;
+};
+
+Z z_obj;
+
+void test()
+{
+ z_obj.getY2(). // expected-error {{no member named 'getY2' in 'Z'; did you mean 'getY0'}}
+getM(). // expected-error {{no member named 'getM' in 'Y'; did you mean 'getX'}}
+foe(); // expected-error {{no member named 'foe' in 'X'; did you mean 'foo'}}
+}
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -7362,6 +7362,30 @@
break;
}
+// The transform is able to produce new TypoExprs while resolving the typos.
+// These new TypoExprs
[PATCH] D37336: [clang-cl] Explicitly set object format to COFF in CL mode
iid_iunknown added a comment. Thanks for reviewing this! Repository: rL LLVM https://reviews.llvm.org/D37336 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37336: [clang-cl] Explicitly set object format to COFF in CL mode
iid_iunknown added a comment. > Do you think you can write a test for your patch that will work on the > buildbots we have? I am afraid, this is not possible with the existing buildbots. `--target` does not affect the default triple. There is `LLVM_TARGET_TRIPLE_ENV` that allows to override the default triple at runtime, but LLVM must be configured to support it and none of the Windows buildbots do this. I see no other way to implement such a test, unfortunately. Repository: rL LLVM https://reviews.llvm.org/D37336 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37336: [clang-cl] Explicitly set object format to COFF in CL mode
iid_iunknown added a comment. In https://reviews.llvm.org/D37336#857802, @hans wrote: > But why isn't that failure showing on some buildbot, then? The test needs `system-windows` to run: // REQUIRES: system-windows There is no windows buildbot that builds clang defaulted to the AArch64 target. Repository: rL LLVM https://reviews.llvm.org/D37336 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37336: [clang-cl] Explicitly set object format to COFF in CL mode
iid_iunknown created this revision. Herald added subscribers: kristof.beyls, aemerson. Currently object format is taken from the default target triple. For toolchains with a non-COFF default target this may result in an object format inappropriate for pc-windows and lead to compilation issues. For example, the default triple `aarch64-linux-elf` may produce something like `aarch64-pc-windows-msvc19.0.24215-elf` in CL mode. Clang creates `MicrosoftARM64TargetInfo` for such triple with data layout `e-m:w-p:64:64-i32:32-i64:64-i128:128-n32:64-S128`. On the other hand, the AArch64 backend in `computeDataLayout` detects a non-COFF target and selects `e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128` as data layout for little endian. Different layouts used by clang and the backend cause an error: error: backend data layout 'e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128' does not match expected target description 'e-m:w-p:64:64-i32:32-i64:64-i128:128-n32:64-S128' This can be observed on the clang's Driver/cl-pch.c test with AArch64 as a default target. This patch enforces COFF in CL mode. Repository: rL LLVM https://reviews.llvm.org/D37336 Files: lib/Driver/Driver.cpp Index: lib/Driver/Driver.cpp === --- lib/Driver/Driver.cpp +++ lib/Driver/Driver.cpp @@ -663,6 +663,7 @@ T.setOS(llvm::Triple::Win32); T.setVendor(llvm::Triple::PC); T.setEnvironment(llvm::Triple::MSVC); +T.setObjectFormat(llvm::Triple::COFF); DefaultTargetTriple = T.str(); } if (const Arg *A = Args.getLastArg(options::OPT_target)) Index: lib/Driver/Driver.cpp === --- lib/Driver/Driver.cpp +++ lib/Driver/Driver.cpp @@ -663,6 +663,7 @@ T.setOS(llvm::Triple::Win32); T.setVendor(llvm::Triple::PC); T.setEnvironment(llvm::Triple::MSVC); +T.setObjectFormat(llvm::Triple::COFF); DefaultTargetTriple = T.str(); } if (const Arg *A = Args.getLastArg(options::OPT_target)) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34430: [Clang][TypoCorrection] Clang hangs in typo correction
iid_iunknown added a comment. A gentle reminder about this patch. Repository: rL LLVM https://reviews.llvm.org/D34430 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34430: [Clang][TypoCorrection] Clang hangs in typo correction
iid_iunknown added a comment. No hang with this patch on the test case from PR33484. Clang reports a number of expected compilation errors instead. Repository: rL LLVM https://reviews.llvm.org/D34430 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34430: [Clang][TypoCorrection] Clang hangs in typo correction
iid_iunknown updated this revision to Diff 103297.
iid_iunknown added a comment.
Fixed white spaces.
Repository:
rL LLVM
https://reviews.llvm.org/D34430
Files:
lib/Sema/SemaExprCXX.cpp
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -7246,10 +7246,13 @@
/// TransformCache). Returns true if there is still any untried combinations
/// of corrections.
bool CheckAndAdvanceTypoExprCorrectionStreams() {
+bool CheckLimitExceeded =
+SemaRef.TyposCorrected >=
+SemaRef.getDiagnostics().getDiagnosticOptions().SpellCheckingLimit;
for (auto TE : TypoExprs) {
auto = SemaRef.getTypoExprState(TE);
TransformCache.erase(TE);
- if (!State.Consumer->finished())
+ if (!CheckLimitExceeded && !State.Consumer->finished())
return true;
State.Consumer->resetCorrectionStream();
}
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -7246,10 +7246,13 @@
/// TransformCache). Returns true if there is still any untried combinations
/// of corrections.
bool CheckAndAdvanceTypoExprCorrectionStreams() {
+bool CheckLimitExceeded =
+SemaRef.TyposCorrected >=
+SemaRef.getDiagnostics().getDiagnosticOptions().SpellCheckingLimit;
for (auto TE : TypoExprs) {
auto = SemaRef.getTypoExprState(TE);
TransformCache.erase(TE);
- if (!State.Consumer->finished())
+ if (!CheckLimitExceeded && !State.Consumer->finished())
return true;
State.Consumer->resetCorrectionStream();
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34430: [Clang][TypoCorrection] Clang hangs in typo correction
iid_iunknown created this revision.
iid_iunknown added a project: clang.
This is a patch for PR33484.
The clang's typo correction logic may fall into an infinite loop when reaching
the typo correction limit.
When some complex expression has multiple typos in it, clang finds possible
corrections for each typo and processes various permutations of these
corrections. The typo corrections counter is incremented during the process and
compared to the max allowed limit (50 by default). Hang may happen if the limit
is reached in the middle of complex expression processing. In this case
transform of the current part of the expression fails, making clang ignore its
subsequent parts and bail out to the main transform loop in
`TransformTypos::Transform(Expr *E)`. On the next iteration, clang fails at the
very beginning of transform and no longer reaches the rest of the expression
immediately bailing out to the main loop. The transform cannot advance since
this moment. As the loop exit condition requires either a fully successful
transform or having all the permutations processed, it can never become true.
Yet another reproducer with a readable C++ code:
namespace
{
struct V
{
float x;
};
class H
{
public:
const V& getV() const { return mV; }
int getN() const { return mN; }
private:
V mV;
int mN;
};
class T
{
public:
const H& getH0() const { return mH0; }
const H& getH1() const { return mH1; }
const V& getV0() const { return mH0.getV(); }
const V& getV1() const { return mH1.getV(); }
private:
H mH0;
H mH1;
};
T sT;
H sH;
}
void func()
{
( sT.getH2().getM() > sH.getM() ?
sT.getH3() :
0 );
}
Here, both the condition and LHS of `?:` have typos. If the limit is exceeded,
the transform will fail on the condition and will no longer proceed to LHS and
process its substitutions.
The patch adds a limit check into the main loop exit condition.
Repository:
rL LLVM
https://reviews.llvm.org/D34430
Files:
lib/Sema/SemaExprCXX.cpp
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -7246,10 +7246,13 @@
/// TransformCache). Returns true if there is still any untried combinations
/// of corrections.
bool CheckAndAdvanceTypoExprCorrectionStreams() {
+ bool CheckLimitExceeded =
+SemaRef.TyposCorrected >=
+SemaRef.getDiagnostics().getDiagnosticOptions().SpellCheckingLimit;
for (auto TE : TypoExprs) {
auto = SemaRef.getTypoExprState(TE);
TransformCache.erase(TE);
- if (!State.Consumer->finished())
+ if (!CheckLimitExceeded && !State.Consumer->finished())
return true;
State.Consumer->resetCorrectionStream();
}
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -7246,10 +7246,13 @@
/// TransformCache). Returns true if there is still any untried combinations
/// of corrections.
bool CheckAndAdvanceTypoExprCorrectionStreams() {
+ bool CheckLimitExceeded =
+SemaRef.TyposCorrected >=
+SemaRef.getDiagnostics().getDiagnosticOptions().SpellCheckingLimit;
for (auto TE : TypoExprs) {
auto = SemaRef.getTypoExprState(TE);
TransformCache.erase(TE);
- if (!State.Consumer->finished())
+ if (!CheckLimitExceeded && !State.Consumer->finished())
return true;
State.Consumer->resetCorrectionStream();
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D16901: [Clang driver, ARM] Do not add +long-calls in PIC mode
iid_iunknown abandoned this revision. iid_iunknown added a comment. Abandoning the patch as it is too old. Repository: rL LLVM https://reviews.llvm.org/D16901 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
