D18224: Clang Plugin: Report some problems from included files

Milian Wolff noreply at phabricator.kde.org
Mon Apr 15 09:17:43 BST 2019


mwolff added a comment.


  In D18224#449416 <https://phabricator.kde.org/D18224#449416>, @thomassc wrote:
  
  > I re-tested the behavior on files from an actual project and it showed the same behavior when editing files as described in my last post, regardless of how many files are included from the files that are edited. I noticed that the behavior is different when re-opening files: in this case, problemsForFile() is actually called for a larger set of files (although from only looking at this set of files, it seemed a bit random and it was not clear to me how it is determined, I'll have to dig into the source code for this at some point ...).
  
  
  Upon reopening a file, we should only update the problems of changed files that got updated. Otherwise we should only grab the TU for the main .cpp file and attach it, such that we can do code completion.
  
  > In any case, the diagnostics cache seems to be clearly broken to me at the moment. The issue is that the ParseSession class and the diagnostics cache is related to a translation unit, but problemsForFile() is called for a specific file of that translation unit. So, if problemsForFile() is called for file A first and then for file B, the problems returned for file B will wrongly re-use cached entries that were created for file A before, while the actual problems might be completely different for both files.
  
  Yes and no. The problems should be reused, it's the whole idea of the cache. Note how `clang_getNumDiagnostics` is per-TU and thus applies to all files encountered in this TU. If we thus happen to use the same diagnostic, identified by its index, in two files, we can easily reuse the same problem.
  
  > Thus, as a concrete issue, if a header file is edited and then the user switches to the source file that was used within the translation unit for determining the header's problems, then KDevelop might show a wrong set of problems for that source file as a result (until the source file is edited again).
  
  Really, does this happen? Note how in both cases, the .cpp file should be used for parsing, and since that happens to include the header file, it will update that one too.
  
  > I haven't seen a case where the cache has been used correctly. So far I have only seen problemsForFile() being called once for a file from a translation unit. In this case, no correct re-use of Problems is possible on the file level. The incorrect re-use does currently speed up the function, but not in a desirable way. A place where valid re-use would be possible are the IncludeFileNotFoundProblem that are propagated to each document that includes the file with the error. But for that, the cache must be limited to only such problems, which has not been the case.
  
  Apparently you've figured out that this statement is wrong :)

REPOSITORY
  R32 KDevelop

REVISION DETAIL
  https://phabricator.kde.org/D18224

To: thomassc, #kdevelop, mwolff
Cc: aaronpuchert, pino, mwolff, kdevelop-devel, glebaccon, domson, antismap, iodelay, alexeymin, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20190415/ce80bbe6/attachment-0001.html>


More information about the KDevelop-devel mailing list