On Wed, Jul 24, 2013 at 9:37 PM, Chris Lattner <clattner at apple.com> wrote:> > On Jul 22, 2013, at 2:25 PM, Chandler Carruth <chandlerc at google.com> wrote: > > > On Mon, Jul 22, 2013 at 2:21 PM, Eric Christopher <echristo at gmail.com> > wrote: >> >> >> This is pretty much the same as what Quentin proposed (with the >> >> addition of the enum), isn't it? >> >> >> > >> > Pretty close yeah. >> > >> >> Another thought and alternate strategy for dealing with these sorts of >> things: >> >> A much more broad set of callback machinery that allows the backend to >> communicate values or other information back to the front end that can >> then decide what to do. We can define an interface around this, but >> instead of having the backend vending diagnostics we have the callback >> take a "do something with this value" which can just be "communicate >> it back to the front end" or a diagnostic callback can be passed down >> from the front end, etc. >> >> This will probably take a bit more design to get a general framework >> set up, but imagine the usefulness of say being able to automatically >> reschedule a jitted function to a thread with a larger default stack >> size if the callback states that the thread size was N+1 where N is >> the size of the stack for a thread you've created. > > > FWIW, *this* is what I was trying to get across. Not that it wouldn't be a > callback-based mechanism, but that it should be a fully general mechanism > rather than having something to do with warnings, errors, notes, etc. If a > frontend chooses to use it to produce such diagnostics, cool, but there are > other use cases that the same machinery should serve. > > > How about this: keep the jist of the current API, but drop the "warning"- or > "error"-ness of the API. Instead, the backend just includes an enum value > (plus string message for extra data). The frontend makes the decision of > how to render the diagnostic (or not, dropping them is fine) along with how > to map them onto warning/error or whatever concepts they use.For the cases where backend diagnostics make sense (such as asm parsing or the like) it'll make sense for the backend to include a warning/error classification (& I don't think its absence is the key to what Chandler's getting at - the point is to avoid phrasing things as "diagnostics" from LLVM whenever it's possible to expose some broader information & let frontends do whatever they want with it) - that'll save every frontend having to make a, probably very similar, classification - epsecially in the case of errors it seems unlikely a frontend could do anything else but classify it as an error. The diagnostic blob should also include some 'id' though, yes, so frontends can, in a pinch, classify things differently (we have an LLVM flag "-fatal-assembly-warnings" or the like - we could kill that & just have the client callback on diagnostics elevate the warning to an error (indeed Clang already has infrastructure for promoting warnings to errors, so that should be used - no need for other implementations of the same idea)). I'm not quite clear from your suggestion whether you're suggesting the backend would produce a complete diagnostic string, or just the parameters - requiring/leaving it up to the frontend to have a full textual string for each backend diagnostic with the right number of placeholders, etc. I'm sort of in two minds about that - I like the idea that frontends keep all the user-rendered text (means localization issues are in one place, the frontend - rather than ending up with english text backend diagnostics rendered in a non-english client (yeah, pretty hypothetical, I'm not sure anyone has localized uses of LLVM)). But this does mean that there's no "free ride" - frontends must have some explicit handling of each backend diagnostic (some crappy worst-case fallback, but it won't be a useful message). & I don't think this avoids the desire to have non-diagnostic callbacks whenever possible (notify of interesting things, frontends can decide whether to use that information to emit a diagnostic based on some criteria or behave differently in another way).
On Jul 24, 2013, at 10:16 PM, David Blaikie <dblaikie at gmail.com> wrote:>> How about this: keep the jist of the current API, but drop the "warning"- or >> "error"-ness of the API. Instead, the backend just includes an enum value >> (plus string message for extra data). The frontend makes the decision of >> how to render the diagnostic (or not, dropping them is fine) along with how >> to map them onto warning/error or whatever concepts they use. > > I'm not quite clear from your suggestion whether you're suggesting the > backend would produce a complete diagnostic string, or just the > parameters - requiring/leaving it up to the frontend to have a full > textual string for each backend diagnostic with the right number of > placeholders, etc. I'm sort of in two minds about that - I like the > idea that frontends keep all the user-rendered text (means > localization issues are in one place, the frontend - rather than > ending up with english text backend diagnostics rendered in a > non-english client (yeah, pretty hypothetical, I'm not sure anyone has > localized uses of LLVM)). But this does mean that there's no "free > ride" - frontends must have some explicit handling of each backend > diagnostic (some crappy worst-case fallback, but it won't be a useful > message).I don't have a specific proposal in mind, other than thinking along the exact same lines as you above. :) The best approach is probably hybrid: the diagnostic producer can produce *both* a full string like today, as well as an "ID + enum" pair. This way, clang can use the later, but llc (as one example of something we want to keep simple) could print the former, and frontends that get unknown enums could fall back on the full string.> & I don't think this avoids the desire to have non-diagnostic > callbacks whenever possible (notify of interesting things, frontends > can decide whether to use that information to emit a diagnostic based > on some criteria or behave differently in another way).Sure, but we also don't want to block progress in some area because we have a desire to solve a bigger problem. -Chris
On Wed, Jul 24, 2013 at 10:23 PM, Chris Lattner <clattner at apple.com> wrote:> On Jul 24, 2013, at 10:16 PM, David Blaikie <dblaikie at gmail.com> wrote: >>> How about this: keep the jist of the current API, but drop the "warning"- or >>> "error"-ness of the API. Instead, the backend just includes an enum value >>> (plus string message for extra data). The frontend makes the decision of >>> how to render the diagnostic (or not, dropping them is fine) along with how >>> to map them onto warning/error or whatever concepts they use. >> >> I'm not quite clear from your suggestion whether you're suggesting the >> backend would produce a complete diagnostic string, or just the >> parameters - requiring/leaving it up to the frontend to have a full >> textual string for each backend diagnostic with the right number of >> placeholders, etc. I'm sort of in two minds about that - I like the >> idea that frontends keep all the user-rendered text (means >> localization issues are in one place, the frontend - rather than >> ending up with english text backend diagnostics rendered in a >> non-english client (yeah, pretty hypothetical, I'm not sure anyone has >> localized uses of LLVM)). But this does mean that there's no "free >> ride" - frontends must have some explicit handling of each backend >> diagnostic (some crappy worst-case fallback, but it won't be a useful >> message). > > I don't have a specific proposal in mind, other than thinking along the exact same lines as you above. :) > > The best approach is probably hybrid: the diagnostic producer can produce *both* a full string like today, as well as an "ID + enum" pair. This way, clang can use the later, but llc (as one example of something we want to keep simple) could print the former, and frontends that get unknown enums could fall back on the full string.Fair-ish. If it were just for the sake of llc it'd be hard to justify having the strings in LLVM rather than just in llc itself, but providing them as fallbacks is probably reasonable/convenient & not likely to be a technical burden. Hopefully if we wanted that we'd still put something in Clang to maintain the frontend diagnostic line rather than letting it slip.> >> & I don't think this avoids the desire to have non-diagnostic >> callbacks whenever possible (notify of interesting things, frontends >> can decide whether to use that information to emit a diagnostic based >> on some criteria or behave differently in another way). > > Sure, but we also don't want to block progress in some area because we have a desire to solve a bigger problem.Sure enough - I think the only reason to pre-empt the bigger problem is to ensure that the immediate progress doesn't lead to bad implementations of those bigger issues being committed due to convenience. Ensuring that the solution we implement now makes it hard to justify (not hard to /do/ badly, just hard to justify doing it badly by ensuring that the right solution is convenient/easy) taking shortcuts later would be good. That might just be the difference between having a function pointer callback for the diagnostic case and instead having a callback type with the diagnostic callback as the first one, with the intent to add more in cases where backend-diagnostics aren't the right tool. That way we have an callback interface we can easily extend. (ideally I'd love to have two things in the callback interface early, as an example - but that's not necessary & probably won't happen) I don't know enough about the particular things Quentin's planning to implement to know whether any of them fall into the "probably shouldn't be an LLVM diagnostic" bag.
It seems there is a desire to 'overload' the callback interfaces for very different purposes. IMVHO, there should be different categories of callbacks, and diagnostic callbacks should have their own. The design principle/tradeoffs of different categories can be quite different. My 2 cents. thanks, David On Wed, Jul 24, 2013 at 10:23 PM, Chris Lattner <clattner at apple.com> wrote:> On Jul 24, 2013, at 10:16 PM, David Blaikie <dblaikie at gmail.com> wrote: >>> How about this: keep the jist of the current API, but drop the "warning"- or >>> "error"-ness of the API. Instead, the backend just includes an enum value >>> (plus string message for extra data). The frontend makes the decision of >>> how to render the diagnostic (or not, dropping them is fine) along with how >>> to map them onto warning/error or whatever concepts they use. >> >> I'm not quite clear from your suggestion whether you're suggesting the >> backend would produce a complete diagnostic string, or just the >> parameters - requiring/leaving it up to the frontend to have a full >> textual string for each backend diagnostic with the right number of >> placeholders, etc. I'm sort of in two minds about that - I like the >> idea that frontends keep all the user-rendered text (means >> localization issues are in one place, the frontend - rather than >> ending up with english text backend diagnostics rendered in a >> non-english client (yeah, pretty hypothetical, I'm not sure anyone has >> localized uses of LLVM)). But this does mean that there's no "free >> ride" - frontends must have some explicit handling of each backend >> diagnostic (some crappy worst-case fallback, but it won't be a useful >> message). > > I don't have a specific proposal in mind, other than thinking along the exact same lines as you above. :) > > The best approach is probably hybrid: the diagnostic producer can produce *both* a full string like today, as well as an "ID + enum" pair. This way, clang can use the later, but llc (as one example of something we want to keep simple) could print the former, and frontends that get unknown enums could fall back on the full string. > >> & I don't think this avoids the desire to have non-diagnostic >> callbacks whenever possible (notify of interesting things, frontends >> can decide whether to use that information to emit a diagnostic based >> on some criteria or behave differently in another way). > > Sure, but we also don't want to block progress in some area because we have a desire to solve a bigger problem. > > -Chris > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev