[PATCH] D80286: [Analyzer] Allow creation of stack frame for functions without definition

Gabor Marton via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed May 20 05:56:05 PDT 2020


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





More information about the cfe-commits mailing list