-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace logger #955
Replace logger #955
Conversation
launch jenkins |
launch jenkins |
launch jenkins |
launch jenkins |
launch jenkins |
@@ -234,8 +234,8 @@ bool PassRemoveScalars::run( | |||
// Check if we have unsupported statements. If we do, warn the user and skip the pass execution. | |||
for(const auto& stmt : iterateIIROverStmt(*stencilInstantiation->getIIR())) { | |||
if(isStatementUnsupported(stmt, stencilInstantiation->getMetaData())) { | |||
DAWN_LOG(INFO) << "Unsupported statement at line " << stmt->getSourceLocation() | |||
<< ". Skipping removal of scalar variables."; | |||
DAWN_DIAG(INFO, stencilInstantiation->getMetaData().getFileName(), stmt->getSourceLocation()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is an example of the diagnostic in code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the idea to not switch in this but in a follow up PR?
std::cout << "PASS: " << getName() << ": " << stencilInstantiation->getName() | ||
<< ": DoMethod: " << doMethod->getID() << " removed variable: " << varName | ||
<< std::endl; | ||
DAWN_LOG(INFO) << stencilInstantiation->getName() << ": DoMethod: " << doMethod->getID() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switched to DAWN_LOG(INFO)
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
break; | ||
case google::protobuf::LOGLEVEL_FATAL: | ||
DAWN_LOG(FATAL) << "Protobuf: " << message; | ||
throw SyntacticError(std::string("[FATAL] Protobuf error occurred: ") + message); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should match the other error before merging.
launch jenkins |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks great
std::cout << "PASS: " << getName() << ": " << stencilInstantiation->getName() | ||
<< ": DoMethod: " << doMethod->getID() << " removed variable: " << varName | ||
<< std::endl; | ||
DAWN_LOG(INFO) << stencilInstantiation->getName() << ": DoMethod: " << doMethod->getID() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@@ -234,8 +234,8 @@ bool PassRemoveScalars::run( | |||
// Check if we have unsupported statements. If we do, warn the user and skip the pass execution. | |||
for(const auto& stmt : iterateIIROverStmt(*stencilInstantiation->getIIR())) { | |||
if(isStatementUnsupported(stmt, stencilInstantiation->getMetaData())) { | |||
DAWN_LOG(INFO) << "Unsupported statement at line " << stmt->getSourceLocation() | |||
<< ". Skipping removal of scalar variables."; | |||
DAWN_DIAG(INFO, stencilInstantiation->getMetaData().getFileName(), stmt->getSourceLocation()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the idea to not switch in this but in a follow up PR?
Technical Description
Replace Logger by a similar but improved infrastructure.
A follow-up PR will replace the diagnostics with this.
The Logger design is similar to before, however there are separate loggers
dawn::log::info
,dawn::log::warning
, anddawn::log::error
which are globals similar tostd::cout
andstd::cerr
.Each logger has an
LoggerProxy operator()(string, int)
as before that returns the stream operatorEach logger also acts as a container that stores the messages (a la
DiagnosticsEngine
).Instead of a virtual LoggerInterface that potentially leaks memory when registering other class instance pointers, this is replaced with a lightweight std::function object
Logger::Formatter
, so GTClang and other frameworks can register their own message formatting.Loggers also take underlying
std::ostream
s, which can be customized to write to strings, files, etc.Resolves / Enhances
Part of #950.
Testing
Integrates into ctest.