[PATCH] D34506: Relax an assert in the comparison of source locations

Gábor Horváth via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jun 27 02:49:16 PDT 2017


xazax.hun added a comment.

In https://reviews.llvm.org/D34506#791089, @akyrtzi wrote:

> Comparing SourceLocations from different translation units is not meaningful and my concern is that treating source locations like this can very easily lead to errors where by mistake the code is resolving a SourceLocation with the wrong translation unit and not the translation unit that it came from.
>
> If you intend to sort source locations across translation units I think you should use an abstraction that includes the source location _and_ the ASTContext or ASTUnit (or something to identify the translation unit) that the source location came from, and use that for sorting (by checking whether the TU are the same, and if not sort appropriately with the TUs, before sorting the SourceLocations).
>  That way you don't need to 'allow' comparing source locations from different TUs, which IMO is a good sanity check to make sure the code did not mix-up the source locations by accident.


Thank you for the suggestions! 
The use-case is to make the analyzer deterministic when reporting the diagnostics. Unfortunately, we are already using FullSourceLocs, and it does not help, because the imported AST (using the ASTImporter) has imported source locations that are using the same source manager as the original AST. But I will look into some workaround.


Repository:
  rL LLVM

https://reviews.llvm.org/D34506





More information about the cfe-commits mailing list