[PATCH] D60499: [ASTImporter] Various source location and range import fixes.

balazske added a comment. The fix looks OK (other alternative is to remove the CHECK with CXXRecordDecl, or make a single line with regular expressions). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60499/new/ https://reviews.llvm.org/D60499

[PATCH] D60499: [ASTImporter] Various source location and range import fixes.

dyung added a comment. This change caused the Import/namespace/struct-and-var/test.cpp test to fail on ARM due to an extra line with `-CXXRecordDecl being emitted by the compiler that was being matched instead of the intended line. I checked in a fix to tighten up the check a little more so

[PATCH] D60499: [ASTImporter] Various source location and range import fixes.

This revision was automatically updated to reflect the committed changes. Closed by commit rGa9f10ebffaa2: [ASTImporter] Various source location and range import fixes. (authored by balazske). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60499/new/

[PATCH] D60499: [ASTImporter] Various source location and range import fixes.

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60499/new/ https://reviews.llvm.org/D60499

[PATCH] D60499: [ASTImporter] Various source location and range import fixes.

balazske updated this revision to Diff 232097. balazske added a comment. Rebased to monorepo and newer master. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60499/new/ https://reviews.llvm.org/D60499 Files: clang/lib/AST/ASTImporter.cpp

[PATCH] D60499: [ASTImporter] Various source location and range import fixes.

martong added a comment. Ping Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60499/new/ https://reviews.llvm.org/D60499 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D60499: [ASTImporter] Various source location and range import fixes.

martong added a comment. Ping @shafik @balazske Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60499/new/ https://reviews.llvm.org/D60499 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D60499: [ASTImporter] Various source location and range import fixes.

balazske added a comment. Ping Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60499/new/ https://reviews.llvm.org/D60499 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D60499: [ASTImporter] Various source location and range import fixes.

balazske added a comment. Added lit tests for the simple (`Decl`) cases. The `Expr` and type related changes are more tricky to do. But I do not like this approach to add new tests because the test function in D60463 does almost the same thing for every import

[PATCH] D60499: [ASTImporter] Various source location and range import fixes.

balazske updated this revision to Diff 202438. balazske added a comment. - Import BraceRange of EnumDecl. - Added lit tests. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60499/new/ https://reviews.llvm.org/D60499 Files: lib/AST/ASTImporter.cpp

[PATCH] D60499: [ASTImporter] Various source location and range import fixes.

shafik requested changes to this revision. shafik added a comment. This revision now requires changes to proceed. Actually I was mistaken, we can see the difference for `EnumDecl` and `ClassTemplateSpecializationDecl` as well. For `EnumDecl` before: EnumDecl 0x7fd0ae884800 col:6 referenced

[PATCH] D60499: [ASTImporter] Various source location and range import fixes.

balazske added a comment. It looks like that the same test can be applied as in D60463 but check only the first line of the AST dump. The first line contains information about the actual Decl only. This checks less than checking the full AST dump but finds

[PATCH] D60499: [ASTImporter] Various source location and range import fixes.

shafik added a comment. I think the AST dump for `EnumDecl` and `ClassTemplateSpecializationDecl` should be dumping the missing `SourceLocations` but I am having trouble tracking down where that should be done. Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D60499: [ASTImporter] Various source location and range import fixes.

shafik added a comment. So an alternative to testing could be matching the AST output from a `clang-import-test` like we did here: https://reviews.llvm.org/D61140 although it unfortunately looks like only only the AST dump of `NamespaceDecl` output the `SourceLocation` you are fixing, see it

[PATCH] D60499: [ASTImporter] Various source location and range import fixes.

balazske marked 3 inline comments as done. balazske added inline comments. Comment at: lib/AST/ASTImporter.cpp:6129 if (Error Err = -ImportTemplateArgumentListInfo(E->template_arguments(), ToTAInfo)) +ImportTemplateArgumentListInfo(E->getLAngleLoc(),

[PATCH] D60499: [ASTImporter] Various source location and range import fixes.

balazske added a comment. There is a test for the SourceLocation import in https://reviews.llvm.org/D60463 (after this patch is applied that test should not fail and the "return" statement is to be removed). Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D60499: [ASTImporter] Various source location and range import fixes.

shafik added a comment. I don't see any regressions but I am a little uncomfortable since there are no tests. So I would feel better if this was split into three parts: Namespaces, Enums and Templates. Are there small test programs that fail due to the missing data? We can add them as

[PATCH] D60499: [ASTImporter] Various source location and range import fixes.

balazske updated this revision to Diff 201521. balazske added a comment. - Import BraceRange of EnumDecl. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60499/new/ https://reviews.llvm.org/D60499 Files: lib/AST/ASTImporter.cpp Index: lib/AST/ASTImporter.cpp

[PATCH] D60499: [ASTImporter] Various source location and range import fixes.

balazske created this revision. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp. Herald added a reviewer: martong. Herald added a reviewer: a.sidorin. Herald added a reviewer: shafik. Herald added a project: clang. ASTImporter contained wrong or missing imports of