This revision was automatically updated to reflect the committed changes.
Closed by commit rC334804: [ASTImporter] Corrected diagnostic client handling
in tests. (authored by baloghadamsoftware, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D47445?vs=149412&id=151459#toc
Re
balazske added a comment.
I want this change to be committed by somebody. I have no commit right.
Repository:
rC Clang
https://reviews.llvm.org/D47445
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/l
akyrtzi added a comment.
We could leave `disableSourceFileDiagnostics` off until someone finds a use
case for it.
Repository:
rC Clang
https://reviews.llvm.org/D47445
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/
balazske added a comment.
From API point of view if there is a `enableSourceFileDiagnostics` there should
be a `disableSourceFileDiagnostics` too (that calls the `EndSourceFile`). But I
am not sure how and if to use it at all. In the unit tests it is not needed,
the ASTUnit contains a single en
balazske updated this revision to Diff 149412.
balazske added a comment.
- Added comment, renamed beginSourceFile, removed check for PP.
Check for PP is removed because it is allowed to be nullptr.
Repository:
rC Clang
https://reviews.llvm.org/D47445
Files:
include/clang/Frontend/ASTUnit.
akyrtzi added a comment.
Thanks for the explanation. Please do add documentation comments for the new
method so people using ASTUnit in their own code have an idea when and why they
would need to call this. Something like "if you intend to emit additional
diagnostics after the ASTUnit is create
balazske added a comment.
The new `ASTUnit::beginSourceFile` is only there to simplify the code. It is
possible to get `Ctx`, `PP` and `getDiagnostic()` from outside of `ASTUnit` and
call the same thing, but requires more code to write. Probably a more smart
place to call `BeginSourceFile` can
balazske added a comment.
If `BeginSourceFile` is not called on the diagnostic client object, it is not
possible to have compiler warnings or errors that come from the "To" context
while importing something (there is some assertion if a source file related
warning is to be emitted but no source
akyrtzi added a comment.
It's not clear to me what kind of issue you are addressing, for example is the
unit test hitting an assertion hit without your changes ? Or is there something
else ?
Also it would be good to add some documentation comments to clarify what's the
use case and when `ASTUni
a.sidorin accepted this revision.
a.sidorin added a comment.
Looks good to me, but the approval from AST code owners is required, I think.
Repository:
rC Clang
https://reviews.llvm.org/D47445
___
cfe-commits mailing list
cfe-commits@lists.llvm.or
martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: rnkovacs.
LGTM! But let's wait the review of those people who have history in
`ASTUnit.cpp`.
Repository:
rC Clang
https://reviews.llvm.org/D47445
__
martong added reviewers: akyrtzi, ddunbar.
martong added a comment.
Adding Argyrios and Daniel as a reviewer for ASTUnit related changes.
Repository:
rC Clang
https://reviews.llvm.org/D47445
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
balazske created this revision.
Herald added subscribers: cfe-commits, martong.
Herald added a reviewer: a.sidorin.
ASTImporter tests may produce source file related warnings, the diagnostic
client should be in correct state to handle it. Added 'beginSourceFile' to set
the client state.
Reposito
13 matches
Mail list logo
