> On Oct 31, 2024, at 12:55, David Malcolm <dmalc...@redhat.com> wrote:
>
> On Wed, 2024-10-30 at 20:51 +0000, Qing Zhao wrote:
>> Hi, David,
>>
>>> On Oct 30, 2024, at 14:54, David Malcolm <dmalc...@redhat.com>
>>> wrote:
>>>
>>> On Wed, 2024-10-30 at 15:53 +0000, Qing Zhao wrote:
>>>>
>>>>
>>>>> On Oct 30, 2024, at 10:48, David Malcolm <dmalc...@redhat.com>
>>>>> wrote:
>>>>>
>>>>> On Wed, 2024-10-30 at 14:34 +0000, Sam James wrote:
>>>>>> Qing Zhao <qing.z...@oracle.com> writes:
>>>>>>
>>>>>>> Control this with a new option -fdiagnostics-details.
>>>>>>>
>>>
>
> [...]
>
>>
>>>> Should I control the creation of “rich_location" with the flag
>>>> “flag_diagnostics_details” (Similar as I control the creation of
>>>> “move_history” data structure with the flag
>>>> “flag_diagnostics_details”?
>>>>
>>>> If so, how should I do it? Do you have a suggestion on a clean
>>>> and
>>>> simply coding here (Sorry for the stupid question on this)
>>>
>>> You can probably do all of this on the stack; make a new
>>> rich_location
>>> subclass, with something like:
>>>
>>> class rich_location_with_details : public gcc_rich_location
>>> {
>>> public:
>>> rich_location_with_details (location_t location, gimple *stmt);
>>>
>>> private:
>>> class deferred_move_history_path {
>>> public:
>>> deferred_move_history_path (location_t location, gimple *stmt)
>>> : m_location (location), m_stmt (stmt)
>>> {
>>> }
>>>
>>> std::unique_ptr<diagnostic_path>
>>> make_path () const final override;
>>> /* TODO: you'll need to implement this; it will be called on
>>> demand if a diagnostic is acutally emitted for this
>>> rich_location. */
>>
>> What do you mean by “it will be called on demand if a diagnostic is
>> actually emitted”?
>> Do I need to do anything special in the code to call this
>> “make_path”?
>
> I think I got the above wrong, sorry - I meant
> deferred_move_history_path to be derived from the
> deferred_diagnostic_path from my patch
Okay, I see.
>
> I've rewritten my patch somewhat, and I pushed it to trunk as r15-4807
> a few minutes ago:
> https://gcc.gnu.org/pipermail/gcc-patches/2024-October/667070.html
> I renamed "deferred_diagnostic_path" to "lazy_diagnostic_path" which
> hopefully is a clearer name.
>
>
> With that, I think you want something like rich_location subclass
> (caveat: untested):
>
> #include "lazy-diagnostic-path.h"
>
> /* A rich_location subclass that lazily populates a diagnostic_path
> with move history events, but only if the path is actually to be
> used. */
>
> class rich_location_with_details : public gcc_rich_location
> {
> public:
> rich_location_with_details (location_t location, gimple *stmt)
> : gcc_rich_location (location),
> m_deferred_move_history_path (location, stmt)
> {
> set_path (&m_deferred_move_history_path);
> }
>
> private:
> class lazy_move_history_path : public lazy_diagnostic_path {
> public:
> lazy_move_history_path (location_t location, gimple *stmt)
> : m_location (location), m_stmt (stmt)
> {
> }
>
> std::unique_ptr<diagnostic_path>
> make_inner_path () const final override;
> /* TODO: you'll need to implement this; it will be called on
> demand if a diagnostic is actually emitted for this
> rich_location. */
>
> location_t m_location;
> gimple *m_stmt;
> } m_lazy_move_history_path;
> };
>
> ...and with that support subclass, you should be able to do something like
> this to make them on the stack:
>
> rich_location_with_details richloc (location, stmt);
> warned = warning_at (&richloc, OPT_Warray_bounds_,
> "array subscript %E is outside array"
> " bounds of %qT", low_sub_org, artype);
>
> and all that will be done in richloc's constructor will be some cheap
> field initializations.
>
> Only if a diagnostic actually gets emitted will
> lazy_move_history_path::make_inner_path
> be called, at which point it can query the move history and make a path
> with the appropriate events.
Now, I understand. Thanks a lot for the detailed explanation.
>
> There's an example of this approach in test_emission in
> https://gcc.gnu.org/pipermail/gcc-patches/2024-October/667070.html
> which has types test_lazy_path and test_rich_location, which are
> analogous to lazy_move_history_path and rich_location_with_details
> above.
Thanks a lot for the example, it’s very helpful.
>
> Hope this makes sense; sorry for being unclear. Alternatively I can
> have a go at implementing this if you send me the latest version of
> your patches.
I will try to write this myself, if there is any issue, I will ask for your
help.
Again, thanks a lot for the help.
Qing
>
> Dave
