On 11/12/24 9:02 AM, David Malcolm wrote:
[from 0/3]
The most natural way to present textual output is to use indentation,
so patch 1 does this. However, our existing textual format uses the
source location as a prefix, e.g.
PATH/foo.cc: error: message goes here
and unfortunately this makes the indented hierarchy unreadable
(especially if a tree of diagnostics spans multiple source files).
I've previously tried to do a bit of diagnostic nesting with initial
spaces, which this patch removes. This seems like a small regression in
the default mode, though I'm not too concerned about it.
Maybe there could be a mode that puts the indenting from this framework
at the beginning of the message rather than the beginning of the line?
This patch uses the nested diagnostics capabilities added in the earlier
patch in the C++ frontend.
With this, and enabling the non-standard text formatting via:
-fdiagnostics-set-output=text:experimental-nesting=yes
and using:
-std=c++20 -fconcepts-diagnostics-depth=2
then the output for the example in SG15's P3358R0 ("SARIF for Structured
Diagnostics") is:
P3358R0.C: In function ‘int main()’:
P3358R0.C:26:6: error: no matching function for call to ‘pet(lizard)’
26 | pet(lizard{});
| ~~~^~~~~~~~~~
• note: candidate: ‘template<class auto:1> requires pettable<auto:1> void
pet(auto:1)’
P3358R0.C:21:6:
Putting the source location on a separate line seems like an interesting
approach to the issue of variable-length pathnames vs. nesting.
In a quick test with nested-diagnostics-1-truncated.C, it seems like
emacs compile-mode has trouble with the indented source locations. The
singly indented ones it doesn't recognize at all; the doubly indented
ones it correctly identifies as locations, but thinks the file name is
e.g. "default nested-diagnostics-1-truncated.C:25". And it
misidentifies the line
• nested-diagnostics-1-truncated.C: In substitution of
‘template<class auto:1> requires pettable<auto:1> void \
pet(auto:1) [with auto:1 = lizard]’:
as the last :1 being a line number in a very long and strange filename.
I wonder about an option to put source locations at the start of the
line while nesting the actual diagnostics? Possibly before the quoted
source line, to conserve the actual number of lines of output?
21 | void pet(pettable auto t);
| ^~~
• note: template argument deduction/substitution failed:
• note: constraints not satisfied
• P3358R0.C: In substitution of ‘template<class auto:1> requires
pettable<auto:1> void pet(auto:1) [with auto:1 = lizard]’:
• required from here
P3358R0.C:26:6:
26 | pet(lizard{});
| ~~~^~~~~~~~~~
• required for the satisfaction of ‘pettable<auto:1>’ [with auto:1 =
lizard]
Incidentally, the wording of these two context lines seems a bit weird;
we're checking the satisfaction of pet<lizard>, not pettable<lizard>.
The latter is a constraint on the former.
P3358R0.C:19:9:
19 | concept pettable = has_member_pet<T> or has_default_pet<T>;
| ^~~~~~~~
• note: no operand of the disjunction is satisfied
P3358R0.C:19:38:
19 | concept pettable = has_member_pet<T> or has_default_pet<T>;
| ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~
• note: the operand ‘has_member_pet<T>’ is unsatisfied because
P3358R0.C:19:20:
19 | concept pettable = has_member_pet<T> or has_default_pet<T>;
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
• required for the satisfaction of ‘has_member_pet<T>’ [with T =
lizard]
P3358R0.C:13:9:
13 | concept has_member_pet = requires(T t) { t.pet(); };
| ^~~~~~~~~~~~~~
It would be nice to avoid two lines about has_member_pet in this case.
• required for the satisfaction of ‘pettable<auto:1>’ [with auto:1
= lizard]
P3358R0.C:19:9:
19 | concept pettable = has_member_pet<T> or
has_default_pet<T>;
| ^~~~~~~~
It would be nice to do away with this redundant context that we already
printed above.
• in requirements with ‘T t’ [with T = lizard]
P3358R0.C:13:26:
13 | concept has_member_pet = requires(T t) { t.pet(); };
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
• note: the required expression ‘t.pet()’ is invalid, because
P3358R0.C:13:47:
13 | concept has_member_pet = requires(T t) { t.pet(); };
| ~~~~~^~
• error: ‘struct lizard’ has no member named ‘pet’
P3358R0.C:13:44:
13 | concept has_member_pet = requires(T t) { t.pet(); };
| ~~^~~
• note: the operand ‘has_default_pet<T>’ is unsatisfied because
P3358R0.C:19:41:
19 | concept pettable = has_member_pet<T> or has_default_pet<T>;
| ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~
• required for the satisfaction of ‘has_default_pet<T>’ [with T =
lizard]
P3358R0.C:16:9:
16 | concept has_default_pet = T::is_pettable;
| ^~~~~~~~~~~~~~~
• required for the satisfaction of ‘pettable<auto:1>’ [with auto:1
= lizard]
P3358R0.C:19:9:
19 | concept pettable = has_member_pet<T> or
has_default_pet<T>;
| ^~~~~~~~
There's also the broader issue of weird context ordering in g++, where
sometimes we get context from outside in (as expressed by the nesting)
and sometimes from inside out (the "required for" lines), mixed
together. Maybe we could try to avoid printing inside-out context (i.e.
everything added by cp_diagnostic_text_starter) at all when we're
already printing outside-in context? Or use formatting to distinguish
it more clearly?
+++ b/gcc/testsuite/g++.dg/concepts/nested-diagnostics-1-truncated.C
@@ -0,0 +1,41 @@
+// { dg-do compile { target c++17 } }
+// { dg-options "-fconcepts" }
+// { dg-additional-options
"-fdiagnostics-set-output=text:experimental-nesting=yes,experimental-nesting-show-locations=no"
}
+
+struct dog {};
+struct cat {};
+
+void pet(dog);
+void pet(cat);
+
+template <class T>
+concept has_member_pet = requires(T t) { t.pet(); };
+
+template <class T>
+concept has_default_pet = T::is_pettable;
+
+template <class T>
+concept pettable = has_member_pet<T> or has_default_pet<T>;
+
+void pet(pettable auto t);
+
+struct lizard {};
+
+int main() {
+ pet(lizard{}); // { dg-error "no matching function for call to
'pet\\\(lizard\\\)'" }
+}
+
+/* { dg-begin-multiline-output "" }
+ * note: candidate: 'template<class auto:1> requires pettable<auto:1> void
pet(auto:1)'
+ * note: template argument deduction/substitution failed:
+ * note: constraints not satisfied
+ * In substitution of 'template<class auto:1> requires
pettable<auto:1> void pet(auto:1) [with auto:1 = lizard]':
+ * required from here
"required from here" sure seems pointless with no source location...
Anyway, all these comments aren't requests for changes to this patch so
much as thoughts about future directions.
The patch is OK as is.
Jason
