llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) <details> <summary>Changes</summary> This was originally removed in `8bdcd522510f923185cdfaec66c4a78d0a0d38c0` under the assumption this wasn't required (i.e., LLDB should just always pretend it's in a mutable context). But since function qualifiers affect overloading in C++, this assumption can lead to unexpected expression evaluator behaviour. Instead, in this patch we try to add function qualifiers to `$__lldb_class::$__lldb_expr` that resemble the qualifiers in the method that we're stopped in. However, mutating variables or calling arbitrary member functions from CV-qualified methods can be useful/is something users already may be used to. To provide users with the ability to ignore the CV-qualifiers of the current context, we will provide an expression evaluator flag that switches this off in a follow-up patch. --- Patch is 37.45 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/177922.diff 26 Files Affected: - (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp (+7-1) - (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp (+34-9) - (modified) lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py (+2-2) - (modified) lldb/test/API/lang/cpp/const_this/TestConstThis.py (+4-6) - (added) lldb/test/API/lang/cpp/expression-context-qualifiers/const_method/Makefile (+3) - (added) lldb/test/API/lang/cpp/expression-context-qualifiers/const_method/TestExprInConstMethod.py (+106) - (added) lldb/test/API/lang/cpp/expression-context-qualifiers/const_method/main.cpp (+49) - (added) lldb/test/API/lang/cpp/expression-context-qualifiers/const_volatile_method/Makefile (+3) - (added) lldb/test/API/lang/cpp/expression-context-qualifiers/const_volatile_method/TestExprInConstVolatileMethod.py (+60) - (added) lldb/test/API/lang/cpp/expression-context-qualifiers/const_volatile_method/main.cpp (+43) - (added) lldb/test/API/lang/cpp/expression-context-qualifiers/cv_qualified_objects/Makefile (+3) - (added) lldb/test/API/lang/cpp/expression-context-qualifiers/cv_qualified_objects/TestExprOnCVQualifiedObjects.py (+21) - (added) lldb/test/API/lang/cpp/expression-context-qualifiers/cv_qualified_objects/main.cpp (+25) - (added) lldb/test/API/lang/cpp/expression-context-qualifiers/fixit/Makefile (+3) - (added) lldb/test/API/lang/cpp/expression-context-qualifiers/fixit/TestExprInConstMethodWithFixit.py (+42) - (added) lldb/test/API/lang/cpp/expression-context-qualifiers/fixit/main.cpp (+22) - (added) lldb/test/API/lang/cpp/expression-context-qualifiers/non_const_method/Makefile (+3) - (added) lldb/test/API/lang/cpp/expression-context-qualifiers/non_const_method/TestExprInNonConstMethod.py (+85) - (added) lldb/test/API/lang/cpp/expression-context-qualifiers/non_const_method/main.cpp (+49) - (added) lldb/test/API/lang/cpp/expression-context-qualifiers/template_const_method/Makefile (+3) - (added) lldb/test/API/lang/cpp/expression-context-qualifiers/template_const_method/TestExprInTemplateConstMethod.py (+106) - (added) lldb/test/API/lang/cpp/expression-context-qualifiers/template_const_method/main.cpp (+51) - (added) lldb/test/API/lang/cpp/expression-context-qualifiers/template_non_const_method/Makefile (+3) - (added) lldb/test/API/lang/cpp/expression-context-qualifiers/template_non_const_method/TestExprInTemplateNonConstMethod.py (+85) - (added) lldb/test/API/lang/cpp/expression-context-qualifiers/template_non_const_method/main.cpp (+49) - (modified) lldb/test/API/lang/cpp/this/TestCPPThis.py (+9-3) ``````````diff diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp index 24a5dc5362964..41d2d2cb23146 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp @@ -844,6 +844,12 @@ void ClangExpressionDeclMap::LookUpLldbClass(NameSearchContext &context) { QualType class_qual_type = m_ast_context->getCanonicalTagType(class_decl); + // The synthesized __lldb_expr will adopt the qualifiers from this class + // type. Make sure we use the qualifiers of the method that we're currently + // stopped in. + class_qual_type.addFastQualifiers( + method_decl->getMethodQualifiers().getFastQualifiers()); + TypeFromUser class_user_type( class_qual_type.getAsOpaquePtr(), function_decl_ctx.GetTypeSystem()->weak_from_this()); @@ -1991,7 +1997,7 @@ void ClangExpressionDeclMap::AddContextClassType(NameSearchContext &context, std::array<CompilerType, 1> args{void_clang_type.GetPointerType()}; CompilerType method_type = m_clang_ast_context->CreateFunctionType( - void_clang_type, args, false, 0); + void_clang_type, args, false, ut.GetTypeQualifiers()); const bool is_virtual = false; const bool is_static = false; diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp index cfe187ffc4114..ca4199faa3841 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp @@ -10,6 +10,7 @@ #include "ClangExpressionUtil.h" +#include "clang/AST/TypeBase.h" #include "clang/Basic/CharInfo.h" #include "clang/Basic/FileManager.h" #include "clang/Basic/SourceManager.h" @@ -189,6 +190,28 @@ static void AddMacros(const DebugMacros *dm, CompileUnit *comp_unit, } } +/// Return qualifers of the current C++ method. +static clang::Qualifiers GetFrameCVQualifiers(StackFrame *frame) { + if (!frame) + return {}; + + auto this_sp = frame->FindVariable(ConstString("this")); + if (!this_sp) + return {}; + + // Lambdas that capture 'this' have a member variable called 'this'. The class + // context of __lldb_expr for a lambda is the class type of the 'this' capture + // (not the anonymous lambda structure). So use the qualifiers of the captured + // 'this'. + if (auto this_this_sp = this_sp->GetChildMemberWithName("this")) + return clang::Qualifiers::fromCVRMask( + this_this_sp->GetCompilerType().GetPointeeType().GetTypeQualifiers()); + + // Not in a lambda. Return 'this' qualifiers. + return clang::Qualifiers::fromCVRMask( + this_sp->GetCompilerType().GetPointeeType().GetTypeQualifiers()); +} + lldb_private::ClangExpressionSourceCode::ClangExpressionSourceCode( llvm::StringRef filename, llvm::StringRef name, llvm::StringRef prefix, llvm::StringRef body, Wrapping wrap, WrapKind wrap_kind) @@ -463,15 +486,17 @@ bool ClangExpressionSourceCode::GetText( lldb_local_var_decls.GetData(), tagged_body.c_str()); break; case WrapKind::CppMemberFunction: - wrap_stream.Printf("%s" - "void \n" - "$__lldb_class::%s(void *$__lldb_arg) \n" - "{ \n" - " %s; \n" - "%s" - "} \n", - module_imports.c_str(), m_name.c_str(), - lldb_local_var_decls.GetData(), tagged_body.c_str()); + wrap_stream.Printf( + "%s" + "void \n" + "$__lldb_class::%s(void *$__lldb_arg) %s \n" + "{ \n" + " %s; \n" + "%s" + "} \n", + module_imports.c_str(), m_name.c_str(), + GetFrameCVQualifiers(exe_ctx.GetFramePtr()).getAsString().c_str(), + lldb_local_var_decls.GetData(), tagged_body.c_str()); break; case WrapKind::ObjCInstanceMethod: wrap_stream.Printf( diff --git a/lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py b/lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py index e35cfa6a289a7..0a7683b310f43 100644 --- a/lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py +++ b/lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py @@ -127,8 +127,8 @@ def test_expr_inside_lambda(self): # Inside non_capturing_method lldbutil.continue_to_breakpoint(process, bkpt) - self.expect_expr("local", result_type="int", result_value="5") - self.expect_expr("local2", result_type="int", result_value="10") + self.expect_expr("local", result_type="const int", result_value="5") + self.expect_expr("local2", result_type="const int", result_value="10") self.expect_expr("local2 * local", result_type="int", result_value="50") self.expectExprError( diff --git a/lldb/test/API/lang/cpp/const_this/TestConstThis.py b/lldb/test/API/lang/cpp/const_this/TestConstThis.py index 4b7d3aadb62ab..c2df61fde2b58 100644 --- a/lldb/test/API/lang/cpp/const_this/TestConstThis.py +++ b/lldb/test/API/lang/cpp/const_this/TestConstThis.py @@ -11,10 +11,9 @@ def run_class_tests(self): # Expression not referencing context class. self.expect_expr("1 + 1", result_type="int", result_value="2") # Referencing context class. - # FIXME: This and the expression below should return const types. - self.expect_expr("member", result_type="int", result_value="3") + self.expect_expr("member", result_type="const int", result_value="3") # Check the type of context class. - self.expect_expr("this", result_type="ContextClass *") + self.expect_expr("this", result_type="const ContextClass *") def test_member_func(self): self.build() @@ -36,10 +35,9 @@ def run_template_class_tests(self): # Expression not referencing context class. self.expect_expr("1 + 1", result_type="int", result_value="2") # Referencing context class. - # FIXME: This and the expression below should return const types. - self.expect_expr("member", result_type="int", result_value="4") + self.expect_expr("member", result_type="const int", result_value="4") # Check the type of context class. - self.expect_expr("this", result_type="TemplatedContextClass<int> *") + self.expect_expr("this", result_type="const TemplatedContextClass<int> *") def test_template_member_func(self): self.build() diff --git a/lldb/test/API/lang/cpp/expression-context-qualifiers/const_method/Makefile b/lldb/test/API/lang/cpp/expression-context-qualifiers/const_method/Makefile new file mode 100644 index 0000000000000..99998b20bcb05 --- /dev/null +++ b/lldb/test/API/lang/cpp/expression-context-qualifiers/const_method/Makefile @@ -0,0 +1,3 @@ +CXX_SOURCES := main.cpp + +include Makefile.rules diff --git a/lldb/test/API/lang/cpp/expression-context-qualifiers/const_method/TestExprInConstMethod.py b/lldb/test/API/lang/cpp/expression-context-qualifiers/const_method/TestExprInConstMethod.py new file mode 100644 index 0000000000000..0ba6c8a837db4 --- /dev/null +++ b/lldb/test/API/lang/cpp/expression-context-qualifiers/const_method/TestExprInConstMethod.py @@ -0,0 +1,106 @@ +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class TestCase(TestBase): + def test(self): + self.build() + (_, process, _, _) = lldbutil.run_to_source_breakpoint( + self, "Break: const_method begin", lldb.SBFileSpec("main.cpp") + ) + + self.expect_expr("bar()", result_value="2", result_type="int") + self.expect( + "expression m_const_mem = 2.0", + error=True, + substrs=[ + "cannot assign to non-static data member", + "with const-qualified type", + ], + ) + self.expect( + "expression m_mem = 2.0", + error=True, + substrs=[ + "cannot assign to non-static data member within const member function" + ], + ) + self.expect_expr("((Foo*)this)->bar()", result_type="double", result_value="5") + + lldbutil.continue_to_source_breakpoint( + self, + process, + "Break: const_method no-this lambda", + lldb.SBFileSpec("main.cpp"), + ) + + self.expect( + "expression x = 7.0", + error=True, + substrs=[ + "cannot assign to non-static data member within const member function" + ], + ) + + lldbutil.continue_to_source_breakpoint( + self, + process, + "Break: const_method mutable no-this lambda", + lldb.SBFileSpec("main.cpp"), + ) + + self.expect_expr("x = 7.0") + self.expect_expr("x", result_value="7") + + lldbutil.continue_to_source_breakpoint( + self, process, "Break: const_method lambda", lldb.SBFileSpec("main.cpp") + ) + + # FIXME: mutating this capture should be disallowed in a non-mutable lambda. + self.expect_expr("y = 8.0") + self.expect_expr("bar()", result_value="2", result_type="int") + self.expect( + "expression m_const_mem = 2.0", + error=True, + substrs=[ + "cannot assign to non-static data member", + "with const-qualified type", + ], + ) + self.expect( + "expression m_mem = 2.0", + error=True, + substrs=[ + "cannot assign to non-static data member within const member function" + ], + ) + self.expect_expr("m_mem", result_value="-2") + self.expect_expr("((Foo*)this)->bar()", result_type="double", result_value="5") + + lldbutil.continue_to_source_breakpoint( + self, + process, + "Break: const_method mutable lambda", + lldb.SBFileSpec("main.cpp"), + ) + + self.expect_expr("y = 9.0") + self.expect_expr("bar()", result_value="2", result_type="int") + self.expect( + "expression m_const_mem = 2.0", + error=True, + substrs=[ + "cannot assign to non-static data member", + "with const-qualified type", + ], + ) + self.expect( + "expression m_mem = 2.0", + error=True, + substrs=[ + "cannot assign to non-static data member within const member function" + ], + ) + self.expect_expr("m_mem", result_value="-2") diff --git a/lldb/test/API/lang/cpp/expression-context-qualifiers/const_method/main.cpp b/lldb/test/API/lang/cpp/expression-context-qualifiers/const_method/main.cpp new file mode 100644 index 0000000000000..7cb74767b458a --- /dev/null +++ b/lldb/test/API/lang/cpp/expression-context-qualifiers/const_method/main.cpp @@ -0,0 +1,49 @@ +#include <cassert> +#include <cstdio> + +struct Foo { + double bar() { return 5.0; } + + int bar() const { return 2; } + + int const_method() const { + auto x = bar(); + assert(x == 2); + std::puts("Break: const_method begin"); + + [x] { + std::puts("Keep on multiple lines..."); + std::puts("Break: const_method no-this lambda"); + }(); + + [x]() mutable { + std::puts("Keep on multiple lines..."); + std::puts("Break: const_method mutable no-this lambda"); + }(); + + [this, y = x] { + auto x = bar() + y; + std::puts("Break: const_method lambda"); + }(); + + [this, y = x]() mutable { + auto x = bar() + y; + std::puts("Break: const_method mutable lambda"); + }(); + + return 120; + } + + float m_mem = -2.0; + const float m_const_mem = -3.0; +}; + +int main() { + const Foo f; + f.bar(); + + Foo f2; + f2.bar(); + + return Foo{}.const_method(); +} diff --git a/lldb/test/API/lang/cpp/expression-context-qualifiers/const_volatile_method/Makefile b/lldb/test/API/lang/cpp/expression-context-qualifiers/const_volatile_method/Makefile new file mode 100644 index 0000000000000..99998b20bcb05 --- /dev/null +++ b/lldb/test/API/lang/cpp/expression-context-qualifiers/const_volatile_method/Makefile @@ -0,0 +1,3 @@ +CXX_SOURCES := main.cpp + +include Makefile.rules diff --git a/lldb/test/API/lang/cpp/expression-context-qualifiers/const_volatile_method/TestExprInConstVolatileMethod.py b/lldb/test/API/lang/cpp/expression-context-qualifiers/const_volatile_method/TestExprInConstVolatileMethod.py new file mode 100644 index 0000000000000..f20b9d8164841 --- /dev/null +++ b/lldb/test/API/lang/cpp/expression-context-qualifiers/const_volatile_method/TestExprInConstVolatileMethod.py @@ -0,0 +1,60 @@ +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class TestCase(TestBase): + def test(self): + self.build() + (_, process, _, _) = lldbutil.run_to_source_breakpoint( + self, "Break here: const", lldb.SBFileSpec("main.cpp") + ) + + self.expect_expr("bar()", result_type="double", result_value="5") + self.expect_expr("const_volatile_method()") + self.expect_expr("const_method()") + self.expect( + "expression volatile_method()", + error=True, + substrs=["has type 'const Foo'", "but function is not marked const"], + ) + + lldbutil.continue_to_source_breakpoint( + self, process, "Break here: volatile", lldb.SBFileSpec("main.cpp") + ) + + self.expect_expr( + "bar()", result_type="const char *", result_summary='"volatile_bar"' + ) + self.expect_expr("const_volatile_method()") + self.expect( + "expression const_method()", + error=True, + substrs=["has type 'volatile Foo'", "but function is not marked volatile"], + ) + self.expect_expr("volatile_method()") + + lldbutil.continue_to_source_breakpoint( + self, process, "Break here: const volatile", lldb.SBFileSpec("main.cpp") + ) + + self.expect_expr("bar()", result_type="int", result_value="2") + self.expect_expr("other_cv_method()") + + self.expect( + "expression const_method()", + error=True, + substrs=[ + "has type 'const volatile Foo'", + "but function is not marked const or volatile", + ], + ) + self.expect( + "expression volatile_method()", + error=True, + substrs=[ + "has type 'const volatile Foo'", + "but function is not marked const or volatile", + ], + ) diff --git a/lldb/test/API/lang/cpp/expression-context-qualifiers/const_volatile_method/main.cpp b/lldb/test/API/lang/cpp/expression-context-qualifiers/const_volatile_method/main.cpp new file mode 100644 index 0000000000000..da0f7e7d1be95 --- /dev/null +++ b/lldb/test/API/lang/cpp/expression-context-qualifiers/const_volatile_method/main.cpp @@ -0,0 +1,43 @@ +#include <cassert> +#include <cstdio> + +struct Foo { + double bar() const { return 5.0; } + const char *bar() volatile { return "volatile_bar"; } + int bar() volatile const { return 2; } + + int volatile_method() volatile { + std::puts("Break here: volatile"); + return 0; + } + int const_method() const { + std::puts("Break here: const"); + return 0; + } + int other_cv_method() const volatile { return 20; } + + int const_volatile_method() const volatile { + auto x = bar(); + assert(x == 2); + other_cv_method(); + + std::puts("Break here: const volatile"); + + return 120; + } +}; + +int main() { + const Foo f; + f.bar(); + f.const_method(); + + volatile Foo f2; + f2.bar(); + f2.volatile_method(); + + const volatile Foo f3; + f3.bar(); + + return Foo{}.const_volatile_method(); +} diff --git a/lldb/test/API/lang/cpp/expression-context-qualifiers/cv_qualified_objects/Makefile b/lldb/test/API/lang/cpp/expression-context-qualifiers/cv_qualified_objects/Makefile new file mode 100644 index 0000000000000..99998b20bcb05 --- /dev/null +++ b/lldb/test/API/lang/cpp/expression-context-qualifiers/cv_qualified_objects/Makefile @@ -0,0 +1,3 @@ +CXX_SOURCES := main.cpp + +include Makefile.rules diff --git a/lldb/test/API/lang/cpp/expression-context-qualifiers/cv_qualified_objects/TestExprOnCVQualifiedObjects.py b/lldb/test/API/lang/cpp/expression-context-qualifiers/cv_qualified_objects/TestExprOnCVQualifiedObjects.py new file mode 100644 index 0000000000000..0f661471df749 --- /dev/null +++ b/lldb/test/API/lang/cpp/expression-context-qualifiers/cv_qualified_objects/TestExprOnCVQualifiedObjects.py @@ -0,0 +1,21 @@ +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class TestCase(TestBase): + def test(self): + self.build() + lldbutil.run_to_source_breakpoint( + self, + "Break here", + lldb.SBFileSpec("main.cpp"), + ) + + self.expect_expr("f.bar()", result_type="double", result_value="5") + self.expect_expr("cf.bar()", result_type="int", result_value="2") + self.expect_expr("vf.bar()", result_type="short", result_value="8") + self.expect_expr( + "cvf.bar()", result_type="const char *", result_summary='"volatile"' + ) diff --git a/lldb/test/API/lang/cpp/expression-context-qualifiers/cv_qualified_objects/main.cpp b/lldb/test/API/lang/cpp/expression-context-qualifiers/cv_qualified_objects/main.cpp new file mode 100644 index 0000000000000..eb7f8b82f2695 --- /dev/null +++ b/lldb/test/API/lang/cpp/expression-context-qualifiers/cv_qualified_objects/main.cpp @@ -0,0 +1,25 @@ +#include <cstdio> + +struct Foo { + double bar() { return 5.0; } + int bar() const { return 2; } + short bar() volatile { return 8; } + char const *bar() const volatile { return "volatile"; } + + float m_mem = -2.0; + const float m_const_mem = -3.0; +}; + +int main() { + Foo f; + const Foo cf; + volatile Foo vf; + const volatile Foo cvf; + + f.bar(); + cf.bar(); + vf.bar(); + cvf.bar(); + + std::puts("Break here"); +} diff --git a/lldb/test/API/lang/cpp/expression-context-qualifiers/fixit/Makefile b/lldb/test/API/lang/cpp/expression-context-qualifiers/fixit/Makefile new file mode 100644 index 0000000000000..99998b20bcb05 --- /dev/null +++ b/lldb/test/API/lang/cpp/expression-context-qualifiers/fixit/Makefile @@ -0,0 +1,3 @@ +CXX_SOURCES := main.cpp + +include Makefile.rules diff --git a/lldb/test/API/lang/cpp/expression-context-qualifiers/fixit/TestExprInConstMethodWithFixit.py b/lldb/test/API/lang/cpp/expression-context-qualifiers/fixit/TestExprInConstMethodWithFixit.py new file mode 100644 index 0000000000000..3c239c89d0ca7 --- /dev/null +++ b/lldb/test/API/lang/cpp/expression-context-qualifiers/fixit/TestExprInConstMethodWithFixit.py @@ -0,0 +1,42 @@ +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class TestCase(TestBase): + def test(self): + self.build() + (_, process, _, _) = lldbutil.run_to_source_breakpoint( + self, "Break here", lldb.SBFileSpec("main.cpp") + ) + + self.expect( + "expression m_bar->method()", + error=True, + substrs=[ + "member reference type 'const Bar' is not a pointer", + "but function is not marked const", + ], + ) + + # Two fix-its + self.expect( + "expression -- m_ba... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/177922 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
