Skip to content

Improve TreeTransform for positional parameter expressions. #576

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 20, 2018

Conversation

dtarditi
Copy link
Member

In TreeTransform.h, track the transformed types for parameters. Use
this information to update the types for positional parameter expressions.
This fixes issue #484, which was a typechecking error caused by different types
for uses of parameters in bounds expressions in function types.

The tracking is straightforward: we add a vector of types for parameters.
Positional parameters only occur in bounds expressions in types. When
we transform a function prototype type, we record the transformed types for
parameters before transforming the bounds expressions in the types.

This change also contains a few clean up changes:

  1. While debugging an alternative approach, I noticed that TreeTransform
    had some code that is flagged as specific to template instantiation. It
    was mostly code for marking uses. The code in turn called into code that
    checked for detecting references to local variables defined in other lexical
    contexts (for example, block scopes). Those checks depend on the lexical context
    state for Sema. We don't want any of this code running for the kinds
    of transforms that we are doing, so I added a new method to TreeTransform.h that
    can be overridden to suppress this code.
  2. Clean up and improve some comments in TreeTransform.h.
  3. Improve the parameters for TransformExtendedParameterInfo, marking some
    as const and changing the order of parameters to group related parameters
    together.

Testing:

  • Added new regression test for issue 484.
  • Passed local testing on Windows.
  • Passed automated testing.

Change the transformation of positional parameters in TreeTransform.h
to use the transformed types for parameters.  Do this by using the
types passed into TransformExtendedParameterInfo.

Before this, the code was just transforming the type for the positional
parameter based on however types were being transformed.,  This worked fine
for a type-only transform, but didn't work properly when the parameter type
was transformed based on some other informaton in the program (such as a
bounds-safe interface).

This fixes issue #484.

Testing:
- Add a regression test based on the case described in issue #484.
@dtarditi dtarditi merged commit 970cba4 into master Oct 20, 2018
@dtarditi dtarditi deleted the issue484-take2 branch November 1, 2018 00:42
sulekhark pushed a commit that referenced this pull request Jul 8, 2021
This requires replacing `%clang_cc1 -verify` with `%clang -Xclang
-verify` in the tests that use it.

This is one part of the RUN command cleanup that we've wanted to do for
a while in #346, and now it is blocking part of #576. This is because
the single behavior difference we know of between %clang_cc1 and %clang
(other than the need for `-Xclang`) is that %clang_cc1 turns off the
system headers, and we want two of our tests that currently use
%clang_cc1 to be able to get checked declarations from the system
headers.

As recently as dac3f97 (August 2020),
all of the tests that used %clang_cc1 used -verify. Since then, a few
tests have been added that use %clang_cc1 and not -verify. This lends
support to my theory that the only reason we used %clang_cc1 was that we
were unaware that -verify could be used with %clang via -Xclang, and
some subsequently added tests copied the use of %clang_cc1 only because
the authors didn't know any better. Thus, I believe that the risk of the
migration affecting the validity of the tests (e.g., causing them to
falsely pass) is low.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant