Improved import of 'C implicit' functions.

[balazske]

This fix still does not fix the error for #684 but maybe it is useful.

[martong]

This fix still does not fix the error for #684 but maybe it is useful.

Calling functions without prototype is not allowed in C99 and in C11 (AFAIK), so this seems like a very borderline feature, I am not sure if we should spend so much time with this.

[balazske]

We can wait until the source of problems in #684 is found, probably this PR is not needed then (but still some old code can use the "C implicit function" feature) (but it could be a bug in the code too: the function is missing from the include file).

[balazske]

Or we can generate the "UnsupportedConstruct" error for this case.

[martong]

We can wait until the source of problems in #684 is found, probably this PR is not needed then (but still some old code can use the "C implicit function" feature) (but it could be a bug in the code too: the function is missing from the include file).

Let's see if we can find the real root cause of #684. If there is no actual crash related to "C functions without prototype" then we might not need these changes.

[balazske]

The test "ctu-main.c" contains this:

// The external function prototype is incomplete.
// warning:implicit functions are prohibited by c99
void testImplicit() {
  int res = identImplicit(6);   // external implicit functions are not inlined
  clang_analyzer_eval(res == 6); // expected-warning{{TRUE}}

The comment says "external implicit functions are not inlined" but this is not true (without the fix too) because the function is inlined if it is found in the externapDefMap file. This test should be changed, or really the external implicit function should not be inlined? The root cause of the SourceLocation problem in #684 is really that there is not a valid SourceLocation if the function is external implicit (but it is inlined because it is visited by the visitor). So that fix is correct if we want these functions to be inlined.

[martong]

The test "ctu-main.c" contains this:

// The external function prototype is incomplete.
// warning:implicit functions are prohibited by c99
void testImplicit() {
  int res = identImplicit(6);   // external implicit functions are not inlined
  clang_analyzer_eval(res == 6); // expected-warning{{TRUE}}

The comment says "external implicit functions are not inlined" but this is not true (without the fix too) because the function is inlined if it is found in the externapDefMap file. This test should be changed, or really the external implicit function should not be inlined?

I think we should inline it, could you please change the comment in the test?

The root cause of the SourceLocation problem in #684 is really that there is not a valid SourceLocation if the function is external implicit (but it is inlined because it is visited by the visitor). So that fix is correct if we want these functions to be inlined.

So, if I understand it correctly then #684 will be solved by correctly importing the "external implicit" C functions without prototypes? If that is the case then let's continue with this PR. Though I am confused now, because previously you wrote that this PR would not solve #684.

[balazske]

The fix in this PR makes the links between multiple FunctionDecls (for same function) correct. Probably the analyzer still works correct by using the inlined definition for analysis but the "CallEvent" contains the CallExpr that contains the original FunctionDecl (regardless if there is a link to the definition of the inlined function). The same conditions appear if the fix in this PR is applied or not, the difference is that the getDefinition returns the correct inlined definition or not. In this way, this fix does not make something work that was wrong before (but makes the imported AST more correct, maybe later some problem appears that needs this fix).

[martong]

Ok.

Now I don't understand why we hadn't have any assertion before when we run the ctu-main.c test ? We do inline the "external implicit" identImplicit function there and without a SourceLocation, right?

[martong]

Perhaps that is because there is no bug reported, so the BugReporterVisitor is not invoked.

Anyway this observation makes it cleaner to synthesize a test for #684: We could have something similar what we have in ctu-main.c with identImplicit but in the other TU where identImplicit is defined we could make a division by zero. Then the bug path would go through the callExpr of idendImplicit ...

[balazske]

Finally in #690 there is a test to reproduce the problem in #684. This fix is independent of those problems because these happens even with this fix applied. Still it is an improvement to ASTImporter so it can be useful.