Skip to content

tools: API boosting support for rewriting elaborated types.#9375

Merged
htuch merged 5 commits intoenvoyproxy:masterfrom
htuch:api-type-upgrade
Dec 19, 2019
Merged

tools: API boosting support for rewriting elaborated types.#9375
htuch merged 5 commits intoenvoyproxy:masterfrom
htuch:api-type-upgrade

Conversation

@htuch
Copy link
Copy Markdown
Member

@htuch htuch commented Dec 17, 2019

This PR introduces API boosting support for ElaboratedTypeLoc, one of a
number of Clang AST nodes that we need to analyze and rewrite to move to
the latest type according to the type database.

The approach taken is to generate a replacements YAML file suitable for
consumption by clang-apply-replacements; this is how clang-tidy works
internally. Replacements are a series of patch like file transformation
operations, Clang has nice support for building replacement sets and
serializing to YAML.

This PR also starts to split out api_booster into more modules (just
some utils to start with), introduces some more unit tests and a golden
C++ source file test framework.

Risk level: Low
Testing: New unit and e2e golden tests added.

Signed-off-by: Harvey Tuch htuch@google.com

@htuch
tools: API boosting support for rewriting elaborated types.
d3eb71c 
This PR introduces API boosting support for ElaboratedTypeLoc, one of a
number of Clang AST nodes that we need to analyze and rewrite to move to
the latest type according to the type database.

The approach taken is to generate a replacements YAML file suitable for
consumption by clang-apply-replacements; this is how clang-tidy works
internally. Replacements are a series of patch like file transformation
operations, Clang has nice support for building replacement sets and
serializing to YAML.

This PR also starts to split out api_booster into more modules (just
some utils to start with), introduces some more unit tests and a golden
C++ source file test framework.

Risk level: Low
Testing: New unit and e2e golden tests added.

Signed-off-by: Harvey Tuch <htuch@google.com>
@htuch
Cleanup only *.clang-replacements.yaml instead of *.yaml.
72252cb 
Signed-off-by: Harvey Tuch <htuch@google.com>
@htuch
Copy link
Copy Markdown
Member Author

htuch commented Dec 17, 2019

Note that CI tests are coming, I need to sort out #9339 first.

lizan
lizan reviewed Dec 18, 2019
View reviewed changes
tools/clang_tools/api_booster/proto_cxx_utils.h
// Convert from a C++ type, e.g. foo::bar::v2, to a protobuf type, e.g.
// foo.bar.v2.
static std::string cxxToProtoType(const std::string& cxx_type_name) {
// Convert from C++ to a qualified proto type. This is fairly hacky stuff,
Copy link
Copy Markdown
Member

@lizan lizan Dec 18, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to read this from proto descriptor that embedded in pb.cc source code? I guess it is not that easy in this context?

Copy link
Copy Markdown
Member Author

@htuch htuch Dec 18, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how that would work, since there is still not a direct associative map between the proto and C++ types. I think we could be more robust if we did consult the proto descriptor in addition, but that's probably P2 IMHO in terms of importance at this stage.

Copy link
Copy Markdown
Member Author

@htuch htuch Dec 18, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, it's worth noting that types aren't the only thing that need munging here; all the C++ generated methods and statics, e.g. Foo_Parse() or .mutable_foo() need similar kinds of reverse engineering. I think the conventions permit this all using string operations, but AFAIK there is no reflection possible on this.

derekargueta
derekargueta reviewed Dec 18, 2019
View reviewed changes
@htuch
Review feedback.
8994ecc 
Signed-off-by: Harvey Tuch <htuch@google.com>
@htuch htuch merged commit cdad62e into envoyproxy:master Dec 19, 2019
spenceral added a commit to spenceral/envoy that referenced this pull request Dec 20, 2019
@spenceral
Merge branch 'master' into filter-config-dump
ade8dbe 
* master: (167 commits)
  stats: Avoid asserts in fuzz-tests by eliminating arbitrary length limits, using the new MemBlock wrapper for memcpy (envoyproxy#8779)
  Make %UPSTREAM_LOCAL_ADDRESS% access-log format work for HTTP requests. (envoyproxy#9362)
  tools: API boosting support for using decls and enum constants. (envoyproxy#9418)
  Fix incorrect cluster InitializePhase type (envoyproxy#9379)
  build: fix merge race between envoyproxy#9241 and envoyproxy#9413. (envoyproxy#9427)
  fuzz: fix incorrect evaluator test (envoyproxy#9402)
  server: fix bogus startup log message (envoyproxy#9404)
  tools: Add protoxform tests (envoyproxy#9241)
  api: options after import (envoyproxy#9413)
  misc: use std::move instead of constructing a copy (envoyproxy#9415)
  tools: API boosting support for rewriting elaborated types. (envoyproxy#9375)
  docs: fix invalid transport_socket value (envoyproxy#9403)
  fix typo in docs (envoyproxy#9394)
  srds: remove to-de-removed scopes first and then apply additions to avoid scope key conflict. (envoyproxy#9366)
  api: generate whole directory and sync (envoyproxy#9382)
  bazel: Add load statements for proto_library (envoyproxy#9367)
  Fix typo (envoyproxy#9388)
  Correct test of OptionsImpl argc type (Was: Correct type for std::array size() result) (envoyproxy#9290)
  http1 encode trailers in chunk encoding (envoyproxy#8667)
  Add mode to PipeInstance (envoyproxy#8423)
  ...
prakhag1 pushed a commit to prakhag1/envoy that referenced this pull request Jan 3, 2020
@htuch @prakhag1
tools: API boosting support for rewriting elaborated types. (envoypro…
62b9368 
…xy#9375)

This PR introduces API boosting support for ElaboratedTypeLoc, one of a
number of Clang AST nodes that we need to analyze and rewrite to move to
the latest type according to the type database.

The approach taken is to generate a replacements YAML file suitable for
consumption by clang-apply-replacements; this is how clang-tidy works
internally. Replacements are a series of patch like file transformation
operations, Clang has nice support for building replacement sets and
serializing to YAML.

This PR also starts to split out api_booster into more modules (just
some utils to start with), introduces some more unit tests and a golden
C++ source file test framework.

Risk level: Low
Testing: New unit and e2e golden tests added.

Part of envoyproxy#8082

Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Prakhar <prakhar_au@yahoo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mattklein123 mattklein123 Awaiting requested review from mattklein123

2 more reviewers

@lizan lizan lizan approved these changes

@derekargueta derekargueta derekargueta approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@lizan lizan

@derekargueta derekargueta

@mattklein123 mattklein123

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@htuch @lizan @derekargueta @mattklein123