dblaikie added a comment.
Moving this to a top level discussion because it's easier to write replies here
than in an inline comment.
>> If it's that the tool is trying to get the type information purely from the
>> string representation, maybe the tool should be doing things differently -
>> relying on Clang's AST/type APIs, rather than on inspecting the resulting
>> string?
>
> I completely agree, however, the documentation on type API is not very good,
> and couldn't find a better way to get a string representation of the type
> using this API, besides manually traversing the AST and concatenating bits
> and pieces into a string which is essentially the same thing that TypePrinter
> does...
Sorry, I'm confused - it sounded like you didn't actually want a string, though
- you want the type information to make various semantic-aware choices in your
tool, yes?
> On the other hand, inventing new data structures to hold the type information
> or directly saving pointers to parts of the AST does not sound appealing to
> me, at least not for the current level of complexity I'm aiming for.
Generally the way to do it, if you want to introspect into the type, its
template parameters, etc, then yes, keeping pointers to the AST or reparsing
the name with Clang as-needed, would be the way to go - or walking the AST
up-front and generating your own data structure (not necessarily a string) with
the data you want in some structured format. Relying on the string printing of
Clang to produce enough introspective information for a static analysis tool I
think would be at odds with other tradeoffs that stringification is trying to
achieve.
>> But if the type as a string is correct as far as C++ is concerned, is that
>> not enough information (necessarily - the C++ compiler can determine all the
>> type information from the type as written with only the top level types in
>> the NTTP names)?
>
> I think that the essential problem here is the as far as C++ is concerned?
> What about the developer? Developers may have more benefit from getting a
> full type name in their diagnostic than just something that is correct as far
> as C++ is concerned.
>
> Consider this error message, which is completely useless and compare it to
> the much more informative errors by GCC and MSVC that immediately tell you
> what's wrong with your code.
>
> So, why shouldn't Clang behave the same way as well?
Again, I'm not advocating for the printing as-is, I think adding the top level
name that disambiguates would be a good thing - and I think the GCC and MSVC
examples somewhat show why adding all the other layers would be harmful to
readability - there's a lot of text in those messages that doesn't directly
help the user and gets in the way of identifying the important differences
between the type names.
For instance, let's take the GCC message:
<source>: In function 'void foo()':
<source>:25:9: error: no match for 'operator=' (operand types are
'NDArray<float, Width{DimensionImpl<Width>{Dimension{0}}}>' and 'NDArray<float,
Height{DimensionImpl<Height>{Dimension{0}}}>')
25 | a = b;
| ^
<source>:2:8: note: candidate: 'constexpr NDArray<float,
Width{DimensionImpl<Width>{Dimension{0}}}>& NDArray<float,
Width{DimensionImpl<Width>{Dimension{0}}}>::operator=(const NDArray<float,
Width{DimensionImpl<Width>{Dimension{0}}}>&)'
2 | struct NDArray {};
| ^~~~~~~
<source>:2:8: note: no known conversion for argument 1 from 'NDArray<float,
Height{DimensionImpl<Height>{Dimension{0}}}>' to 'const NDArray<float,
Width{DimensionImpl<Width>{Dimension{0}}}>&'
<source>:2:8: note: candidate: 'constexpr NDArray<float,
Width{DimensionImpl<Width>{Dimension{0}}}>& NDArray<float,
Width{DimensionImpl<Width>{Dimension{0}}}>::operator=(NDArray<float,
Width{DimensionImpl<Width>{Dimension{0}}}>&&)'
<source>:2:8: note: no known conversion for argument 1 from 'NDArray<float,
Height{DimensionImpl<Height>{Dimension{0}}}>' to 'NDArray<float,
Width{DimensionImpl<Width>{Dimension{0}}}>&&'
And compare it to:
<source>: In function 'void foo()':
<source>:25:9: error: no match for 'operator=' (operand types are
'NDArray<float, Width{{{0}}}>' and 'NDArray<float, Height{{{0}}}>')
25 | a = b;
| ^
<source>:2:8: note: candidate: 'constexpr NDArray<float, Width{{{0}}}>&
NDArray<float, Width{{{0}}}>::operator=(const NDArray<float, Width{{{0}}}>&)'
2 | struct NDArray {};
| ^~~~~~~
<source>:2:8: note: no known conversion for argument 1 from 'NDArray<float,
Height{{{0}}}>' to 'const NDArray<float, Width{{{0}}}>&'
<source>:2:8: note: candidate: 'constexpr NDArray<float, Width{{{0}}}>&
NDArray<float, Width{{{0}}}>::operator=(NDArray<float, Width{{{0}}}>&&)'
<source>:2:8: note: no known conversion for argument 1 from 'NDArray<float,
Height{{{0}}}>' to 'NDArray<float, Width{{{0}}}>&&'
That seems significantly easier to read/easier to identify the relevant
difference for the developer.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134453/new/
https://reviews.llvm.org/D134453
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
