[PATCH] D136886: [clang] ASTImporter: Fix importing of va_list types and declarations

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/Sema/Sema.cpp:452
 
+  if (auto *ND = dyn_cast_or_null<NamedDecl>(Context.getVaListTagDecl())) {
+    if (auto *DC = ND->getLexicalDeclContext(); DC->isStdNamespace()) {
----------------
The API changed names recently.


================
Comment at: clang/lib/Sema/Sema.cpp:460-462
+        Scope S(TUScope, Scope::DeclScope, getDiagnostics());
+        PushDeclContext(&S, DC);
+        PushOnScopeChains(ND, &S);
----------------
Is it valid to use a local variable as a `Scope` object? I would assume that scope goes away at the end of this compound statement? I didn't spot other cases where we did this -- usually we call `Parser::EnterScope()` which allocates a new `Scope` that `Sema` then uses.


================
Comment at: clang/test/AST/ast-dump-overloaded-operators.cpp:27
 // CHECK-NEXT: | `-ParmVarDecl {{.*}} <col:18> col:19{{( imported)?}} 'E'
-// CHECK-NEXT: `-FunctionDecl {{.*}} <line:14:1, line:18:1> line:14:6{{( imported)?}} test 'void ()'
+// CHECK-NEXT:  -FunctionDecl {{.*}} <line:14:1, line:18:1> line:14:6{{( imported)?}} test 'void ()'
 // CHECK-NEXT:   `-CompoundStmt {{.*}} <col:13, line:18:1>
----------------
This looks like a benign typo -- we still match the line because FileCheck will match partial lines, but I'm pretty sure nothing in your patch would have necessitated this change. Then again, you make this change in a lot of tests, so maybe I'm wrong -- in which case, what changed?


================
Comment at: clang/test/AST/ast-dump-traits.cpp:55
 // CHECK-NEXT: |     `-ExpressionTraitExpr {{.*}} <col:10, col:28> 'bool' __is_lvalue_expr
-// CHECK-NEXT: `-FunctionDecl {{.*}} <line:30:1, line:35:1> line:30:6{{( imported)?}} test_unary_expr_or_type_trait 'void ()'
+// CHECK-NEXT:  -FunctionDecl {{.*}} <line:30:1, line:35:1> line:30:6{{( imported)?}} test_unary_expr_or_type_trait 'void ()'
 // CHECK-NEXT:   `-CompoundStmt {{.*}} <col:38, line:35:1>
----------------
Typo?


================
Comment at: clang/test/AST/fixed_point.c:405
 
-//CHECK-NEXT: `-VarDecl {{.*}} literallast '_Accum' cinit
+//CHECK-NEXT:  -VarDecl {{.*}} literallast '_Accum' cinit
 //CHECK-NEXT:   `-FixedPointLiteral {{.*}} '_Accum' 1.0
----------------
Typo?


================
Comment at: clang/test/PCH/stmt-openmp_structured_block-bit.cpp:16
 // CHECK: TranslationUnitDecl 0x{{.*}} <<invalid sloc>> <invalid sloc>
-// CHECK: `-FunctionDecl 0x{{.*}} <{{.*}}stmt-openmp_structured_block-bit.cpp:8:1, line:11:1> line:8:6 {{(test|imported test)}} 'void ()'
+// CHECK:  -FunctionDecl 0x{{.*}} <{{.*}}stmt-openmp_structured_block-bit.cpp:8:1, line:11:1> line:8:6 {{(test|imported test)}} 'void ()'
 // CHECK-NEXT:   `-CompoundStmt 0x{{.*}} <col:13, line:11:1>
----------------
Typo?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136886/new/

https://reviews.llvm.org/D136886