[PATCH] D55646: [ASTImporter] Make ODR diagnostics warning by default

gamesh411 abandoned this revision. gamesh411 added a comment. I am creating a new revision that keeps the old handling of ODR violations the same when used by parts of the Sema, but emit warnings when used by the ASTImporter. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D55646: [ASTImporter] Make ODR diagnostics warning by default

gamesh411 added a comment. Herald added a subscriber: jdoerfert. Herald added a project: clang. I am going to abandon this modification, as setting ODR violations as warnings, seems like a change that could have unforeseen consequences. Repository: rC Clang CHANGES SINCE LAST ACTION https:

[PATCH] D55646: [ASTImporter] Make ODR diagnostics warning by default

aaron.ballman added a comment. In general, I agree that it doesn't make sense for ODR violations (with false positives) to trigger error diagnostics while performing AST merging, so downgrading this to warnings is a good idea. However, I do worry about impacting modules and C compatibility with

[PATCH] D55646: [ASTImporter] Make ODR diagnostics warning by default

martong added a comment. Just a quick note. We are pretty sure that `StructuralEquivalency` can have false positive results, i.e. it can report two decls as nonequivalent falsely. My understanding is that we should report an error only if we are absolutely certain that an error has happened, th

[PATCH] D55646: [ASTImporter] Make ODR diagnostics warning by default

gamesh411 added a comment. Ping. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55646/new/ https://reviews.llvm.org/D55646 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/

[PATCH] D55646: [ASTImporter] Make ODR diagnostics warning by default

gamesh411 added a comment. Hey Aleksei! Thank you for the input! ODR violations being warnings would be beneficial from the code maintenance point of view, as we would not have to resort to duplicate some (if not most) of them as errors. There is also a flexibility advantage in the diagnostics,

[PATCH] D55646: [ASTImporter] Make ODR diagnostics warning by default

a_sidorin added subscribers: bruno, aaron.ballman, rsmith. a_sidorin added a comment. Hello Endre. I agree that it doesn't make sense to have 'errors' in AST merging tools, and the changes for ASTImporter part are welcome. However, I don't feel so positive to modules support changes and I don't

[PATCH] D55646: [ASTImporter] Make ODR diagnostics warning by default

martong added a comment. The primary reason why want these to be Warnings instead of Errors because there are many false positive ODR Errors at the moment. These are not fatal errors but there is a maximum error count which once reached the whole process (analysis) is aborted. Usually, such fal

[PATCH] D55646: [ASTImporter] Make ODR diagnostics warning by default

gamesh411 added a comment. In order to be able to handle ODR-related diagnostics with command line options, these diagnostics were moved from Error category to Warning. What are your thoughts? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55646/new/ https://rev

[PATCH] D55646: [ASTImporter] Make ODR diagnostics warning by default

gamesh411 created this revision. Herald added subscribers: cfe-commits, Szelethus, martong, dkrupp. Herald added a reviewer: a.sidorin. Herald added a reviewer: shafik. - Make ODR diagnostics warning by default After this change all odr-related diagnostics are considered warnings belonging to Dia