[cfe-dev] Same AST for is_same<char, int>::value and is_same<char, long>::value

[David Rector via cfe-dev]

> On Nov 24, 2021, at 5:00 AM, benicsbalazs at gmail.com wrote:
> 
> 
> Thanks for your response David!
> Actually, clang-tidy does something similar in the misc-redundant-expression check.
>  
>   ...
>   areEquivalentNameSpecifier(
>     cast<DependentScopeDeclRefExpr>(Left)->getQualifier(),
>     cast<DependentScopeDeclRefExpr>(Right)->getQualifier());
>  
> static bool areEquivalentNameSpecifier(const NestedNameSpecifier *Left,
>                                        const NestedNameSpecifier *Right) {
>   llvm::FoldingSetNodeID LeftID, RightID;
>   Left->Profile(LeftID);
>   Right->Profile(RightID);
>   return LeftID == RightID;
> }
>  
> But when I try to create a positive test case I fail.
> At line 7 in the next example, I don’t get a warning.
> It is because the folding set nodes (`LeftID` and `RightID`) are different.
> Just like the pointer values of `Left` and `Right`.
> I'm surprised about this, since I'm referring to the very same variable, thus I would expect the same pointer values.
> When I dump nested name specifiers I get the same string: “my_trait<char>::”
>  
> 1  template <bool V> struct boolean_value {
> 2    static constexpr bool value = V;
> 3  };
> 4  template <typename T> struct my_trait : boolean_value<true> {};
> 5  bool respect_nested_name_specifiers(bool sink) {
> 6    sink = my_trait<char>::value || my_trait<int>::value;  // no-warning, awesome
> 7    sink = my_trait<char>::value || my_trait<char>::value; // no warning for this either :(
> 8    return sink;
> 9  }
> 
> How is it possible to have different NestedNameSpecifier pointers describing the same thing?
> Am I missing something?
>  
> Thanks,
> Balázs

NestedNameSpecifier holds syntax info, and so has no particular obligation to unique even for identical syntax and semantics, like say a canonical type needs to be.  Only by calling one of its getAs* methods can you access the underlying semantics you probably want to test.

You probably want to use getAsType, since that shouldn’t crash like getAsRecordDecl might and will give you flexibility eg:

using charB = char;
sink = my_trait<char>::value || my_trait<charB>::value; // should this warn?

If that should warn, then you want to compare the getQualifier()->getAsType()s without desugaring; if it shouldn’t, you probably should just compare the canonical types (RecordTypes); if more complex, manually desugar step by step.

All this said, even though two NNSs representing identical syntax and semantics don’t strictly need to be the same pointer, I’m slightly surprised they aren’t - I assumed NestedNameSpecifierLocBuilder handled that sort of thing - but probably it is the case that the savings of reusing NNSs don’t exceed its costs.  (Though, it seems like the same rationale would apply to sugar types, which I believe are unique/reused.)


  

> 
> From: David Rector <davrecthreads at gmail.com> 
> Sent: 2021. november 23., kedd 23:39
> To: Benics Balázs <benicsbalazs at gmail.com>
> Cc: cfe-dev <cfe-dev at lists.llvm.org>
> Subject: Re: [cfe-dev] Same AST for is_same<char, int>::value and is_same<char, long>::value
>  
> Comparing the getQualifier()->getAsRecordDecl()s of the two DeclRefExprs should do it I think (untested).  Careful though, NNS::getAsRecordDecl() crashes instead of returning null if the qualifier is e.g. a namespace, check its implem for the permissible kinds (or maybe better to add a getAsRecordDeclUnsafe() method).
> 
> 
> On Nov 23, 2021, at 11:03 AM, Benics Balázs via cfe-dev <cfe-dev at lists.llvm.org> wrote:
> 
> 
> (Resending from my personal email address)
>  
>  
> From: Balázs Benics <balazs.benics at sigmatechnology.se>
> Sent: Tuesday, November 23, 2021, 14:45
> To: cfe-dev
> Subject: Same AST for is_same::value and is_same::value
> 
>  
> I seek some advice on the following AST scenario.
> https://godbolt.org/z/WcTeoGcbG
>  
> 1  #include <type_traits>
> 2  using std::is_same;
> 3  bool top() {
> 4    return is_same<char, int>::value || is_same<char, long>::value;
> 5  }
>  
> The `alpha.core.IdenticalExpr` static analyzer checker warns about that the two `..::value` are identical DeclRefExprs.
> According to the AST, they are indeed identical:
>  
>   BinaryOperator <col:10, col:60> 'bool' '||'
>   |-ImplicitCastExpr <col:10, col:30> 'bool':'bool' <LValueToRValue>
>   | `-DeclRefExpr <col:10, col:30> 'const bool':'const bool' lvalue Var 0x563b67cde0a8 'value' 'const bool':'const bool'
>   `-ImplicitCastExpr <col:39, col:60> 'bool':'bool' <LValueToRValue>
>     `-DeclRefExpr <col:39, col:60> 'const bool':'const bool' lvalue Var 0x563b67cde0a8 'value' 'const bool':'const bool'
>  
> Both of them refers to the same `VarDecl`, corresponding (probably) to the `std::false_type::value`.
>  
> How can I detect that even though the `values` look similar, they correspond to different template instances in that sense?
> What do you suggest?
> In the end, I want to suppress these reports in the static analyzer.
>  
> Balázs
>  
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20211124/be3a71cb/attachment-0001.html>