Handle typeof() expressions

[JDevlieghere]

Summary

Before this patch, LLDB was not able to evaluate expressions that
resulted in a value with a typeof-type.

(lldb) p int i; __typeof__(i) j = 1; j
(typeof (i)) $0 =

This fixes that. The type is still printed as (typeof (i)) but at least
we get a value now.

(lldb) p int i; __typeof__(i) j = 1; j
(typeof (i)) $0 = 1

I'm looking into printing this as int but will keep that for a follow-up commit either way.

[JDevlieghere]

Formatting

[labath]

Seems straight-forward enough, but technically Jim is the owner of the expression evaluator these days, so I'll leave the honours to him.

packages/Python/lldbsuite/test/expression_command/test/TestExprs.py

270

Are you sure this command is actually correct? The "p" here seems odd...

source/Symbol/ClangASTContext.cpp

5252

I'm guessing this means decltype doesn't work either ?

[davide]

Jonas, this looks a good use case for using lit.
Is it possible to reuse the machinery we use in lldb/lit/Expr ?
If not, well, we know there's something we can improve :)

[JDevlieghere]

Change to lit test

[JDevlieghere]

source/Symbol/ClangASTContext.cpp

5252

Indeed, I was thinking about doing this in a separate diff but since it's so similar I'll update this patch.

[davide]

lit/Expr/TestTypeOfExpr.test

2 ↗

(On Diff #134942)

I really really love how concise and clear the new test is!

[JDevlieghere]

Do this for decltype too

[JDevlieghere]

Make sure the lit test actually checks the lldb output instead of the # CHECK lines.

Without the check-next, CHECK matches the # CHECK lines, as they are printed by lldb:

(lldb) # RUN: %lldb -b -s %s | FileCheck %s
(lldb) expression int i; __typeof__(i) j = 1; j
(typeof (i)) $0 = 1
(lldb) # CHECK: (lldb) expression int i; __typeof__(i) j = 1; j
(lldb) # CHECK-NEXT: (typeof (i)) $0 = 1
(lldb) expression int i; typeof(i) j = 1; j
(typeof (i)) $1 = 1
(lldb) # CHECK: (lldb) expression int i; typeof(i) j = 1; j
(lldb) # CHECK-NEXT: (typeof (i)) $1 = 1
(lldb) expression int i; decltype(i) j = 1; j
(decltype(i)) $2 = 1
(lldb) # CHECK: (lldb) expression int i; decltype(i) j = 1; j
(lldb) # CHECK-NEXT: (decltype(i)) $2 = 1

[jingham]

The code part of this looks fine. I had a few quibbles with the test, see inline.

packages/Python/lldbsuite/test/expression_command/test/TestExprs.py

270

Yes, all the other commands in this test that run expressions either get the stopped frame and run SBFrame.EvaluateExpression, or they spell out "expression" completely. Unless you are actually testing that an alias works, it's better not to use aliases in tests.

Also, was there a reason why you didn't use TestEBasicExprCommandsTestCase.build_and_run? It looks like everything you do is done there. I was going to say you should make sure that run stopped at the breakpoint - otherwise if that fails you'll get some weird error from the expression that may be harder to diagnose. But then I noticed the build_and_run function which ALSO doesn't test that the run stopped at your breakpoint, but if all the tests used that then we could just add the check there.

[JDevlieghere]

In D43471#1012484, @jingham wrote:

The code part of this looks fine. I had a few quibbles with the test, see inline.

Jim, it looks like you're commenting on an older version of the diff. I've since switched to checking this with lit.

[jingham]

Oh, somehow my browser hadn't updated to show the lit test. Ignore the previous comment.

This is fine, except can you make the test check not depend on the particular result variable number ($0, $1, $2). You aren't testing here that the result variable numbers are monotonically increasing integers that start at zero, and it will make adding to the test annoying over time. This command output type of test is fragile because it depends on irrelevant details of command output. For instance TestCallStopAndContinue.test knows that the text we output for a stop is exactly "Execution was interrupted, reason: breakpoint" seems like more detail than a test should rely on. Similarly the exact format of the result variable seems more detail than this test needs.

[jingham]

Other than that it's fine.

[JDevlieghere]

Thanks for the review Jim! I've updated the test accordingly.

[jingham]

Looks good

[davide]

LGTM

[zturner]

One issue I can see with this is that if you check the documentation of -b it says this:

-b
--batch
     Tells the debugger to run the commands from -s, -S, -o & -O, and
     then quit.  However if any run command stopped due to a signal or
     crash, the debugger will return to the interactive prompt at the
     place of the crash.

So, this should work fine as long as none of these commands crash, but if they do crash it will cause lit to hang. It might be useful to add a -B option to LLDB that will *not* return to the interactive prompt in the case of a signal or crash, and instead will just terminate. This way we'd see the crash as just another form of test failure rather than a timeout.

[jingham]

Rather than adding another flag, can't you just put -o quit at the end of your RUN line?

[zturner]

If your commands are A, B, C, and D but A crashes and returns to the interactive prompt, will it still try to execute B, C, and D? If so then I guess that would work (I haven't tried). OTOH, there's a risk of people forgetting to do this (but I'm not sure if the risk would be higher than people using -b instead of -B).

[zturner]

Actually I may have misunderstood the help text. It sounds like it may be referring to a crash of the inferior, not a crash of LLDB itself. If the test contains no run commands (as this one doesn't), then it doesn't seem like there's any risk of this happening, and this is a non issue.

[jingham]

The point of --batch is that if the program crashes, control is transferred from the script to the user so that they can inspect the crash. lit would just hang at that point, right?

I worry that people will use this RUN example as an template, and then make a test that does cause the target to execute, which test would then hang up if the target crashed. So I still think ending with an explicit quit rather than using --batch is safer.

packages/Python/lldbsuite/test/expression_command/test/TestExprs.py

def test_expr_commands_can_handle_quotes(self):
         # runCmd: command alias print_hi expression (int) printf ("\n\tHi!\n")
         # output:
         self.runCmd(
             r'''command alias print_hi expression (int) printf ("\n\tHi!\n")''')
         # This fails currently.
         self.expect('print_hi',
                     substrs=['(int) $',
                              '6'])
+
+    def test_expr_commands_typeof(self):
+        """Throw typeof expression at lldb."""
+        self.build()
+
+        self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
+
+        lldbutil.run_break_set_by_file_and_line(
+            self, "main.cpp", self.line, num_expected_locations=1, loc_exact=False)
+
+        self.runCmd("run", RUN_SUCCEEDED)
+
+        # runCmd: expression p int i; __typeof__(i) j = 1; j
+        # output: (typeof (i)) $0 = 1
+        self.expect("expression p int i; __typeof__(i) j = 1; j",
+                    substrs=['(typeof (i)) $',
+                             "1"])

source/Symbol/ClangASTContext.cpp

ClangASTContext::GetTypeInfo(lldb::opaque_compiler_type_t type,
   case clang::Type::Typedef:
     return eTypeIsTypedef |
            CompilerType(getASTContext(),
                         llvm::cast<clang::TypedefType>(qual_type)
                             ->getDecl()
                             ->getUnderlyingType())
                .GetTypeInfo(pointee_or_element_clang_type);
   case clang::Type::TypeOfExpr:
-    return 0;
+    return CompilerType(getASTContext(),
+                        llvm::cast<clang::TypeOfExprType>(qual_type)->getUnderlyingExpr()->getType())
+        .GetTypeInfo(pointee_or_element_clang_type);
   case clang::Type::TypeOf:
-    return 0;
+    return CompilerType(getASTContext(),
+                        llvm::cast<clang::TypeOfType>(qual_type)->getUnderlyingType())
+        .GetTypeInfo(pointee_or_element_clang_type);
   case clang::Type::UnresolvedUsing:
     return 0;
 
   case clang::Type::ExtVector:
   case clang::Type::Vector: {
     uint32_t vector_type_flags = eTypeHasChildren | eTypeIsVector;
     const clang::VectorType *vector_type = llvm::dyn_cast<clang::VectorType>(
         qual_type->getCanonicalTypeInternal());
ClangASTContext::GetTypeClass(lldb::opaque_compiler_type_t type) {
   case clang::Type::DependentName:
     break;
   case clang::Type::DependentTemplateSpecialization:
     break;
   case clang::Type::PackExpansion:
     break;
 
   case clang::Type::TypeOfExpr:
-    break;
+    return CompilerType(getASTContext(),
+                        llvm::cast<clang::TypeOfExprType>(qual_type)->getUnderlyingExpr()->getType())
+        .GetTypeClass();
   case clang::Type::TypeOf:
-    break;
+    return CompilerType(getASTContext(),
+                        llvm::cast<clang::TypeOfType>(qual_type)->getUnderlyingType())
+        .GetTypeClass();
+
   case clang::Type::Decltype:
     break;
   case clang::Type::TemplateSpecialization:
     break;
   case clang::Type::DeducedTemplateSpecialization:
     break;
   case clang::Type::Atomic:
     break;
return CompilerType(
                getASTContext(),
                llvm::cast<clang::ElaboratedType>(qual_type)->getNamedType())
         .GetEncoding(count);
 
   case clang::Type::Paren:
     return CompilerType(getASTContext(),
                         llvm::cast<clang::ParenType>(qual_type)->desugar())
         .GetEncoding(count);
-
+  case clang::Type::TypeOfExpr:
+    return CompilerType(getASTContext(),
+                        llvm::cast<clang::TypeOfExprType>(qual_type)->getUnderlyingExpr()->getType())
+        .GetEncoding(count);
+  case clang::Type::TypeOf:
+    return CompilerType(getASTContext(),
+                        llvm::cast<clang::TypeOfType>(qual_type)->getUnderlyingType())
+        .GetEncoding(count);
   case clang::Type::DependentSizedArray:
   case clang::Type::DependentSizedExtVector:
   case clang::Type::UnresolvedUsing:
   case clang::Type::Attributed:
   case clang::Type::TemplateTypeParm:
   case clang::Type::SubstTemplateTypeParm:
   case clang::Type::SubstTemplateTypeParmPack:
   case clang::Type::InjectedClassName:
   case clang::Type::DependentName:
   case clang::Type::DependentTemplateSpecialization:
   case clang::Type::PackExpansion:
   case clang::Type::ObjCObject:
 
-  case clang::Type::TypeOfExpr:
-  case clang::Type::TypeOf:
   case clang::Type::Decltype:
   case clang::Type::TemplateSpecialization:
   case clang::Type::DeducedTemplateSpecialization:
   case clang::Type::Atomic:
   case clang::Type::Adjusted:
   case clang::Type::Pipe:
     break;
 
case clang::Type::Paren:
     return CompilerType(getASTContext(),
                         llvm::cast<clang::ParenType>(qual_type)->desugar())
         .GetFormat();
   case clang::Type::Elaborated:
     return CompilerType(
                getASTContext(),
                llvm::cast<clang::ElaboratedType>(qual_type)->getNamedType())
         .GetFormat();
+  case clang::Type::TypeOfExpr:
+    return CompilerType(getASTContext(),
+                        llvm::cast<clang::TypeOfExprType>(qual_type)->getUnderlyingExpr()->getType())
+        .GetFormat();
+  case clang::Type::TypeOf:
+    return CompilerType(getASTContext(),
+                        llvm::cast<clang::TypeOfType>(qual_type)->getUnderlyingType())
+        .GetFormat();
   case clang::Type::DependentSizedArray:
   case clang::Type::DependentSizedExtVector:
   case clang::Type::UnresolvedUsing:
   case clang::Type::Attributed:
   case clang::Type::TemplateTypeParm:
   case clang::Type::SubstTemplateTypeParm:
   case clang::Type::SubstTemplateTypeParmPack:
   case clang::Type::InjectedClassName:
   case clang::Type::DependentName:
   case clang::Type::DependentTemplateSpecialization:
   case clang::Type::PackExpansion:
   case clang::Type::ObjCObject:
 
-  case clang::Type::TypeOfExpr:
-  case clang::Type::TypeOf:
   case clang::Type::Decltype:
   case clang::Type::TemplateSpecialization:
   case clang::Type::DeducedTemplateSpecialization:
   case clang::Type::Atomic:
   case clang::Type::Adjusted:
   case clang::Type::Pipe:
     break;
 
   // pointer type decayed from an array or function type.
return GetNumPointeeChildren(llvm::cast<clang::TypedefType>(qual_type)
                                      ->getUnderlyingType());
   case clang::Type::Auto:
     return GetNumPointeeChildren(
         llvm::cast<clang::AutoType>(qual_type)->getDeducedType());
   case clang::Type::Elaborated:
     return GetNumPointeeChildren(
         llvm::cast<clang::ElaboratedType>(qual_type)->getNamedType());
   case clang::Type::TypeOfExpr:
-    return 0;
+    return GetNumPointeeChildren(
+        llvm::cast<clang::TypeOfExprType>(qual_type)->getUnderlyingExpr()->getType());
   case clang::Type::TypeOf:
-    return 0;
+    return GetNumPointeeChildren(
+        llvm::cast<clang::TypeOfType>(qual_type)->getUnderlyingType());
   case clang::Type::Decltype:
     return 0;
   case clang::Type::Record:
     return 0;
   case clang::Type::Enum:
     return 1;
   case clang::Type::TemplateTypeParm:
     return 1;
ClangASTContextForExpressions::GetPersistentExpressionState() {
   return m_persistent_variables.get();
 }
 
 clang::ExternalASTMerger &
 ClangASTContextForExpressions::GetMergerUnchecked() {
   lldbassert(m_scratch_ast_source_ap != nullptr);
   return m_scratch_ast_source_ap->GetMergerUnchecked();
 }
-