search

eclipse-embed-cdt / eclipse-plugins

The Eclipse Embedded CDT plug-ins for Arm & RISC-V C/C++ developers (formerly known as the GNU MCU Eclipse plug-ins). Includes the archive of previous plug-ins versions, as Releases.
http://eclipse-embed-cdt.github.io/
Eclipse Public License 2.0
557 stars 130 forks source link

Use new ALL_FLAGS feature of CDT for discovery #547

Open jonahgraham opened 1 year ago

jonahgraham commented 1 year ago

CDT 11 includes a new ${ALL_FLAGS} that can be specified in the scanner discovery to pick up all command line options specified in the managed build.

See the N&N entry at:

https://github.com/jonah/cdt/blob/main/NewAndNoteworthy/CDT-11.0.md#scanner-discovery-considering-all-flags (here until https://github.com/eclipse-cdt/cdt/pull/158 is merged)

This change is not required as CDT's change is non-breaking. But to use ALL_FLAGS you need CDT 11 or newer, specifically for org.eclipse.cdt.managedbuilder.core;bundle-version="9.5.0"

jonahgraham commented 1 year ago

Marked as draft until https://github.com/eclipse-cdt/cdt/pull/158 is resolved.

ilg-ul commented 1 year ago

I'm not sure I understand the details. :-(

Anyway, embed-cdt is based on an older CDT, and I am reluctant to update to CDT 11.

jonahgraham commented 1 year ago

I'm not sure I understand the details. :-(

Another concrete example is in the original bug report, if a user specifies -mcpu=cortex-m4 then the __ARM_ARCH pre-defined macro will be 7, but CDT won't know that because it didn't pass the -mcpu=cortex-m4` to GCC when querying GCC for the pre-defined macros. As a result the indexing of source files will be incorrect.

With CDT 10, you can fix the above situation (if you agree it is a problem) by adding the useByScannerDiscovery="true" to the -mcpu= option in the plugin.xml, like you have done for other options.

What this change does is effectively defaults to include all options for scanner discovery, except those explictly excluded.

I am reluctant to update to CDT 11.

That's fine - if users are using CDT 11, like with Eclipse 2022-12 they can make the change manually. This commit only changes the default for new projects and on its own is certainly not enough to bother updating to CDT 11.

ilg-ul commented 1 year ago

As things are now, the update URL is https://download.eclipse.org/embed-cdt/updates/v6 and is expected to be valid for existing Eclipses back to the initial Eclipse used after the migration to EF, so as long as these old Eclipses can still be updated, we should be fine.

If, for any reason, we decide to bump the base version to a more recent Eclipse/CDT, we should increase the version to v7 and change the update URL. These changes take some time, thus my reluctance, but otherwise any improvements are welcome.

jonahgraham commented 1 year ago

We can leave this as a draft until then, whenever "then" is.