tomasz-kaminski-sonarsource wrote:

> Thanks for your help very much.I only fixed the symptoms of this bug without 
> spending much time to find the root cause. I feel ashamed. 

There is nothing to be ashamed of. You have spent the time to debug the crash, 
and localize the use of non-engaged optional, this is already helpful for the 
project. And addressing the crash (for example by stopping analysis) makes 
sure, that analysis continues and we are able to report issues from other 
functions. This is an improvement over the status quo, and sometimes 
determining the root cause is very difficult.

> But I am not sure whether this change has some side 
> effects.`MD->isInstance()` replaced with 
> `MD->isImplicitObjectMemberFunction()` will miss some cases create 
> `<CXXMemberOperatorCall>` in my opinion.Are these missing cases unnecessary?

There are 4 possible choices on how an operator can be overloaded in C++:
a) free functions, including friends
b) implicit object member functions (old C++ member methods)
c) explicit object member functions (`this` parameters)
d) static member functions (you can do `operator()` and `operator[]`)

When the `CXXOperatorCallExpr` is constructed, it will always contain an object 
parameter on the argument list. So for the argument to parameter adjustment, we 
need to do:
a) nothing -> same number of params and args
b) adjust + 1 -> there is implicit object param mapped to this, and we need to 
expose this region
c) nothing -> object has the corresponding param, so the number is the same
d) this is a bit tricky, as `a(10)` will have 2 args and only one parameter.

Before C++23, the option didn't exist, and `isInstance()` implied that we are 
in case (`b`). Now `isInstance()` covers both `b` and `c`, which is leading to 
the crash you have found. This we need to fallback to the old status queue by 
using `isImplicitObjectParameter()`.

We may still not cover the case `d`, of static operators, for example, we 
should verify if div by zero is raised here if the operator is static, and if 
removing `static` helps. But I think this should be handled as a separate PR.
```C++
struct C {
  static void operator()(int x) { return 10 / x; }
};

void test(C c) {
   c(10);
}
```
If you would be interested in looking at also fixing it after this PR, I would 
be happy to provide my thoughts, on how to fix it. 


https://github.com/llvm/llvm-project/pull/83585
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to