andreyg/ifc-reader

ifc-reader's Issues

`using enum` is only supported in clang >= 13

See: https://clang.llvm.org/cxx_status.html

This affects:

• ifc-reader/examples/dump-decls/Presenter.cpp

  Line 12 in b8ea308

  using enum ifc::NameSort;

• ifc-reader/examples/dump-decls/Presenter.cpp

  Line 50 in b8ea308

  using enum ifc::FunctionTypeTraits;

Remove boost dependency

Currently ifc-reader depends on boost-iostreams for memory mapping the ifc file.

I propose letting this be application defined, and which means they can use boost-iostreams themselves, or use something more lightweight like mio, or just reading a file the normal way.

Then the ifc-reader would only need to accept a const char*, similar to raw_pdb for example.

We could still have boost-iostreams as a dependency for our examples & unit tests. Just have it not be required for any applications that want to use this as a library.

Let me know what you think!

Name implementation not complete yet

I ran into an issue where I wanted to access the conversion function name, which wasn't exposed yet.

So I fully implemented the Name kinds and structures, submitted as PR #34.

Parenthesized initialization of aggregates is unsupported in `clang`

See: https://clang.llvm.org/cxx_status.html

This affects:

• ifc-reader/lib/msvc/src/MSVCEnvironment.cpp

  Line 30 in b0d44e9

  config.imported_header_units.emplace_back(std::move(header), std::move(bmi));

• ifc-reader/lib/msvc/src/MSVCEnvironment.cpp

  Line 41 in b0d44e9

  config.imported_modules.emplace_back(std::move(name), std::move(bmi));

Make reflifc types classes

Partly a stylistic choice, but it's commonly agreed that structs meant for more "Plain old data" and classes are meant for types with functions.

This is also supported by the C++ Core Guidelines C.2.

License

Hi @AndreyG,

Thanks for the great work you've done on this project so far, it looks seriously awesome!

I'd be interested in playing around with some of your code, but I'm hesitant to do so without their being a license on the repo -- would you be able to add one? MIT would be awesome, if you'd be happy with that.

Thanks!

reflifc default constructibility of types

In principle, reflifc wraps an AbstractReference with the accompanying ifc File. These both have sane defaults. An AbstractReference has AbstractReference::is_null() and the file can be nullptr.

I wanted to start a discussion, if making these types default constructible was something you had already considered. I've found it nice if I can default construct them.

Support Expression::TemplateReference

See https://github.com/microsoft/ifc-spec/blob/03aa5c1bd5fd7946fd55ed76ee31734f6a9e603b/ltx/exprs.tex#L1537

Ability to get reflifc::Module from any reflifc object.

All reflifc types combine the ifc::File and some pointer/index, however the file is never exposed.

This might be partly a design decision, but for my current use case I need to check if a certain type/declaration from a certain module (so ifc::File, or reflifc::Module). To determine if a type is visibility or reachable.

Do you have any concerns with adding this to the API?

Support Declaration::{Specialization, PartialSpecialization}

See
https://github.com/microsoft/ifc-spec/blob/03aa5c1bd5fd7946fd55ed76ee31734f6a9e603b/ltx/decls.tex#L663, https://github.com/microsoft/ifc-spec/blob/03aa5c1bd5fd7946fd55ed76ee31734f6a9e603b/ltx/decls.tex#L700.

Allow reflifc types to be assignable.

Currently I see that most types have a reference back to their file as ifc::File const&. This is considered bad practice because it makes the type not copy/move assignable. I suggest keeping the references in the constructor to keep the invariance, but store a pointer in the class.

This is also backed up by the C++ Core Guidelines C.12.

Unit testing

It would be good to have some unit tests to maintain the library, as you mentioned too.

The simple approach would be to upload a bunch of .ifc files for specific tests.

However, I'd argue it's also important to build those .ifc's from source code. So you are verifying a platform instead of just the platform of the developer who uploaded that specific .ifc file. So I prefer this approach.

My issue is I am not aware of a reliable way to get the path to a .ifc file from a .ixx file.
Eg Visual Studo Open Folder (so with Ninja as the build system) will follow relative subfolders to the .ifc file, and a .sln build will plonk everything in the intermediate directory.

Do you think this be abstracted in the MSVCEnvironment? we could have different defines identifying if we are running from CMake, Visual Studio, Ninja or MSBuild for example.

(Incidently this linking of an .ixx to it's .ifc file is something I need to solve for my own side project too.

`d.json` suffix doesn't match MSVC

With cl.exe @ 19.30.30705.0, compiling the following:

cl.exe /W4 /WX /experimental:module /interface /sourceDependencies . /std:c++latest /EHsc /MD /c mod_moo.cpp

where the argument to /sourceDependencies is ., then MSVC gives you:

mod_moo.cpp.json

There are two things here:

1. ifc-reader looks for the suffix .d.json, but the output from MSVC is just .json
2. ifc-reader looks for <ifc_name>.<suffix> but MSVC generates <src_file_name>.<suffix> (that is, the name of the output file is the name of the source file and not the name of the IFC file)

Maybe a suggested fix here should be that ifc-reader could take an additional argument for the path to the source dependencies file?

`uint32_t` becomes `Unsupported DeclSort '8'`

Issue

I've just been revisiting my steps from:

• #10 (comment)

which work apart from the MSVC hello.cpp.json needs to renamed to hello.ifc.d.json (but that's fine).

However, when I run dump-decls on this example:

export module hello;

import std.core;

export namespace hello {
  void say_hello(uint32_t count);
}

I get:

IFC Version: 0.43
File contains 18 partitions
Global Scope Index: 1
Total declarations count: 2
Scopes count: 2
Count of declarations from all scopes: 2
-------------------------------------- Global Scope --------------------------------------
Namespace 'hello' {
  Function 'say_hello', type: void(Unsupported DeclSort '8')
}

So I'm surprised I'm seeing Unsupported DeclSort '8' rather than uint32_t (the 8 is also weird, because if this was bytes it should be 4).

Is this expected? Should I be able to see uint32_t here?

Notes

The IFC was generated with:

cl.exe /Bv
Microsoft (R) C/C++ Optimizing Compiler Version 19.40.33521 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

Compiler Passes:
 Z:\home\avj\visual_studio\MSVC\14.40.33521\bin\Hostx64\x64\cl.exe:        Version 19.40.33521.0
 Z:\home\avj\visual_studio\MSVC\14.40.33521\bin\Hostx64\x64\c1.dll:        Version 19.40.33521.0
 Z:\home\avj\visual_studio\MSVC\14.40.33521\bin\Hostx64\x64\c1xx.dll:      Version 19.40.33521.0
 Z:\home\avj\visual_studio\MSVC\14.40.33521\bin\Hostx64\x64\c2.dll:        Version 19.40.33521.0
 Z:\home\avj\visual_studio\MSVC\14.40.33521\bin\Hostx64\x64\c1xx.dll:      Version 19.40.33521.0
 Z:\home\avj\visual_studio\MSVC\14.40.33521\bin\Hostx64\x64\link.exe:      Version 14.40.33521.0
 Z:\home\avj\visual_studio\MSVC\14.40.33521\bin\Hostx64\x64\mspdb140.dll:  Version 14.40.33521.0
 Z:\home\avj\visual_studio\MSVC\14.40.33521\bin\Hostx64\x64\1033\clui.dll: Version 19.40.33521.0

Make reflifc types hashable and Comparable.

For my use cases I really need to cache declarations etc to avoid circular dependencies. While traversing all types.

Most of the reflifc types are build as: ifc::File* + ifc::SomeIndex or ifc::PartitionData*.
Those two numbers can easily be hashed together with a hash combine. But I cannot do it outside the library as the members are private.

Would you like me to work on a pull request for this? Or would you prefer to put it in yourself.