[clang] Extend pragma dump to support expressions

[Endill]

Summary

Extend #pragma clang __debug dump to support not only single identifier, but an expression as well. This makes it possible to test ADL and overload resolution directly, without being creative to make them observable via diagnostics (e.g. when over.match.best is involved). This implementation has a known limitation of not supporting dependent expressions properly, but it's quite useful even without such support.

[aaron.ballman]

Thanks for this! You should add test coverage for the changes, especially around things like dependent expressions, ADL use, etc. Also, the changes need a release note and the new functionality should be explicitly documented.

clang/lib/Parse/ParsePragma.cpp

723–727

I think you should have another variant of ActOnPragmaDump that does the actual dumping, instead of doing it from the parser. I think we should not try to dump a dependent expression (to support those, I think we need to create a new AST node for the pragma so that we can transform it from TreeTransform.h and perform the dump on the non-dependent expression -- not certain if we want to handle that now, or if we want it to be an error to use this pragma with a dependent expression).

[Endill]

Thank you for reviewing this!

In D144115#4147440, @aaron.ballman wrote:

You should add test coverage for the changes, especially around things like dependent expressions, ADL use, etc. Also, the changes need a release note and the new functionality should be explicitly documented.

Despite having a dozen of #pragma __debug directives, none of them are tested neither mentioned in any kind of documentation, including release notes. Are you sure about this? If so, what is the right place to put them into?

clang/lib/Parse/ParsePragma.cpp

723–727

I think you should have another variant of ActOnPragmaDump that does the actual dumping, instead of doing it from the parser.

Would it be fine to do it like this: Actions.ActOnPragmaDump(E), where E is the result of ParseExpression()?

I think we should not try to dump a dependent expression (to support those, I think we need to create a new AST node for the pragma so that we can transform it from TreeTransform.h and perform the dump on the non-dependent expression -- not certain if we want to handle that now, or if we want it to be an error to use this pragma with a dependent expression).

As I mentioned in summary, dependent expressions are out of scope of this patch. How can I test whether expression is dependent?

[aaron.ballman]

In D144115#4148026, @Endill wrote:

Despite having a dozen of #pragma __debug directives, none of them are tested neither mentioned in any kind of documentation, including release notes. Are you sure about this? If so, what is the right place to put them into?

Yeah, we've been very bad about documenting our implementation-defined behaviors and I've been on a push to change that moving forward for about the past year or so. You don't have to document all of the debug pragmas by any means, but getting some basic documentation up for the pragma being worked on is good incremental progress. I'd recommend adding the tests to clang/test/Preprocessor and adding the documentation to clang\docs\LanguageExtensions.rst.

clang/lib/Parse/ParsePragma.cpp

723–727

I think you should have another variant of ActOnPragmaDump that does the actual dumping, instead of doing it from the parser.

Would it be fine to do it like this: Actions.ActOnPragmaDump(E), where E is the result of ParseExpression()?

I think it'd make sense to do Actions.ActOnPragmaDump(Tok.getLocation(), ParseExpression()); so that we have the location of the pragma token itself in addition to the expression result for the argument.

As I mentioned in summary, dependent expressions are out of scope of this patch. How can I test whether expression is dependent?

Expr::isValueDependent() and Expr::isTypeDependent()

[Endill]

• Diagnose dependent expressions
• Move AST dumping from parser to sema
• Add documentation
• Add tests

[Endill]

ping

[aaron.ballman]

You should also add a release note to clang/docs/ReleaseNotes.rst so users know about the new functionality.

clang/docs/LanguageExtensions.rst

4926

4930–4937

clang/include/clang/Basic/DiagnosticLexKinds.td

658–661

We can use a select here so we only need one new diagnostic.

This should be moved to DiagnosticParseKinds.td because it's emitted from the parser, not the lexer.

clang/lib/Parse/ParsePragma.cpp

15–16

730–735

clang/test/AST/ast-dump-lookups.cpp

101

You should add the newline back to the end of the file.

I'd also like to see a test case where the expression has errors, like:

#pragma clang __debug dump this is nonsense

and where the expression causes an instantiation:

template <typename T>
struct S {
  static constexpr const T *str = "string";
};

template <>
struct S<wchar_t> {
  static constexpr const wchar_t *str = L"wide string";
};

void func() {
  #pragma clang __debug dump S<wchar_t>::str;
}

[Endill]

Thank you for review!

clang/lib/Parse/ParsePragma.cpp

15–16

I'm using warn_pragma_debug_unexpected_argument and warn_pragma_debug_missing_argument from there as well, and not just added diagnostics. What should I do about that? Copying them over to DiagnosticParseKinds.td doesn't work.

730–735

I see quite a lot of items inside ExprDependence. Are you sure we don't miss any corner cases by using isValueDependent() to select between value- and type-dependent diagnostic text?

[aaron.ballman]

clang/lib/Parse/ParsePragma.cpp

15–16

Ah, those should move into DiagnosticCommonKinds.td under the // Parse && Lex heading, then they should be available from here as well without the extra include.

730–735

I was trying to think of a case where we'd run into problems, but I can't come up with one, so I believe we're fine here.

[Endill]

clang/lib/Parse/ParsePragma.cpp

730–735

Apparently, we should do it the other way round, because An id-expression is value-dependent if: — it is type-dependent (temp.dep.constexpr/2.2). I observe that in one of the tests I added, T{} is TypeValueInstantiation.

[Endill]

• Handle expressions as unevaluated operands
• Numerous fixes to documentation wording
• Add release notes entry
• Prevent generic "unknown argument" diagnostic from being issued when other more specific diagnostics have been emitted
• Add more tests

[aaron.ballman]

LGTM!

[Endill]

Rebase

clang/docs/LanguageExtensions.rst

 * ``__rord``
 * ``__rorq``
 * ``_rotl``
 * ``_rotr``
 * ``_rotwl``
 * ``_rotwr``
 * ``_lrotl``
 * ``_lrotr``
+
+Debugging the Compiler
+======================
+
+Clang supports a number of pragma directives that help debugging the compiler itself.
+Syntax is the following: `#pragma clang __debug <command> <arguments>`.
+Note, all of debugging pragmas are subject to change.
+
+`dump`
+------
+Accepts either a single identifier or an expression. When a single identifier is passed,
+the lookup results for the identifier are printed to `stderr`. When an expression is passed, 
+the AST for the expression is printed to `stderr`. The expression is an unevaluated operand,
+so things like overload resolution and template instantiations are performed,
+but the expression has no runtime effects.
+Type- and value-dependent expressions are not supported yet.
+
+This facility is designed to aid with testing name lookup machinery.

clang/docs/ReleaseNotes.rst

- Clang now saves the address of ABI-indirect function parameters on the stack,
   optimizations.
 - Clang now supports ``__builtin_nondeterministic_value`` that returns a
   nondeterministic value of the same type as the provided argument.
 - Clang now supports ``__builtin_FILE_NAME()`` which returns the same
   information as the ``__FILE_NAME__`` macro (the presumed file name
   from the invocation point, with no path components included).
 - Clang now supports ``__builtin_assume_separate_storage`` that indicates that
   its arguments point to objects in separate storage allocations.
+- Clang now supports expressions in ``#pragma clang __debug dump``.
 
 New Compiler Flags
 ------------------
 
 Deprecated Compiler Flags
 -------------------------
 
 Modified Compiler Flags

clang/include/clang/Basic/DiagnosticCommonKinds.td

"%select{in %1|for diagnostic message in static_assert|"
           "for %select{language name|source container name|USR}1 in "
           "'external_source_symbol' attribute}0">;
 def err_invalid_string_udl : Error<
   "string literal with user-defined suffix cannot be used here">;
 def err_invalid_character_udl : Error<
   "character literal with user-defined suffix cannot be used here">;
 def err_invalid_numeric_udl : Error<
   "numeric literal with user-defined suffix cannot be used here">;
+def warn_pragma_debug_missing_argument : Warning<
+  "missing argument to debug command '%0'">, InGroup<IgnoredPragmas>;
+def warn_pragma_debug_unexpected_argument : Warning<
+  "unexpected argument to debug command">, InGroup<IgnoredPragmas>;
 
 }
 
 // Parse && Sema
 
 let CategoryName = "Parse Issue" in {
 
 def err_expected : Error<"expected %0">;

clang/include/clang/Basic/DiagnosticLexKinds.td

 def warn_pragma_diagnostic_unknown_warning :
    ExtWarn<"unknown warning group '%0', ignored">,
    InGroup<UnknownWarningOption>;
 // - #pragma __debug
 def warn_pragma_debug_missing_command : Warning<
   "missing debug command">, InGroup<IgnoredPragmas>;
 def warn_pragma_debug_unexpected_command : Warning<
   "unexpected debug command '%0'">, InGroup<IgnoredPragmas>;
-def warn_pragma_debug_missing_argument : Warning<
-  "missing argument to debug command '%0'">, InGroup<IgnoredPragmas>;
-def warn_pragma_debug_unexpected_argument : Warning<
-  "unexpected argument to debug command">, InGroup<IgnoredPragmas>;
 def warn_pragma_debug_unknown_module : Warning<
   "unknown module '%0'">, InGroup<IgnoredPragmas>;
 // #pragma module
 def err_pp_expected_module_name : Error<
   "expected %select{identifier after '.' in |}0module name">;
 def err_pp_module_begin_wrong_module : Error<
   "must specify '-fmodule-name=%0' to enter %select{|submodule of }1"
   "this module%select{ (current module is %3)|}2">;
 def err_pp_module_begin_no_module_map : Error<
   "no module map available for module %0">;
 def err_pp_module_begin_no_submodule : Error<
   "submodule %0.%1 not declared in module map">;

clang/include/clang/Basic/DiagnosticParseKinds.td

 def err_pragma_attribute_namespace_on_attribute : Error<
   "namespace can only apply to 'push' or 'pop' directives">;
 def note_pragma_attribute_namespace_on_attribute : Note<
   "omit the namespace to add attributes to the most-recently"
   " pushed attribute group">;
 def warn_no_support_for_eval_method_source_on_m32 : Warning<
   "Setting the floating point evaluation method to `source` on a target"
   " without SSE is not supported.">, InGroup<Pragmas>;
+// - #pragma __debug
+def warn_pragma_debug_dependent_argument : Warning<
+  "%select{value|type}0-dependent expression passed as an argument to debug "
+  "command">, InGroup<IgnoredPragmas>;
 
 // OpenCL EXTENSION pragma (OpenCL 1.1 [9.1])
 def warn_pragma_expected_colon : Warning<
   "missing ':' after %0 - ignoring">, InGroup<IgnoredPragmas>;
 def warn_pragma_expected_predicate : Warning<
   "expected %select{'enable', 'disable', 'begin' or 'end'|'disable'}0 - ignoring">, InGroup<IgnoredPragmas>;
 def warn_pragma_unknown_extension : Warning<
   "OpenCL extension %0 unknown or does not require pragma - ignoring">, InGroup<IgnoredPragmas>;

clang/include/clang/Sema/Sema.h

   void ActOnPragmaMSAllocText(
       SourceLocation PragmaLocation, StringRef Section,
       const SmallVector<std::tuple<IdentifierInfo *, SourceLocation>>
           &Functions);
 
   /// Called on #pragma clang __debug dump II
   void ActOnPragmaDump(Scope *S, SourceLocation Loc, IdentifierInfo *II);
 
+  /// Called on #pragma clang __debug dump E
+  void ActOnPragmaDump(Expr *E);
+
   /// ActOnPragmaDetectMismatch - Call on well-formed \#pragma detect_mismatch
   void ActOnPragmaDetectMismatch(SourceLocation Loc, StringRef Name,
                                  StringRef Value);
 
   /// Are precise floating point semantics currently enabled?
   bool isPreciseFPEnabled() {
     return !CurFPFeatures.getAllowFPReassociate() &&
            !CurFPFeatures.getNoSignedZero() &&

clang/lib/Lex/Pragma.cpp

if (II->isStr("assert")) {
       if (!PP.getPreprocessorOpts().DisablePragmaDebugCrash) {
         Token Crasher;
         Crasher.startToken();
         Crasher.setKind(tok::annot_pragma_parser_crash);
         Crasher.setAnnotationRange(SourceRange(Tok.getLocation()));
         PP.EnterToken(Crasher, /*IsReinject*/ false);
       }
     } else if (II->isStr("dump")) {
-      Token Identifier;
-      PP.LexUnexpandedToken(Identifier);
-      if (auto *DumpII = Identifier.getIdentifierInfo()) {
-        Token DumpAnnot;
-        DumpAnnot.startToken();
-        DumpAnnot.setKind(tok::annot_pragma_dump);
-        DumpAnnot.setAnnotationRange(
-            SourceRange(Tok.getLocation(), Identifier.getLocation()));
-        DumpAnnot.setAnnotationValue(DumpII);
-        PP.DiscardUntilEndOfDirective();
-        PP.EnterToken(DumpAnnot, /*IsReinject*/false);
-      } else {
-        PP.Diag(Identifier, diag::warn_pragma_debug_missing_argument)
-            << II->getName();
-      }
+      DumpAnnot.setAnnotationRange(SourceRange(Tok.getLocation()));
     } else if (II->isStr("diag_mapping")) {
       Token DiagName;
       PP.LexUnexpandedToken(DiagName);
       if (DiagName.is(tok::eod))
         PP.getDiagnostics().dump();
       else if (DiagName.is(tok::string_literal) && !DiagName.hasUDSuffix()) {
         StringLiteralParser Literal(DiagName, PP);
         if (Literal.hadError)

clang/lib/Parse/ParsePragma.cpp

 //===--- ParsePragma.cpp - Language specific pragma parsing ---------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the language specific #pragma handlers.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/AST/ASTContext.h"
 #include "clang/Basic/PragmaKinds.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/Token.h"
 #include "clang/Parse/LoopHint.h"
 #include "clang/Parse/ParseDiagnostic.h"
 #include "clang/Parse/Parser.h"
 #include "clang/Parse/RAIIObjectsForParser.h"
 #include "clang/Sema/Scope.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringSwitch.h"
void Parser::HandlePragmaAlign() {
   Actions.ActOnPragmaOptionsAlign(Kind, Tok.getLocation());
   // Consume the token after processing the pragma to enable pragma-specific
   // #include warnings.
   ConsumeAnnotationToken();
 }
 
 void Parser::HandlePragmaDump() {
   assert(Tok.is(tok::annot_pragma_dump));
-  IdentifierInfo *II =
-      reinterpret_cast<IdentifierInfo *>(Tok.getAnnotationValue());
-  Actions.ActOnPragmaDump(getCurScope(), Tok.getLocation(), II);
   ConsumeAnnotationToken();
+  if (Tok.is(tok::eod)) {
+    PP.Diag(Tok, diag::warn_pragma_debug_missing_argument) << "dump";
+  } else if (NextToken().is(tok::eod)) {
+    if (Tok.isNot(tok::identifier)) {
+      PP.Diag(Tok, diag::warn_pragma_debug_unexpected_argument);
+      ConsumeAnyToken();
+      ExpectAndConsume(tok::eod);
+      return;
+    }
+    IdentifierInfo *II = Tok.getIdentifierInfo();
+    Actions.ActOnPragmaDump(getCurScope(), Tok.getLocation(), II);
+    ConsumeToken();
+  } else {
+    SourceLocation StartLoc = Tok.getLocation();
+    EnterExpressionEvaluationContext Ctx(
+      Actions, Sema::ExpressionEvaluationContext::Unevaluated);
+    ExprResult E = ParseExpression();
+    if (!E.isUsable() || E.get()->containsErrors()) {
+      // Diagnostics were emitted during parsing. No action needed.
+    } else if (E.get()->getDependence() != ExprDependence::None) {
+      PP.Diag(StartLoc, diag::warn_pragma_debug_dependent_argument)
+        << E.get()->isTypeDependent()
+        << SourceRange(StartLoc, Tok.getLocation());
+    } else {
+      Actions.ActOnPragmaDump(E.get());
+    }
+    SkipUntil(tok::eod, StopBeforeMatch);
+  }
+  ExpectAndConsume(tok::eod);
 }
 
 void Parser::HandlePragmaWeak() {
   assert(Tok.is(tok::annot_pragma_weak));
   SourceLocation PragmaLoc = ConsumeAnnotationToken();
   Actions.ActOnPragmaWeakID(Tok.getIdentifierInfo(), PragmaLoc,
                             Tok.getLocation());
   ConsumeToken(); // The weak name.

clang/lib/Sema/SemaLookup.cpp

 void Sema::ActOnPragmaDump(Scope *S, SourceLocation IILoc, IdentifierInfo *II) {
   DeclarationNameInfo Name(II, IILoc);
   LookupResult R(*this, Name, LookupAnyName, Sema::NotForRedeclaration);
   R.suppressDiagnostics();
   R.setHideTags(false);
   LookupName(R, S);
   R.dump();
 }
+
+void Sema::ActOnPragmaDump(Expr *E) {
+  E->dump();
+}

clang/test/AST/ast-dump-lookups.cpp

 // RUN: %clang_cc1 -std=c++11 -ast-dump -ast-dump-filter Test %s | FileCheck -check-prefix DECLS %s
 // RUN: %clang_cc1 -std=c++11 -ast-dump-lookups -ast-dump-filter Test %s | FileCheck -check-prefix LOOKUPS %s
 // RUN: %clang_cc1 -std=c++11 -ast-dump -ast-dump-lookups -ast-dump-filter Test %s | FileCheck -check-prefix DECLS-LOOKUPS %s
-// RUN: %clang_cc1 -std=c++11 -DPRAGMA -fsyntax-only %s 2>&1 | FileCheck -check-prefix PRAGMA %s
+// RUN: %clang_cc1 -std=c++11 -DPRAGMA -fsyntax-only -verify %s 2>&1 | FileCheck -check-prefix PRAGMA %s
 
 namespace Test {
   typedef int T;
   extern int a;
   int a = 0;
 }
 
 #ifdef PRAGMA

 // DECLS-LOOKUPS:       -DeclarationName 'a'
 // DECLS-LOOKUPS-NEXT:   `-Var [[A:[^ ]*]] 'a' 'int'
 // DECLS-LOOKUPS-NEXT:     |-VarDecl [[EXTERN_A:0x[^ ]*]] {{.*}} a 'int' extern
 // DECLS-LOOKUPS-NEXT:     `-VarDecl [[A]] prev [[EXTERN_A]] {{.*}} a 'int' cinit
 // DECLS-LOOKUPS-NEXT:       `-IntegerLiteral {{.*}} 'int' 0
 //
 // DECLS-LOOKUPS: Dumping Test:
 // DECLS-LOOKUPS-NEXT: Lookup map is in primary DeclContext
+
+#ifdef PRAGMA
+namespace Test {
+  struct S {
+    const S& operator+(const S&) { return *this; }
+  };
+  void foo(S) {}
+}
+
+#pragma clang __debug dump foo(Test::S{})
+// PRAGMA: CallExpr {{.*}} adl
+// PRAGMA-NEXT: |-ImplicitCastExpr {{.*}}
+// PRAGMA-NEXT: | `-DeclRefExpr {{.*}} 'void (S)' lvalue Function {{.*}} 'foo' 'void (S)'
+
+#pragma clang __debug dump Test::S{} + Test::S{}
+// PRAGMA: CXXOperatorCallExpr {{.*}}
+// PRAGMA-NEXT: |-ImplicitCastExpr {{.*}}
+// PRAGMA-NEXT: | `-DeclRefExpr {{.*}} 'const S &(const S &)' lvalue CXXMethod {{.*}} 'operator+' 'const S &(const S &)'
+
+#pragma clang __debug dump &Test::S::operator+
+// PRAGMA: UnaryOperator {{.*}}
+// PRAGMA-NEXT: `-DeclRefExpr {{.*}} 'const S &(const S &)' CXXMethod {{.*}} 'operator+' 'const S &(const S &)'
+
+template<typename T, int I>
+void bar() {
+#pragma clang __debug dump T{} // expected-warning {{type-dependent expression}}
+#pragma clang __debug dump +I  // expected-warning {{value-dependent expression}}
+}
+
+template <typename T>
+struct S {
+  static constexpr const T *str = "string";
+};
+
+template <>
+struct S<wchar_t> {
+  static constexpr const wchar_t *str = L"wide string";
+};
+
+void func() {
+  #pragma clang __debug dump S<wchar_t>::str;
+  // PRAGMA: DeclRefExpr {{.*}} 'const wchar_t *const' lvalue Var {{.*}} 'str' 'const wchar_t *const'
+}
+
+#pragma clang __debug dump this is nonsense // expected-error {{invalid use of 'this'}}
+
+#endif