From fc64644401c0498a5b00427c0860b967f8a308ba Mon Sep 17 00:00:00 2001 From: Vassil Vassilev Date: Tue, 6 Aug 2024 11:17:49 +0000 Subject: [PATCH] Add location information and improve clarity of diagnostics. This patch is a first step towards diagnostics refactoring in the context of non-differentiable propagators. --- include/clad/Differentiator/VisitorBase.h | 15 +++----- lib/Differentiator/BaseForwardModeVisitor.cpp | 3 +- lib/Differentiator/DerivativeBuilder.cpp | 9 +---- .../ReverseModeForwPassVisitor.cpp | 1 - lib/Differentiator/ReverseModeVisitor.cpp | 5 +-- lib/Differentiator/VisitorBase.cpp | 36 +++++++++++-------- test/Gradient/NonDifferentiableError.C | 7 ++++ test/NumericalDiff/GradientMultiArg.C | 8 ++--- test/NumericalDiff/NoNumDiff.C | 11 +++--- test/NumericalDiff/NumDiff.C | 15 ++++---- test/NumericalDiff/PrintErrorNumDiff.C | 8 ++--- 11 files changed, 57 insertions(+), 61 deletions(-) diff --git a/include/clad/Differentiator/VisitorBase.h b/include/clad/Differentiator/VisitorBase.h index b68318f7b..c3759e2d4 100644 --- a/include/clad/Differentiator/VisitorBase.h +++ b/include/clad/Differentiator/VisitorBase.h @@ -394,14 +394,12 @@ namespace clad { /// to avoid recomputation. static bool UsefulToStore(clang::Expr* E); /// A flag for silencing warnings/errors output by diag function. - bool silenceDiags = false; /// Shorthand to issues a warning or error. template void diag(clang::DiagnosticsEngine::Level level, // Warning or Error clang::SourceLocation loc, const char (&format)[N], llvm::ArrayRef args = {}) { - if (!silenceDiags) - m_Builder.diag(level, loc, format, args); + m_Builder.diag(level, loc, format, args); } /// Creates unique identifier of the form "_nameBase" that is @@ -584,17 +582,14 @@ namespace clad { clang::Expr* GetSingleArgCentralDiffCall( clang::Expr* targetFuncCall, clang::Expr* targetArg, unsigned targetPos, unsigned numArgs, llvm::SmallVectorImpl& args); + /// Emits diagnostic messages on differentiation (or lack thereof) for /// call expressions. /// - /// \param[in] \c funcName The name of the underlying function of the - /// call expression. + /// \param[in] \c FD - The function declaration. /// \param[in] \c srcLoc Any associated source location information. - /// \param[in] \c isDerived A flag to determine if differentiation of the - /// call expression was successful. - void CallExprDiffDiagnostics(llvm::StringRef funcName, - clang::SourceLocation srcLoc, - bool isDerived); + void CallExprDiffDiagnostics(const clang::FunctionDecl* FD, + clang::SourceLocation srcLoc); clang::QualType DetermineCladArrayValueType(clang::QualType T); diff --git a/lib/Differentiator/BaseForwardModeVisitor.cpp b/lib/Differentiator/BaseForwardModeVisitor.cpp index c53757261..7a70f3bcb 100644 --- a/lib/Differentiator/BaseForwardModeVisitor.cpp +++ b/lib/Differentiator/BaseForwardModeVisitor.cpp @@ -63,7 +63,6 @@ DerivativeAndOverload BaseForwardModeVisitor::Derive(const FunctionDecl* FD, const DiffRequest& request) { assert(m_DiffReq == request && "Can't pass two different requests!"); - silenceDiags = !request.VerboseDiags; m_Functor = request.Functor; assert(m_DiffReq.Mode == DiffMode::forward); assert(!m_DerivativeInFlight && @@ -1331,7 +1330,7 @@ StmtDiff BaseForwardModeVisitor::VisitCallExpr(const CallExpr* CE) { GetSingleArgCentralDiffCall(fnCallee, CallArgs[0], /*targetPos=*/0, /*numArgs=*/1, CallArgs); } - CallExprDiffDiagnostics(FD->getNameAsString(), CE->getBeginLoc(), callDiff); + CallExprDiffDiagnostics(FD, CE->getBeginLoc()); if (!callDiff) { auto zero = ConstantFolder::synthesizeLiteral(m_Context.IntTy, m_Context, 0); diff --git a/lib/Differentiator/DerivativeBuilder.cpp b/lib/Differentiator/DerivativeBuilder.cpp index a7c657eaa..a4defc87e 100644 --- a/lib/Differentiator/DerivativeBuilder.cpp +++ b/lib/Differentiator/DerivativeBuilder.cpp @@ -194,15 +194,8 @@ static void registerDerivative(FunctionDecl* derivedFD, Sema& semaRef) { } if (!NSD) { NSD = utils::LookupNSD(m_Sema, namespaceID, namespaceShouldExist); - if (!forCustomDerv && !NSD) { - diag(DiagnosticsEngine::Warning, noLoc, - "Numerical differentiation is diabled using the " - "-DCLAD_NO_NUM_DIFF " - "flag, this means that every try to numerically differentiate a " - "function will fail! Remove the flag to revert to default " - "behaviour."); + if (!NSD) return R; - } } DeclContext* DC = NSD; diff --git a/lib/Differentiator/ReverseModeForwPassVisitor.cpp b/lib/Differentiator/ReverseModeForwPassVisitor.cpp index 9d892868f..6cf306f2f 100644 --- a/lib/Differentiator/ReverseModeForwPassVisitor.cpp +++ b/lib/Differentiator/ReverseModeForwPassVisitor.cpp @@ -20,7 +20,6 @@ DerivativeAndOverload ReverseModeForwPassVisitor::Derive(const FunctionDecl* FD, const DiffRequest& request) { assert(m_DiffReq == request); - silenceDiags = !request.VerboseDiags; assert(m_DiffReq.Mode == DiffMode::reverse_mode_forward_pass); diff --git a/lib/Differentiator/ReverseModeVisitor.cpp b/lib/Differentiator/ReverseModeVisitor.cpp index 38a7fbe39..97c78d519 100644 --- a/lib/Differentiator/ReverseModeVisitor.cpp +++ b/lib/Differentiator/ReverseModeVisitor.cpp @@ -256,7 +256,6 @@ Expr* getArraySizeExpr(const ArrayType* AT, ASTContext& context, const DiffRequest& request) { if (m_ExternalSource) m_ExternalSource->ActOnStartOfDerive(); - silenceDiags = !request.VerboseDiags; assert(m_DiffReq == request); // FIXME: reverse mode plugins may have request mode other than @@ -479,7 +478,6 @@ Expr* getArraySizeExpr(const ArrayType* AT, ASTContext& context, // for the two 'Derive's being different functions. if (m_ExternalSource) m_ExternalSource->ActOnStartOfDerive(); - silenceDiags = !request.VerboseDiags; // FIXME: We should not use const_cast to get the decl request here. // NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast) const_cast(m_DiffReq) = request; @@ -1943,8 +1941,7 @@ Expr* getArraySizeExpr(const ArrayType* AT, ASTContext& context, CE->getNumArgs(), dfdx(), PreCallStmts, PostCallStmts, DerivedCallArgs, CallArgDx); } - CallExprDiffDiagnostics(FD->getNameAsString(), CE->getBeginLoc(), - OverloadedDerivedFn); + CallExprDiffDiagnostics(FD, CE->getBeginLoc()); if (!OverloadedDerivedFn) { Stmts& block = getCurrentBlock(direction::reverse); block.insert(block.begin(), PreCallStmts.begin(), diff --git a/lib/Differentiator/VisitorBase.cpp b/lib/Differentiator/VisitorBase.cpp index 7c4b0abb1..aacfad9cb 100644 --- a/lib/Differentiator/VisitorBase.cpp +++ b/lib/Differentiator/VisitorBase.cpp @@ -17,6 +17,7 @@ #include "clang/AST/ASTContext.h" #include "clang/AST/Expr.h" #include "clang/AST/TemplateBase.h" +#include "clang/Lex/Preprocessor.h" #include "clang/Sema/Lookup.h" #include "clang/Sema/Overload.h" #include "clang/Sema/Scope.h" @@ -733,23 +734,28 @@ namespace clad { /*namespaceShouldExist=*/false); } - void VisitorBase::CallExprDiffDiagnostics(llvm::StringRef funcName, - SourceLocation srcLoc, bool isDerived){ - if (!isDerived) { - // Function was not derived => issue a warning. - diag(DiagnosticsEngine::Warning, - srcLoc, - "function '%0' was not differentiated because clad failed to " - "differentiate it and no suitable overload was found in " - "namespace 'custom_derivatives', and function may not be " - "eligible for numerical differentiation.", + void VisitorBase::CallExprDiffDiagnostics(const clang::FunctionDecl* FD, + SourceLocation srcLoc) { + bool NumDiffEnabled = + !m_Sema.getPreprocessor().isMacroDefined("CLAD_NO_NUM_DIFF"); + // FIXME: Switch to the real diagnostics engine and pass FD directly. + std::string funcName = FD->getNameAsString(); + diag(DiagnosticsEngine::Warning, srcLoc, + "function '%0' was not differentiated because clad failed to " + "differentiate it and no suitable overload was found in " + "namespace 'custom_derivatives'", + {funcName}); + if (NumDiffEnabled) { + diag(DiagnosticsEngine::Note, srcLoc, + "falling back to numerical differentiation for '%0' since no " + "suitable overload was found and clad could not derive it; " + "to disable this feature, compile your programs with " + "-DCLAD_NO_NUM_DIFF", {funcName}); } else { - diag(DiagnosticsEngine::Warning, noLoc, - "Falling back to numerical differentiation for '%0' since no " - "suitable overload was found and clad could not derive it. " - "To disable this feature, compile your programs with " - "-DCLAD_NO_NUM_DIFF.", + diag(DiagnosticsEngine::Note, srcLoc, + "fallback to numerical differentiation is disabled by the " + "'CLAD_NO_NUM_DIFF' macro; considering '%0' as 0", {funcName}); } } diff --git a/test/Gradient/NonDifferentiableError.C b/test/Gradient/NonDifferentiableError.C index 501c16268..83558d077 100644 --- a/test/Gradient/NonDifferentiableError.C +++ b/test/Gradient/NonDifferentiableError.C @@ -29,6 +29,12 @@ non_differentiable double fn_s2_mem_fn(double i, double j) { return obj.mem_fn(i, j) + i * j; } +double no_body(double x); + +double fn1(double x) { return no_body(x); } //expected-warning {{function 'no_body' was not differentiated}} +//expected-note@34 {{fallback to numerical differentiation is disabled}} +double fn2(double x) { return fn1(x); } + #define INIT_EXPR(classname) \ classname expr_1(2, 3); \ classname expr_2(3, 5); @@ -48,4 +54,5 @@ int main() { INIT_EXPR(SimpleFunctions2); TEST_CLASS(SimpleFunctions2, mem_fn, 3, 5); TEST_FUNC(fn_s2_mem_fn, 3, 5); // expected-error {{attempted differentiation of function 'fn_s2_mem_fn', which is marked as non-differentiable}} + auto fn2_grad = clad::gradient(fn2); } diff --git a/test/NumericalDiff/GradientMultiArg.C b/test/NumericalDiff/GradientMultiArg.C index 840dc7fb0..721620c8c 100644 --- a/test/NumericalDiff/GradientMultiArg.C +++ b/test/NumericalDiff/GradientMultiArg.C @@ -1,6 +1,6 @@ -// RUN: %cladnumdiffclang %s -I%S/../../include -oGradientMultiArg.out 2>&1 | FileCheck -check-prefix=CHECK %s +// RUN: %cladnumdiffclang %s -I%S/../../include -oGradientMultiArg.out -Xclang -verify 2>&1 | FileCheck -check-prefix=CHECK %s // RUN: ./GradientMultiArg.out | %filecheck_exec %s -// RUN: %cladnumdiffclang -Xclang -plugin-arg-clad -Xclang -enable-tbr %s -I%S/../../include -oGradientMultiArg.out +// RUN: %cladnumdiffclang -Xclang -plugin-arg-clad -Xclang -enable-tbr %s -I%S/../../include -oGradientMultiArg.out -Xclang -verify // RUN: ./GradientMultiArg.out | %filecheck_exec %s //CHECK-NOT: {{.*error|warning|note:.*}} @@ -11,9 +11,9 @@ #include double test_1(double x, double y){ - return std::hypot(x, y); + return std::hypot(x, y); // expected-warning {{function 'hypot' was not differentiated}} + // expected-note@14 {{falling back to numerical differentiation}} } -// CHECK: warning: Falling back to numerical differentiation for 'hypot' since no suitable overload was found and clad could not derive it. To disable this feature, compile your programs with -DCLAD_NO_NUM_DIFF. // CHECK: void test_1_grad(double x, double y, double *_d_x, double *_d_y) { // CHECK-NEXT: { // CHECK-NEXT: double _r0 = 0; diff --git a/test/NumericalDiff/NoNumDiff.C b/test/NumericalDiff/NoNumDiff.C index 2dcf0a618..811efb765 100644 --- a/test/NumericalDiff/NoNumDiff.C +++ b/test/NumericalDiff/NoNumDiff.C @@ -1,4 +1,4 @@ -// RUN: %cladclang %s -I%S/../../include -oNoNumDiff.out 2>&1 | FileCheck -check-prefix=CHECK %s +// RUN: %cladclang %s -I%S/../../include -oNoNumDiff.out -Xclang -verify 2>&1 | FileCheck -check-prefix=CHECK %s //CHECK-NOT: {{.*error|warning|note:.*}} @@ -6,10 +6,9 @@ #include -double func(double x) { return std::tanh(x); } +double func(double x) { return std::tanh(x); } // expected-warning 2{{function 'tanh' was not differentiated because clad failed to differentiate it and no suitable overload was found in namespace 'custom_derivatives'}} +// expected-note@9 2{{fallback to numerical differentiation is disabled by the 'CLAD_NO_NUM_DIFF' macro}} -//CHECK: warning: Numerical differentiation is diabled using the -DCLAD_NO_NUM_DIFF flag, this means that every try to numerically differentiate a function will fail! Remove the flag to revert to default behaviour. -//CHECK: warning: Numerical differentiation is diabled using the -DCLAD_NO_NUM_DIFF flag, this means that every try to numerically differentiate a function will fail! Remove the flag to revert to default behaviour. //CHECK: double func_darg0(double x) { //CHECK-NEXT: double _d_x = 1; //CHECK-NEXT: return 0; @@ -24,6 +23,6 @@ double func(double x) { return std::tanh(x); } int main(){ - clad::differentiate(func, "x"); - clad::gradient(func); + clad::differentiate(func, "x"); + clad::gradient(func); } diff --git a/test/NumericalDiff/NumDiff.C b/test/NumericalDiff/NumDiff.C index c149123eb..1703412ff 100644 --- a/test/NumericalDiff/NumDiff.C +++ b/test/NumericalDiff/NumDiff.C @@ -1,15 +1,14 @@ -// RUN: %cladnumdiffclang %s -I%S/../../include -oNumDiff.out 2>&1 | FileCheck -check-prefix=CHECK %s +// RUN: %cladnumdiffclang %s -I%S/../../include -oNumDiff.out -Xclang -verify 2>&1 | FileCheck -check-prefix=CHECK %s // RUN: ./NumDiff.out | %filecheck_exec %s -// RUN: %cladnumdiffclang -Xclang -plugin-arg-clad -Xclang -enable-tbr %s -I%S/../../include -oNumDiff.out +// RUN: %cladnumdiffclang -Xclang -plugin-arg-clad -Xclang -enable-tbr -Xclang -verify %s -I%S/../../include -oNumDiff.out // RUN: ./NumDiff.out | %filecheck_exec %s //CHECK-NOT: {{.*error|warning|note:.*}} #include "clad/Differentiator/Differentiator.h" double test_1(double x){ - return tanh(x); + return tanh(x); // expected-warning {{function 'tanh' was not differentiated because clad failed to differentiate it and no suitable overload was found in namespace 'custom_derivatives'}} + // expected-note@9 {{falling back to numerical differentiation for 'tanh'}} } -//CHECK: warning: Falling back to numerical differentiation for 'tanh' since no suitable overload was found and clad could not derive it. To disable this feature, compile your programs with -DCLAD_NO_NUM_DIFF. -//CHECK: warning: Falling back to numerical differentiation for 'log10' since no suitable overload was found and clad could not derive it. To disable this feature, compile your programs with -DCLAD_NO_NUM_DIFF. //CHECK: void test_1_grad(double x, double *_d_x) { //CHECK-NEXT: { @@ -21,7 +20,8 @@ double test_1(double x){ double test_2(double x){ - return std::log10(x); + return std::log10(x);// expected-warning {{function 'log10' was not differentiated because clad failed to differentiate it and no suitable overload was found in namespace 'custom_derivatives'}} + // expected-note@23 {{falling back to numerical differentiation for 'log10'}} } //CHECK: double test_2_darg0(double x) { //CHECK-NEXT: double _d_x = 1; @@ -32,7 +32,8 @@ double test_2(double x){ double test_3(double x) { if (x > 0) { double constant = 11.; - return std::hypot(x, constant); + return std::hypot(x, constant); // expected-warning {{function 'hypot' was not differentiated because clad failed to differentiate it and no suitable overload was found in namespace 'custom_derivatives'}} + // expected-note@35 {{falling back to numerical differentiation for 'hypot'}} } return 0; } diff --git a/test/NumericalDiff/PrintErrorNumDiff.C b/test/NumericalDiff/PrintErrorNumDiff.C index 3ea747374..e52980d32 100644 --- a/test/NumericalDiff/PrintErrorNumDiff.C +++ b/test/NumericalDiff/PrintErrorNumDiff.C @@ -1,6 +1,6 @@ -// RUN: %cladnumdiffclang -Xclang -plugin-arg-clad -Xclang -fprint-num-diff-errors %s -I%S/../../include -oPrintErrorNumDiff.out 2>&1 | FileCheck -check-prefix=CHECK %s +// RUN: %cladnumdiffclang -Xclang -plugin-arg-clad -Xclang -fprint-num-diff-errors %s -I%S/../../include -oPrintErrorNumDiff.out -Xclang -verify 2>&1 | FileCheck -check-prefix=CHECK %s // RUN: ./PrintErrorNumDiff.out | %filecheck_exec %s -// RUN: %cladnumdiffclang -Xclang -plugin-arg-clad -Xclang -fprint-num-diff-errors -Xclang -plugin-arg-clad -Xclang -enable-tbr %s -I%S/../../include -oPrintErrorNumDiff.out +// RUN: %cladnumdiffclang -Xclang -plugin-arg-clad -Xclang -fprint-num-diff-errors -Xclang -plugin-arg-clad -Xclang -enable-tbr %s -I%S/../../include -oPrintErrorNumDiff.out -Xclang -verify // RUN: ./PrintErrorNumDiff.out | %filecheck_exec %s //CHECK-NOT: {{.*error|warning|note:.*}} @@ -12,10 +12,10 @@ extern "C" int printf(const char* fmt, ...); double test_1(double x){ - return tanh(x); + return tanh(x); // expected-warning {{function 'tanh' was not differentiated because}} + // expected-note@15 {{falling back to numerical differentiation for 'tanh}} } -//CHECK: warning: Falling back to numerical differentiation for 'tanh' since no suitable overload was found and clad could not derive it. To disable this feature, compile your programs with -DCLAD_NO_NUM_DIFF. //CHECK: void test_1_grad(double x, double *_d_x) { //CHECK-NEXT: { //CHECK-NEXT: double _r0 = 0;