[Lldb-commits] [PATCH] D78329: Allow lldb-test to combine -find with -dump-clang-ast

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.


================
Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:8823
 
     case clang::Type::Typedef: {
       const clang::TypedefType *typedef_type =
----------------
shafik wrote:
> Does it make sense to apply these change to non-objective-C cases?
It does absolutely! I must admit I sort of forgot to update the other cases, because they weren't on my critical path. Done now.


================
Comment at: lldb/tools/lldb-test/lldb-test.cpp:726
   if (DumpClangAST) {
-    if (Find != FindType::None)
-      return make_string_error("Cannot both search and dump clang AST.");
-    if (Regex || !Context.empty() || !File.empty() || Line != 0)
-      return make_string_error(
-          "-regex, -context, -name, -file and -line options are not "
-          "applicable for dumping clang AST.");
-    return dumpClangAST;
+    if (Find == FindType::None) {
+      if (Regex || !Context.empty() || !File.empty() || Line != 0)
----------------
shafik wrote:
> The relationship between `DumpClangAST` and `Find` is not obvious. Basically we are falling through here in the `FindType::Type` case.
> 
> Perhaps a comment here and maybe on the ` Map.Dump(&Stream, false, GetDescriptionLevel());` line might help clarify how these options are related.
I added the explanation to the documentation of the -dump-clang-ast option.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78329/new/

https://reviews.llvm.org/D78329