martong added a comment.
> Retrieving the parameter location of functions was disabled because it may
> causes crashes due to the fact that functions may have multiple declarations
> and without definition it is difficult to ensure that always the same
> declration is used. Now parameters are stored in ParamRegions which are
> independent of the declaration of the function, therefore the same parameters
> always have the same regions, independently of the function declaration used
> actually. This allows us to remove the limitation described above.
I am not sure if this "summary" for the patch is aligned with its title. I
think the title says it all, I'd just skip this summary, it is way too blurry
for me.
Because:
> it may causes crashes
We still need the test cases for that, and I am not sure if we will ever get
one.
> without definition it is difficult to ensure that always the same declration
> is used.
That is not difficult in other parts of the compiler infrastructure (e.g in
ASTImporter), this statement is just a vague allusion to one or several
hypothetical bugs in the analyzer.
> the same parameters always have the same regions, independently of the
> function declaration used actually.
You mentioned personally to me already, but don't forget to add test cases for
that.
================
Comment at: clang/test/Analysis/temporaries.cpp:893
glob = 1;
- // FIXME: Why is destructor not inlined in C++17
clang_analyzer_checkInlined(true);
#ifdef TEMPORARY_DTORS
----------------
baloghadamsoftware wrote:
> I wonder whether `clang_analyzer_checkInlined()` works correctly with this
> patch: it seems it only checks for stack frame which now any function with
> definition can have.
So, why not try that in a test with a function that does not have a definition?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80286/new/
https://reviews.llvm.org/D80286
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
