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

DonĂ¡t Nagy via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 15 07:08:56 PST 2023


donat.nagy added a comment.

In D142822#4102795 <https://reviews.llvm.org/D142822#4102795>, @balazske wrote:

> I looked at some of the failing tests but can not decide how to fix the problems. The problem seems to be with the extra `namespace std` that was not there before. I do not know what the tests are exactly testing. A part of the tests can be fixed by adding new expect lines but these must be platform specific.

It seems that your change in `Sema.cpp` creates the `std` namespace (or makes it visible?) in some situations where it's not declared by the code and should not be visible. I suspect that this behavior of your commit is not compliant with the standard, and there is precedent that Clang code pays attention to avoiding this sort of thing. For example commit 87f54060 <https://github.com/llvm/llvm-project/commit/87f540608106f28d9319b2148eb1b00192e020c2>  introduces the simple testcase

  int *use_new(int N) {
    return new int [N];
  }
  
  int std = 17;

which ensures that the `std` namespace is not visible even if a reference to `operator new` (whose full type references `std:bad_alloc`) is present.

I think the commit needs to be updated to avoid "leaking" the `std` namespace into code that do not declare it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142822



More information about the cfe-commits mailing list