Add #pragma error and warning to OSL preprocessor
#1300
Conversation
src/liboslcomp/osllex.l
Outdated
| if (pragmatype == "error") { | ||
| string_view msg; | ||
| OIIO::Strutil::parse_string(line, msg); | ||
| oslcompiler->errorf(oslcompiler->filename(), | ||
| oslcompiler->lineno(), "%s", msg); | ||
| } |
There was a problem hiding this comment.
It's not obvious to me why #pragma error handling is so different from #error, if this is following some standard or just an oversight.
- Case sensitive
- Ignore return value of
parse_string "%s"vs"\"%s\""
There was a problem hiding this comment.
My original idea was to use #error and #warning but I could not coax the clang preprocessor to forward them to me. Both got treated directly in clang and never made it to my code, no matter what I tried. I can get pragmas, though, so I decided to implement this via #pragma error and #pragma warning.
There was a problem hiding this comment.
parse_string -- you are correct, will fix
case sensitive -- I think this is ok? I really do want it to be "error" and "warning" and I don't think there's merit to allowing people to write #wArNiNg. OSL is a bit of a special case because I find it plausible that somebody could get confused and write #pragma OSL when they meant #pragma osl, though if you argue that should not be allowed and it should also be case-sensitive, I would find that persuasive.
Should the message be quoted? We don't do that for any other error messages. For example:
test.osl:9: error: 'asefasef' was not declared in this scope
NOT
test.osl:9: error: "'asefasef' was not declared in this scope"
There was a problem hiding this comment.
My original idea was to use
#errorand#warningbut I could not coax the clang preprocessor to forward them to me. Both got treated directly in clang and never made it to my code, no matter what I tried. I can get pragmas, though, so I decided to implement this via#pragma errorand#pragma warning.
Why would it be a problem for clang to handle those errors/warnings? Doesn't it have a callback system that lets you extract the warnings/errors that occurred during preprocessing?
There was a problem hiding this comment.
clang itself can handle the #error, but I don't want it to -- I want the error propagated back to me so I can report it through our own system. And for #warning it was worse, clang reports that as an error, too, as an unknown preprocessor directive (as if you'd said #foo).
Logically, it seems like there has to be a way to skin this cat. But I could find no such callback system reading through the docs on the relevant classes. I asked on the mail list and got no replies. I could probably read through the clang source code and figure it out, but for this tiny little convenience feature, it didn't seem like a good trade-off to sink much more time into it.
Doing it as a #pragma solves the problem more than adequately for Lee, who requested this feature. So I decided to cut my losses and move on to the many other things I should be working on. I can always go back and add #error and #warning directly if I get time later to figure it out or the better solution comes to my attention.
There was a problem hiding this comment.
Browsing through the clang docs I found clang::DiagnosticConsumer which looks like what you want?
It has a HandleDiagnostic virtual method that seems to be passed the relevant information.
Currently the code is using clang::TextDiagnosticPrinter which is a built-in implementation that prints to a stream.
There was a problem hiding this comment.
Ooh, thanks, I'll check it out.
There was a problem hiding this comment.
That was not quite it, but I do think I'm on the trail of it now!
There was a problem hiding this comment.
I'm still not able to fully make it work. I got close in two different ways: I found all sorts of ways to register callbacks and special handling for pragmas, but not a bare #warning. Also, I can I can intercept the diagnostic as you suggest with an overloaded HandleDiagnostic, but from there all I get is a diagnostic ID and I can't figure out how to tell which ID reliably corresponds to the particular warning (and only that). I'm sure there's a way to do it, but at this point I've sunk many hours into it for little practical value. I'm going to cut my losses and stick the the thing I can make work.
There was a problem hiding this comment.
Ah sorry for the wild goose chase - it seemed like any other type of warning would pass through the same system.
Its a shame that the clang API is overly complicated here. LGTM otherwise!
Signed-off-by: Larry Gritz <[email protected]>
Signed-off-by: Larry Gritz [email protected]